r293745 - Doxygen comments for prfchwintrin.h

[Ekaterina Romanova via cfe-commits]

Author: kromanova
Date: Wed Feb  1 01:37:40 2017
New Revision: 293745

URL: http://llvm.org/viewvc/llvm-project?rev=293745=rev
Log:
Doxygen comments for prfchwintrin.h

Added doxygen comments to prfchwintrin.h's intrinsics. 

Note: The doxygen comments are automatically generated based on Sony's intrinsic
s document.

I got an OK from Eric Christopher to commit doxygen comments without prior code
review upstream.




Modified:
cfe/trunk/lib/Headers/prfchwintrin.h

Modified: cfe/trunk/lib/Headers/prfchwintrin.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/prfchwintrin.h?rev=293745=293744=293745=diff
==
--- cfe/trunk/lib/Headers/prfchwintrin.h (original)
+++ cfe/trunk/lib/Headers/prfchwintrin.h Wed Feb  1 01:37:40 2017
@@ -29,12 +29,36 @@
 #define __PRFCHWINTRIN_H
 
 #if defined(__PRFCHW__) || defined(__3dNOW__)
+/// \brief Loads a memory sequence containing the specified memory address into
+///all data cache levels. The cache-coherency state is set to exclusive.
+///Data can be read from and written to the cache line without additional
+///delay.
+///
+/// \headerfile 
+///
+/// This intrinsic corresponds to the \c PREFETCHT0 instruction.
+///
+/// \param __P
+///A pointer specifying the memory address to be prefetched.
 static __inline__ void __attribute__((__always_inline__, __nodebug__))
 _m_prefetch(void *__P)
 {
   __builtin_prefetch (__P, 0, 3 /* _MM_HINT_T0 */);
 }
 
+/// \brief Loads a memory sequence containing the specified memory address into
+///the L1 data cache and sets the cache-coherency to modified. This
+///provides a hint to the processor that the cache line will be modified.
+///It is intended for use when the cache line will be written to shortly
+///after the prefetch is performed. Note that the effect of this intrinsic
+///is dependent on the processor implementation.
+///
+/// \headerfile 
+///
+/// This intrinsic corresponds to the \c PREFETCHW instruction.
+///
+/// \param __P
+///A pointer specifying the memory address to be prefetched.
 static __inline__ void __attribute__((__always_inline__, __nodebug__))
 _m_prefetchw(void *__P)
 {


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

[PATCH] D29369: [ubsan] Omit superflous overflow checks for promoted arithmetic (PR20193)

[Vedant Kumar via Phabricator via cfe-commits]

vsk created this revision.

C requires the operands of arithmetic expressions to be promoted if
their types are smaller than an int. Ubsan emits overflow checks when
this sort of type promotion occurs, even if there is no way to actually
get an overflow with the promoted type.

This patch teaches clang how to omit the superflous overflow checks
(addressing PR20193). To my knowledge, this patch only misses the case
where we have multiplications with promoted unsigned operands. E.g, in
this case, we don't need an overflow check when targeting a platform
with >=32-bit ints:

  uint8_t a, b;
  a * b;

Testing: check-clang and check-ubsan.


https://reviews.llvm.org/D29369

Files:
  lib/CodeGen/CGExprScalar.cpp
  test/CodeGen/compound-assign-overflow.c
  test/CodeGen/ubsan-promoted-arith.c
  test/CodeGen/unsigned-promotion.c

Index: test/CodeGen/unsigned-promotion.c
===
--- test/CodeGen/unsigned-promotion.c
+++ test/CodeGen/unsigned-promotion.c
@@ -9,52 +9,6 @@
 unsigned short si, sj, sk;
 unsigned char ci, cj, ck;
 
-extern void opaqueshort(unsigned short);
-extern void opaquechar(unsigned char);
-
-// CHECKS-LABEL:   define void @testshortadd()
-// CHECKU-LABEL: define void @testshortadd()
-void testshortadd() {
-  // CHECKS:load i16, i16* @sj
-  // CHECKS:load i16, i16* @sk
-  // CHECKS:[[T1:%.*]] = call { i32, i1 } @llvm.sadd.with.overflow.i32(i32 [[T2:%.*]], i32 [[T3:%.*]])
-  // CHECKS-NEXT:   [[T4:%.*]] = extractvalue { i32, i1 } [[T1]], 0
-  // CHECKS-NEXT:   [[T5:%.*]] = extractvalue { i32, i1 } [[T1]], 1
-  // CHECKS:call void @__ubsan_handle_add_overflow
-  //
-  // CHECKU:  [[T1:%.*]] = load i16, i16* @sj
-  // CHECKU:  [[T2:%.*]] = zext i16 [[T1]]
-  // CHECKU:  [[T3:%.*]] = load i16, i16* @sk
-  // CHECKU:  [[T4:%.*]] = zext i16 [[T3]]
-  // CHECKU-NOT:  llvm.sadd
-  // CHECKU-NOT:  llvm.uadd
-  // CHECKU:  [[T5:%.*]] = add nsw i32 [[T2]], [[T4]]
-
-  si = sj + sk;
-}
-
-// CHECKS-LABEL:   define void @testshortsub()
-// CHECKU-LABEL: define void @testshortsub()
-void testshortsub() {
-
-  // CHECKS:load i16, i16* @sj
-  // CHECKS:load i16, i16* @sk
-  // CHECKS:[[T1:%.*]] = call { i32, i1 } @llvm.ssub.with.overflow.i32(i32 [[T2:%.*]], i32 [[T3:%.*]])
-  // CHECKS-NEXT:   [[T4:%.*]] = extractvalue { i32, i1 } [[T1]], 0
-  // CHECKS-NEXT:   [[T5:%.*]] = extractvalue { i32, i1 } [[T1]], 1
-  // CHECKS:call void @__ubsan_handle_sub_overflow
-  //
-  // CHECKU:  [[T1:%.*]] = load i16, i16* @sj
-  // CHECKU:  [[T2:%.*]] = zext i16 [[T1]]
-  // CHECKU:  [[T3:%.*]] = load i16, i16* @sk
-  // CHECKU:  [[T4:%.*]] = zext i16 [[T3]]
-  // CHECKU-NOT:  llvm.ssub
-  // CHECKU-NOT:  llvm.usub
-  // CHECKU:  [[T5:%.*]] = sub nsw i32 [[T2]], [[T4]]
-
-  si = sj - sk;
-}
-
 // CHECKS-LABEL:   define void @testshortmul()
 // CHECKU-LABEL: define void @testshortmul()
 void testshortmul() {
@@ -76,50 +30,6 @@
   si = sj * sk;
 }
 
-// CHECKS-LABEL:   define void @testcharadd()
-// CHECKU-LABEL: define void @testcharadd()
-void testcharadd() {
-
-  // CHECKS:load i8, i8* @cj
-  // CHECKS:load i8, i8* @ck
-  // CHECKS:[[T1:%.*]] = call { i32, i1 } @llvm.sadd.with.overflow.i32(i32 [[T2:%.*]], i32 [[T3:%.*]])
-  // CHECKS-NEXT:   [[T4:%.*]] = extractvalue { i32, i1 } [[T1]], 0
-  // CHECKS-NEXT:   [[T5:%.*]] = extractvalue { i32, i1 } [[T1]], 1
-  // CHECKS:call void @__ubsan_handle_add_overflow
-  //
-  // CHECKU:  [[T1:%.*]] = load i8, i8* @cj
-  // CHECKU:  [[T2:%.*]] = zext i8 [[T1]]
-  // CHECKU:  [[T3:%.*]] = load i8, i8* @ck
-  // CHECKU:  [[T4:%.*]] = zext i8 [[T3]]
-  // CHECKU-NOT:  llvm.sadd
-  // CHECKU-NOT:  llvm.uadd
-  // CHECKU:  [[T5:%.*]] = add nsw i32 [[T2]], [[T4]]
-
-  ci = cj + ck;
-}
-
-// CHECKS-LABEL:   define void @testcharsub()
-// CHECKU-LABEL: define void @testcharsub()
-void testcharsub() {
-
-  // CHECKS:load i8, i8* @cj
-  // CHECKS:load i8, i8* @ck
-  // CHECKS:[[T1:%.*]] = call { i32, i1 } @llvm.ssub.with.overflow.i32(i32 [[T2:%.*]], i32 [[T3:%.*]])
-  // CHECKS-NEXT:   [[T4:%.*]] = extractvalue { i32, i1 } [[T1]], 0
-  // CHECKS-NEXT:   [[T5:%.*]] = extractvalue { i32, i1 } [[T1]], 1
-  // CHECKS:call void @__ubsan_handle_sub_overflow
-  //
-  // CHECKU:  [[T1:%.*]] = load i8, i8* @cj
-  // CHECKU:  [[T2:%.*]] = zext i8 [[T1]]
-  // CHECKU:  [[T3:%.*]] = load i8, i8* @ck
-  // CHECKU:  [[T4:%.*]] = zext i8 [[T3]]
-  // CHECKU-NOT:  llvm.ssub
-  // CHECKU-NOT:  llvm.usub
-  // CHECKU:  [[T5:%.*]] = sub nsw i32 [[T2]], [[T4]]
-
-  ci = cj - ck;
-}
-
 // CHECKS-LABEL:   define void @testcharmul()
 // CHECKU-LABEL: define void @testcharmul()
 void testcharmul() {
Index: test/CodeGen/ubsan-promoted-arith.c
===
--- /dev/null
+++ 

r293740 - Remove apparently-unnecessary copy of constructor lookup result.

[Richard Smith via cfe-commits]

Author: rsmith
Date: Tue Jan 31 21:28:59 2017
New Revision: 293740

URL: http://llvm.org/viewvc/llvm-project?rev=293740=rev
Log:
Remove apparently-unnecessary copy of constructor lookup result.

Contrary to the comment, DeclContext intends to guarantee that the lookup
results for a particular name will be stable across non-AST-mutating
operations, so a copy here should not be necessary. Further, if a copy *is*
necessary, the other four instances of this pattern within this file would also
be wrong, and we have no evidence of any problems with them; if this change
unearths problems, we should fix all the instances of this pattern.

Modified:
cfe/trunk/lib/Sema/SemaInit.cpp

Modified: cfe/trunk/lib/Sema/SemaInit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=293740=293739=293740=diff
==
--- cfe/trunk/lib/Sema/SemaInit.cpp (original)
+++ cfe/trunk/lib/Sema/SemaInit.cpp Tue Jan 31 21:28:59 2017
@@ -4679,15 +4679,7 @@ static void TryUserDefinedConversion(Sem
 
 // Try to complete the type we're converting to.
 if (S.isCompleteType(Kind.getLocation(), DestType)) {
-  DeclContext::lookup_result R = S.LookupConstructors(DestRecordDecl);
-  // The container holding the constructors can under certain conditions
-  // be changed while iterating. To be safe we copy the lookup results
-  // to a new container.
-  SmallVector CopyOfCon(R.begin(), R.end());
-  for (SmallVectorImpl::iterator
- Con = CopyOfCon.begin(), ConEnd = CopyOfCon.end();
-   Con != ConEnd; ++Con) {
-NamedDecl *D = *Con;
+  for (NamedDecl *D : S.LookupConstructors(DestRecordDecl)) {
 auto Info = getConstructorInfo(D);
 if (!Info.Constructor)
   continue;


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

Re: Add warning for c++ member variable shadowing

[James Sun via cfe-commits]

Fixed!

From: Saleem Abdulrasool 
Date: Tuesday, January 31, 2017 at 6:53 PM
To: James Sun 
Cc: Richard Smith , "cfe-commits@lists.llvm.org" 
, Aaron Ballman 
Subject: Re: Add warning for c++ member variable shadowing

Hmm, the braces in the if (bases.find(...)...) are not needed.

Could you also add a test case for virtual inheritance?

On Mon, Jan 30, 2017 at 8:34 PM, James Sun 
> wrote:
Hi Saleem

Thanks for the quick response. A test case is added. It covers some ordinary 
cases as well as corner cases like multiple paths to the same base.

Thanks

James

From: Saleem Abdulrasool >
Date: Monday, January 30, 2017 at 6:50 PM
To: James Sun >
Cc: Richard Smith >, 
"cfe-commits@lists.llvm.org" 
>, Aaron Ballman 
>

Subject: Re: Add warning for c++ member variable shadowing

I think that the patch is starting to look pretty good!

Can you add some test cases for the particular cases to diagnose in a separate 
test set to ensure that we have proper coverage of the various cases rather 
than relying on the existing test cases?  Something to make sure that we get 
the simple case right as well as the complex cases (e.g. we don't print 
duplicate warnings for multiple paths).


On Mon, Jan 30, 2017 at 5:50 PM, James Sun 
> wrote:
Hi Richard

Sorry for the late reply. Thank you for giving the feedback! The updated 
version is attached. Please let me know if there is anything improper.

Thanks

James

From: > on behalf of Richard Smith 
>
Date: Friday, January 27, 2017 at 3:03 PM
To: James Sun >
Cc: Saleem Abdulrasool >, 
"cfe-commits@lists.llvm.org" 
>, Aaron Ballman 
>

Subject: Re: Add warning for c++ member variable shadowing

+def warn_shadow_member_variable : Warning<
+  "shadowed variable '%0' in type '%1' inheriting from type '%2'">,

The phrasing of this is incorrect: the things you're warning about are not 
variables, they're non-static data members. Perhaps something like:

  "non-static data member '%0' of '%1' shadows member inherited from type '%2'"

+   InGroup;

Would it make sense to put this in a subgroup of -Wshadow so that it can be 
controlled separately?

+  /// Check if there is a member variable shadowing

Please end comments in a period.

+  void CheckShadowInheritedVariables(const SourceLocation ,

Likewise, 'Variables' is wrong. We would typically use the C term 'Fields' for 
these cases within Clang sources.

+  for (const auto  : DC->bases()) {
+if (const auto *TSI = Base.getTypeSourceInfo())
+  if (const auto *BaseClass = TSI->getType()->getAsCXXRecordDecl()) {
+for (const auto *Field : BaseClass->fields())
+  if (Field->getName() == FieldName)
+Diag(Loc, diag::warn_shadow_member_variable)
+  << FieldName << RD->getName() << BaseClass->getName();
+// Search parent's parents
+CheckShadowInheritedVariables(Loc, FieldName, RD, BaseClass);
+  }
+  }

Maybe we should avoid diagnosing shadowing of members that are inaccessible 
from the derived class? What about if the field name is ambiguous? Also, we 
shouldn't recurse if lookup finds something with the given name in this class, 
and ideally we would only visit each class once, even if it appears multiple 
times in a multiple-inheritance scenario. CXXRecordDecl::lookupInBases can 
handle most of these cases for you automatically, and will also let you build a 
set of paths to problematic base classes in case you want to report those.

On 24 January 2017 at 20:52, James Sun 
> wrote:
Thanks for the comments. The new version is attached.
Wrt two of your questions:

(1)  “The description that you have on CheckShadowInheritedVariables isn't 
really the type of comments that we have in doxygen form.  Im not sure if its 
in line with the rest of the code.”
I’ve read through the doxygen wiki; hopefully it’s fixed; let me know if it’s 
still wrong

(2) “Why are you checking that the DeclContext has a definition rather than the 
record itself?”
There are cases like “struct A; struct B : A {};”, where A does not have a 
definition. The compiler will hit an assertion failure if we call A.bases() 
directly.

Thanks

James


From: 

Re: r291963 - [clang] Emit `diagnose_if` warnings from system headers

[Richard Smith via cfe-commits]

I'm fine with these patches being merged. Hopefully we still have plenty of
time to shake out any problems between now and the release.

On 31 January 2017 at 19:09, George Burgess IV 
wrote:

> > IIUC the major risk is that diagnose_if itself turns out to be broken,
> not that we'd miscompile anything?
>
> Correct. These patches should be NFC to code that doesn't use diagnose_if.
>
> If something about that patch *had* to break existing
> non-diagnose_if-aware code, we're now calling Sema::CheckFunctionCall
> (which checks diagnose_if attributes, checks we're not passing a nullptr
> into _Nonnull args, ...) for more kinds of C++ function calls than before.
> We weren't calling it before in these places as an optimization: in theory,
> it wasn't possible for CheckFunctionCall to find issues in calls to things
> like conversion functions. Now that we have diagnose_if, it is.
>
> On Tue, Jan 31, 2017 at 6:31 PM, Hans Wennborg  wrote:
>
>> I'm Ok with taking the larger patch (r293360 + r293369) too. It's been
>> in tree for a bit, there is still a number of weeks before the
>> release, and IIUC the major risk is that diagnose_if itself turns out
>> to be broken, not that we'd miscompile anything?
>>
>> On Tue, Jan 31, 2017 at 11:11 AM, Richard Smith 
>> wrote:
>> > Yes, this makes sense. We should also decide what we're going to do
>> about
>> > the larger diagnose_if patch that George has asked to be ported to
>> Clang 4.
>> > Are you comfortable taking a patch of that size? If not, perhaps we
>> should
>> > disable the attribute for the Clang 4 release instead.
>> >
>> >
>> > On 31 January 2017 at 10:36, Hans Wennborg  wrote:
>> >>
>> >> Ping?
>> >>
>> >> On Thu, Jan 26, 2017 at 10:21 AM, Hans Wennborg 
>> wrote:
>> >> > Ping?
>> >> >
>> >> > On Mon, Jan 23, 2017 at 4:27 PM, Hans Wennborg 
>> >> > wrote:
>> >> >> Ping?
>> >> >>
>> >> >> On Tue, Jan 17, 2017 at 4:16 PM, Hans Wennborg 
>> >> >> wrote:
>> >> >>> Richard, what do you think?
>> >> >>>
>> >> >>> On Fri, Jan 13, 2017 at 3:16 PM, Eric Fiselier 
>> wrote:
>> >>  I would love to see this merged. It would make it easier to write
>> >>  libc++
>> >>  tests if the tests didn't have to worry about the old 4.0
>> behavior.
>> >> 
>> >>  CC'ing Richard: Would merging this be OK?
>> >> 
>> >>  On Fri, Jan 13, 2017 at 3:46 PM, George Burgess IV
>> >>   wrote:
>> >> >
>> >> > Do we want to consider merging this into the release branch?
>> Seems
>> >> > like
>> >> > more of a bugfix than a feature to me.
>> >> >
>> >> > On Fri, Jan 13, 2017 at 2:11 PM, Eric Fiselier via cfe-commits
>> >> >  wrote:
>> >> >>
>> >> >> Author: ericwf
>> >> >> Date: Fri Jan 13 16:11:40 2017
>> >> >> New Revision: 291963
>> >> >>
>> >> >> URL: http://llvm.org/viewvc/llvm-project?rev=291963=rev
>> >> >> Log:
>> >> >> [clang] Emit `diagnose_if` warnings from system headers
>> >> >>
>> >> >> Summary: In order for libc++ to meaningfully use `diagnose_if`
>> >> >> warnings
>> >> >> they need to be emitted from system headers by default. This
>> patch
>> >> >> changes
>> >> >> the `diagnose_if` warning diagnostic to be shown in system
>> headers.
>> >> >>
>> >> >> Reviewers: george.burgess.iv, rsmith, aaron.ballman
>> >> >>
>> >> >> Subscribers: cfe-commits
>> >> >>
>> >> >> Differential Revision: https://reviews.llvm.org/D28703
>> >> >>
>> >> >> Added:
>> >> >> cfe/trunk/test/Sema/Inputs/diagnose-if-warn-system-header.h
>> >> >> Modified:
>> >> >> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> >> >> cfe/trunk/test/Sema/diagnose_if.c
>> >> >>
>> >> >> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> >> >> URL:
>> >> >>
>> >> >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/
>> Basic/DiagnosticSemaKinds.td?rev=291963=291962=291963=diff
>> >> >>
>> >> >>
>> >> >> 
>> ==
>> >> >> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> (original)
>> >> >> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri
>> Jan 13
>> >> >> 16:11:40 2017
>> >> >> @@ -3380,7 +3380,8 @@ def note_ovl_candidate_has_pass_object_s
>> >> >>  "candidate address cannot be taken because parameter %0
>> has "
>> >> >>  "pass_object_size attribute">;
>> >> >>  def err_diagnose_if_succeeded : Error<"%0">;
>> >> >> -def warn_diagnose_if_succeeded : Warning<"%0">,
>> >> >> InGroup;
>> >> >> +def warn_diagnose_if_succeeded : Warning<"%0">,
>> >> >> InGroup,
>> >> >> +ShowInSystemHeader;
>> >> >>  def 

Re: r291963 - [clang] Emit `diagnose_if` warnings from system headers

[George Burgess IV via cfe-commits]

> IIUC the major risk is that diagnose_if itself turns out to be broken,
not that we'd miscompile anything?

Correct. These patches should be NFC to code that doesn't use diagnose_if.

If something about that patch *had* to break existing non-diagnose_if-aware
code, we're now calling Sema::CheckFunctionCall (which checks diagnose_if
attributes, checks we're not passing a nullptr into _Nonnull args, ...) for
more kinds of C++ function calls than before. We weren't calling it before
in these places as an optimization: in theory, it wasn't possible for
CheckFunctionCall to find issues in calls to things like conversion
functions. Now that we have diagnose_if, it is.

On Tue, Jan 31, 2017 at 6:31 PM, Hans Wennborg  wrote:

> I'm Ok with taking the larger patch (r293360 + r293369) too. It's been
> in tree for a bit, there is still a number of weeks before the
> release, and IIUC the major risk is that diagnose_if itself turns out
> to be broken, not that we'd miscompile anything?
>
> On Tue, Jan 31, 2017 at 11:11 AM, Richard Smith 
> wrote:
> > Yes, this makes sense. We should also decide what we're going to do about
> > the larger diagnose_if patch that George has asked to be ported to Clang
> 4.
> > Are you comfortable taking a patch of that size? If not, perhaps we
> should
> > disable the attribute for the Clang 4 release instead.
> >
> >
> > On 31 January 2017 at 10:36, Hans Wennborg  wrote:
> >>
> >> Ping?
> >>
> >> On Thu, Jan 26, 2017 at 10:21 AM, Hans Wennborg 
> wrote:
> >> > Ping?
> >> >
> >> > On Mon, Jan 23, 2017 at 4:27 PM, Hans Wennborg 
> >> > wrote:
> >> >> Ping?
> >> >>
> >> >> On Tue, Jan 17, 2017 at 4:16 PM, Hans Wennborg 
> >> >> wrote:
> >> >>> Richard, what do you think?
> >> >>>
> >> >>> On Fri, Jan 13, 2017 at 3:16 PM, Eric Fiselier 
> wrote:
> >>  I would love to see this merged. It would make it easier to write
> >>  libc++
> >>  tests if the tests didn't have to worry about the old 4.0 behavior.
> >> 
> >>  CC'ing Richard: Would merging this be OK?
> >> 
> >>  On Fri, Jan 13, 2017 at 3:46 PM, George Burgess IV
> >>   wrote:
> >> >
> >> > Do we want to consider merging this into the release branch? Seems
> >> > like
> >> > more of a bugfix than a feature to me.
> >> >
> >> > On Fri, Jan 13, 2017 at 2:11 PM, Eric Fiselier via cfe-commits
> >> >  wrote:
> >> >>
> >> >> Author: ericwf
> >> >> Date: Fri Jan 13 16:11:40 2017
> >> >> New Revision: 291963
> >> >>
> >> >> URL: http://llvm.org/viewvc/llvm-project?rev=291963=rev
> >> >> Log:
> >> >> [clang] Emit `diagnose_if` warnings from system headers
> >> >>
> >> >> Summary: In order for libc++ to meaningfully use `diagnose_if`
> >> >> warnings
> >> >> they need to be emitted from system headers by default. This
> patch
> >> >> changes
> >> >> the `diagnose_if` warning diagnostic to be shown in system
> headers.
> >> >>
> >> >> Reviewers: george.burgess.iv, rsmith, aaron.ballman
> >> >>
> >> >> Subscribers: cfe-commits
> >> >>
> >> >> Differential Revision: https://reviews.llvm.org/D28703
> >> >>
> >> >> Added:
> >> >> cfe/trunk/test/Sema/Inputs/diagnose-if-warn-system-header.h
> >> >> Modified:
> >> >> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> >> >> cfe/trunk/test/Sema/diagnose_if.c
> >> >>
> >> >> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> >> >> URL:
> >> >>
> >> >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/
> Basic/DiagnosticSemaKinds.td?rev=291963=291962=291963=diff
> >> >>
> >> >>
> >> >> 
> ==
> >> >> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> (original)
> >> >> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri
> Jan 13
> >> >> 16:11:40 2017
> >> >> @@ -3380,7 +3380,8 @@ def note_ovl_candidate_has_pass_object_s
> >> >>  "candidate address cannot be taken because parameter %0 has
> "
> >> >>  "pass_object_size attribute">;
> >> >>  def err_diagnose_if_succeeded : Error<"%0">;
> >> >> -def warn_diagnose_if_succeeded : Warning<"%0">,
> >> >> InGroup;
> >> >> +def warn_diagnose_if_succeeded : Warning<"%0">,
> >> >> InGroup,
> >> >> +ShowInSystemHeader;
> >> >>  def note_ovl_candidate_disabled_by_function_cond_attr : Note<
> >> >>  "candidate disabled: %0">;
> >> >>  def note_ovl_candidate_disabled_by_extension : Note<
> >> >>
> >> >> Added: cfe/trunk/test/Sema/Inputs/dia
> gnose-if-warn-system-header.h
> >> >> URL:
> >> >>
> >> >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/Inpu
> 

r293735 - Follow-up to r293732: add a proper triple to the test

[Hans Wennborg via cfe-commits]

Author: hans
Date: Tue Jan 31 20:48:02 2017
New Revision: 293735

URL: http://llvm.org/viewvc/llvm-project?rev=293735=rev
Log:
Follow-up to r293732: add a proper triple to the test

Modified:
cfe/trunk/test/CodeGenCXX/initializer-list-ctor-order.cpp

Modified: cfe/trunk/test/CodeGenCXX/initializer-list-ctor-order.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/initializer-list-ctor-order.cpp?rev=293735=293734=293735=diff
==
--- cfe/trunk/test/CodeGenCXX/initializer-list-ctor-order.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/initializer-list-ctor-order.cpp Tue Jan 31 
20:48:02 2017
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -std=c++11 %s -emit-llvm -o - -triple %itanium_abi_triple | 
FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-ITANIUM
-// RUN: %clang_cc1 -std=c++11 %s -emit-llvm -o - -triple i686-windows| 
FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-MS
+// RUN: %clang_cc1 -std=c++11 %s -emit-llvm -o - -triple i686-linux-gnu | 
FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-ITANIUM
+// RUN: %clang_cc1 -std=c++11 %s -emit-llvm -o - -triple i686-windows   | 
FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-MS
 
 extern "C" {
 int f();


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

Re: Add warning for c++ member variable shadowing

[Saleem Abdulrasool via cfe-commits]

Hmm, the braces in the if (bases.find(...)...) are not needed.

Could you also add a test case for virtual inheritance?

On Mon, Jan 30, 2017 at 8:34 PM, James Sun  wrote:

> Hi Saleem
>
>
>
> Thanks for the quick response. A test case is added. It covers some
> ordinary cases as well as corner cases like multiple paths to the same base.
>
>
>
> Thanks
>
>
>
> James
>
>
>
> *From: *Saleem Abdulrasool 
> *Date: *Monday, January 30, 2017 at 6:50 PM
> *To: *James Sun 
> *Cc: *Richard Smith , "cfe-commits@lists.llvm.org"
> , Aaron Ballman 
>
> *Subject: *Re: Add warning for c++ member variable shadowing
>
>
>
> I think that the patch is starting to look pretty good!
>
>
>
> Can you add some test cases for the particular cases to diagnose in a
> separate test set to ensure that we have proper coverage of the various
> cases rather than relying on the existing test cases?  Something to make
> sure that we get the simple case right as well as the complex cases (e.g.
> we don't print duplicate warnings for multiple paths).
>
>
>
>
>
> On Mon, Jan 30, 2017 at 5:50 PM, James Sun  wrote:
>
> Hi Richard
>
>
>
> Sorry for the late reply. Thank you for giving the feedback! The updated
> version is attached. Please let me know if there is anything improper.
>
>
>
> Thanks
>
>
>
> James
>
>
>
> *From: * on behalf of Richard Smith <
> rich...@metafoo.co.uk>
> *Date: *Friday, January 27, 2017 at 3:03 PM
> *To: *James Sun 
> *Cc: *Saleem Abdulrasool , "
> cfe-commits@lists.llvm.org" , Aaron Ballman <
> aa...@aaronballman.com>
>
>
> *Subject: *Re: Add warning for c++ member variable shadowing
>
>
>
> +def warn_shadow_member_variable : Warning<
>
> +  "shadowed variable '%0' in type '%1' inheriting from type '%2'">,
>
>
>
> The phrasing of this is incorrect: the things you're warning about are not
> variables, they're non-static data members. Perhaps something like:
>
>
>
>   "non-static data member '%0' of '%1' shadows member inherited from type
> '%2'"
>
>
>
> +   InGroup;
>
>
>
> Would it make sense to put this in a subgroup of -Wshadow so that it can
> be controlled separately?
>
>
>
> +  /// Check if there is a member variable shadowing
>
>
>
> Please end comments in a period.
>
>
>
> +  void CheckShadowInheritedVariables(const SourceLocation ,
>
>
>
> Likewise, 'Variables' is wrong. We would typically use the C term 'Fields'
> for these cases within Clang sources.
>
>
>
> +  for (const auto  : DC->bases()) {
>
> +if (const auto *TSI = Base.getTypeSourceInfo())
>
> +  if (const auto *BaseClass = TSI->getType()->getAsCXXRecordDecl()) {
>
> +for (const auto *Field : BaseClass->fields())
>
> +  if (Field->getName() == FieldName)
>
> +Diag(Loc, diag::warn_shadow_member_variable)
>
> +  << FieldName << RD->getName() << BaseClass->getName();
>
> +// Search parent's parents
>
> +CheckShadowInheritedVariables(Loc, FieldName, RD, BaseClass);
>
> +  }
>
> +  }
>
>
>
> Maybe we should avoid diagnosing shadowing of members that are
> inaccessible from the derived class? What about if the field name is
> ambiguous? Also, we shouldn't recurse if lookup finds something with the
> given name in this class, and ideally we would only visit each class once,
> even if it appears multiple times in a multiple-inheritance scenario.
> CXXRecordDecl::lookupInBases can handle most of these cases for you
> automatically, and will also let you build a set of paths to problematic
> base classes in case you want to report those.
>
>
>
> On 24 January 2017 at 20:52, James Sun  wrote:
>
> Thanks for the comments. The new version is attached.
>
> Wrt two of your questions:
>
>
>
> (1)  “The description that you have on CheckShadowInheritedVariables
> isn't really the type of comments that we have in doxygen form.  Im not
> sure if its in line with the rest of the code.”
>
> I’ve read through the doxygen wiki; hopefully it’s fixed; let me know if
> it’s still wrong
>
>
>
> (2) “Why are you checking that the DeclContext has a definition rather
> than the record itself?”
>
> There are cases like “struct A; struct B : A {};”, where A does not have a
> definition. The compiler will hit an assertion failure if we call A.bases()
> directly.
>
>
>
> Thanks
>
>
>
> James
>
>
>
>
>
> *From: *Saleem Abdulrasool 
> *Date: *Tuesday, January 24, 2017 at 7:10 PM
> *To: *James Sun 
> *Cc: *"cfe-commits@lists.llvm.org" , Aaron
> Ballman , Richard Smith 
> *Subject: *Re: Add warning for c++ member variable shadowing
>
>
>
> Some more stylistic comments:
>
>
>
> The description that you have on CheckShadowInheritedVariables isn't
> really the 

[PATCH] D29350: clang-cl: Evaluate arguments left-to-right in constructor call with initializer list (PR31831)

[Hans Wennborg via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL293732: clang-cl: Evaluate arguments left-to-right in 
constructor call with initializer… (authored by hans).

Changed prior to commit:
  https://reviews.llvm.org/D29350?vs=86496=86549#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D29350

Files:
  cfe/trunk/lib/CodeGen/CGClass.cpp
  cfe/trunk/test/CodeGenCXX/initializer-list-ctor-order.cpp


Index: cfe/trunk/lib/CodeGen/CGClass.cpp
===
--- cfe/trunk/lib/CodeGen/CGClass.cpp
+++ cfe/trunk/lib/CodeGen/CGClass.cpp
@@ -1950,7 +1950,11 @@
 
   // Add the rest of the user-supplied arguments.
   const FunctionProtoType *FPT = D->getType()->castAs();
-  EmitCallArgs(Args, FPT, E->arguments(), E->getConstructor());
+  EvaluationOrder Order = E->isListInitialization()
+  ? EvaluationOrder::ForceLeftToRight
+  : EvaluationOrder::Default;
+  EmitCallArgs(Args, FPT, E->arguments(), E->getConstructor(),
+   /*ParamsToSkip*/ 0, Order);
 
   EmitCXXConstructorCall(D, Type, ForVirtualBase, Delegating, This, Args);
 }
Index: cfe/trunk/test/CodeGenCXX/initializer-list-ctor-order.cpp
===
--- cfe/trunk/test/CodeGenCXX/initializer-list-ctor-order.cpp
+++ cfe/trunk/test/CodeGenCXX/initializer-list-ctor-order.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -std=c++11 %s -emit-llvm -o - -triple %itanium_abi_triple | 
FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-ITANIUM
+// RUN: %clang_cc1 -std=c++11 %s -emit-llvm -o - -triple i686-windows| 
FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-MS
+
+extern "C" {
+int f();
+int g();
+}
+
+struct A {
+  A(int, int);
+};
+
+
+void foo() {
+  A a{f(), g()};
+}
+// CHECK-ITANIUM-LABEL: define void @_Z3foov
+// CHECK-MS-LABEL: define void @"\01?foo@@YAXXZ"
+// CHECK: call i32 @f()
+// CHECK: call i32 @g()
+
+struct B : A {
+  B();
+};
+B::B() : A{f(), g()} {}
+// CHECK-ITANIUM-LABEL: define void @_ZN1BC2Ev
+// CHECK-MS-LABEL: define x86_thiscallcc %struct.B* @"\01??0B@@QAE@XZ"
+// CHECK: call i32 @f()
+// CHECK: call i32 @g()


Index: cfe/trunk/lib/CodeGen/CGClass.cpp
===
--- cfe/trunk/lib/CodeGen/CGClass.cpp
+++ cfe/trunk/lib/CodeGen/CGClass.cpp
@@ -1950,7 +1950,11 @@
 
   // Add the rest of the user-supplied arguments.
   const FunctionProtoType *FPT = D->getType()->castAs();
-  EmitCallArgs(Args, FPT, E->arguments(), E->getConstructor());
+  EvaluationOrder Order = E->isListInitialization()
+  ? EvaluationOrder::ForceLeftToRight
+  : EvaluationOrder::Default;
+  EmitCallArgs(Args, FPT, E->arguments(), E->getConstructor(),
+   /*ParamsToSkip*/ 0, Order);
 
   EmitCXXConstructorCall(D, Type, ForVirtualBase, Delegating, This, Args);
 }
Index: cfe/trunk/test/CodeGenCXX/initializer-list-ctor-order.cpp
===
--- cfe/trunk/test/CodeGenCXX/initializer-list-ctor-order.cpp
+++ cfe/trunk/test/CodeGenCXX/initializer-list-ctor-order.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -std=c++11 %s -emit-llvm -o - -triple %itanium_abi_triple | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-ITANIUM
+// RUN: %clang_cc1 -std=c++11 %s -emit-llvm -o - -triple i686-windows| FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-MS
+
+extern "C" {
+int f();
+int g();
+}
+
+struct A {
+  A(int, int);
+};
+
+
+void foo() {
+  A a{f(), g()};
+}
+// CHECK-ITANIUM-LABEL: define void @_Z3foov
+// CHECK-MS-LABEL: define void @"\01?foo@@YAXXZ"
+// CHECK: call i32 @f()
+// CHECK: call i32 @g()
+
+struct B : A {
+  B();
+};
+B::B() : A{f(), g()} {}
+// CHECK-ITANIUM-LABEL: define void @_ZN1BC2Ev
+// CHECK-MS-LABEL: define x86_thiscallcc %struct.B* @"\01??0B@@QAE@XZ"
+// CHECK: call i32 @f()
+// CHECK: call i32 @g()
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r293732 - clang-cl: Evaluate arguments left-to-right in constructor call with initializer list (PR31831)

[Hans Wennborg via cfe-commits]

Author: hans
Date: Tue Jan 31 20:21:07 2017
New Revision: 293732

URL: http://llvm.org/viewvc/llvm-project?rev=293732=rev
Log:
clang-cl: Evaluate arguments left-to-right in constructor call with initializer 
list (PR31831)

clang-cl would evaluate the arguments right-to-left (see PR), and for
non-Windows targets I suppose we only got it because we were already
emitting left-to-right in CodeGenFunction::EmitCallArgs.

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

Added:
cfe/trunk/test/CodeGenCXX/initializer-list-ctor-order.cpp
Modified:
cfe/trunk/lib/CodeGen/CGClass.cpp

Modified: cfe/trunk/lib/CodeGen/CGClass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGClass.cpp?rev=293732=293731=293732=diff
==
--- cfe/trunk/lib/CodeGen/CGClass.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGClass.cpp Tue Jan 31 20:21:07 2017
@@ -1950,7 +1950,11 @@ void CodeGenFunction::EmitCXXConstructor
 
   // Add the rest of the user-supplied arguments.
   const FunctionProtoType *FPT = D->getType()->castAs();
-  EmitCallArgs(Args, FPT, E->arguments(), E->getConstructor());
+  EvaluationOrder Order = E->isListInitialization()
+  ? EvaluationOrder::ForceLeftToRight
+  : EvaluationOrder::Default;
+  EmitCallArgs(Args, FPT, E->arguments(), E->getConstructor(),
+   /*ParamsToSkip*/ 0, Order);
 
   EmitCXXConstructorCall(D, Type, ForVirtualBase, Delegating, This, Args);
 }

Added: cfe/trunk/test/CodeGenCXX/initializer-list-ctor-order.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/initializer-list-ctor-order.cpp?rev=293732=auto
==
--- cfe/trunk/test/CodeGenCXX/initializer-list-ctor-order.cpp (added)
+++ cfe/trunk/test/CodeGenCXX/initializer-list-ctor-order.cpp Tue Jan 31 
20:21:07 2017
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -std=c++11 %s -emit-llvm -o - -triple %itanium_abi_triple | 
FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-ITANIUM
+// RUN: %clang_cc1 -std=c++11 %s -emit-llvm -o - -triple i686-windows| 
FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-MS
+
+extern "C" {
+int f();
+int g();
+}
+
+struct A {
+  A(int, int);
+};
+
+
+void foo() {
+  A a{f(), g()};
+}
+// CHECK-ITANIUM-LABEL: define void @_Z3foov
+// CHECK-MS-LABEL: define void @"\01?foo@@YAXXZ"
+// CHECK: call i32 @f()
+// CHECK: call i32 @g()
+
+struct B : A {
+  B();
+};
+B::B() : A{f(), g()} {}
+// CHECK-ITANIUM-LABEL: define void @_ZN1BC2Ev
+// CHECK-MS-LABEL: define x86_thiscallcc %struct.B* @"\01??0B@@QAE@XZ"
+// CHECK: call i32 @f()
+// CHECK: call i32 @g()


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

Re: r291963 - [clang] Emit `diagnose_if` warnings from system headers

[Hans Wennborg via cfe-commits]

I'm Ok with taking the larger patch (r293360 + r293369) too. It's been
in tree for a bit, there is still a number of weeks before the
release, and IIUC the major risk is that diagnose_if itself turns out
to be broken, not that we'd miscompile anything?

On Tue, Jan 31, 2017 at 11:11 AM, Richard Smith  wrote:
> Yes, this makes sense. We should also decide what we're going to do about
> the larger diagnose_if patch that George has asked to be ported to Clang 4.
> Are you comfortable taking a patch of that size? If not, perhaps we should
> disable the attribute for the Clang 4 release instead.
>
>
> On 31 January 2017 at 10:36, Hans Wennborg  wrote:
>>
>> Ping?
>>
>> On Thu, Jan 26, 2017 at 10:21 AM, Hans Wennborg  wrote:
>> > Ping?
>> >
>> > On Mon, Jan 23, 2017 at 4:27 PM, Hans Wennborg 
>> > wrote:
>> >> Ping?
>> >>
>> >> On Tue, Jan 17, 2017 at 4:16 PM, Hans Wennborg 
>> >> wrote:
>> >>> Richard, what do you think?
>> >>>
>> >>> On Fri, Jan 13, 2017 at 3:16 PM, Eric Fiselier  wrote:
>>  I would love to see this merged. It would make it easier to write
>>  libc++
>>  tests if the tests didn't have to worry about the old 4.0 behavior.
>> 
>>  CC'ing Richard: Would merging this be OK?
>> 
>>  On Fri, Jan 13, 2017 at 3:46 PM, George Burgess IV
>>   wrote:
>> >
>> > Do we want to consider merging this into the release branch? Seems
>> > like
>> > more of a bugfix than a feature to me.
>> >
>> > On Fri, Jan 13, 2017 at 2:11 PM, Eric Fiselier via cfe-commits
>> >  wrote:
>> >>
>> >> Author: ericwf
>> >> Date: Fri Jan 13 16:11:40 2017
>> >> New Revision: 291963
>> >>
>> >> URL: http://llvm.org/viewvc/llvm-project?rev=291963=rev
>> >> Log:
>> >> [clang] Emit `diagnose_if` warnings from system headers
>> >>
>> >> Summary: In order for libc++ to meaningfully use `diagnose_if`
>> >> warnings
>> >> they need to be emitted from system headers by default. This patch
>> >> changes
>> >> the `diagnose_if` warning diagnostic to be shown in system headers.
>> >>
>> >> Reviewers: george.burgess.iv, rsmith, aaron.ballman
>> >>
>> >> Subscribers: cfe-commits
>> >>
>> >> Differential Revision: https://reviews.llvm.org/D28703
>> >>
>> >> Added:
>> >> cfe/trunk/test/Sema/Inputs/diagnose-if-warn-system-header.h
>> >> Modified:
>> >> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> >> cfe/trunk/test/Sema/diagnose_if.c
>> >>
>> >> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> >> URL:
>> >>
>> >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=291963=291962=291963=diff
>> >>
>> >>
>> >> ==
>> >> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
>> >> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Jan 13
>> >> 16:11:40 2017
>> >> @@ -3380,7 +3380,8 @@ def note_ovl_candidate_has_pass_object_s
>> >>  "candidate address cannot be taken because parameter %0 has "
>> >>  "pass_object_size attribute">;
>> >>  def err_diagnose_if_succeeded : Error<"%0">;
>> >> -def warn_diagnose_if_succeeded : Warning<"%0">,
>> >> InGroup;
>> >> +def warn_diagnose_if_succeeded : Warning<"%0">,
>> >> InGroup,
>> >> +ShowInSystemHeader;
>> >>  def note_ovl_candidate_disabled_by_function_cond_attr : Note<
>> >>  "candidate disabled: %0">;
>> >>  def note_ovl_candidate_disabled_by_extension : Note<
>> >>
>> >> Added: cfe/trunk/test/Sema/Inputs/diagnose-if-warn-system-header.h
>> >> URL:
>> >>
>> >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/Inputs/diagnose-if-warn-system-header.h?rev=291963=auto
>> >>
>> >>
>> >> ==
>> >> --- cfe/trunk/test/Sema/Inputs/diagnose-if-warn-system-header.h
>> >> (added)
>> >> +++ cfe/trunk/test/Sema/Inputs/diagnose-if-warn-system-header.h Fri
>> >> Jan
>> >> 13 16:11:40 2017
>> >> @@ -0,0 +1,11 @@
>> >> +#pragma GCC system_header
>> >> +
>> >> +inline int system_header_func(int x)
>> >> +  __attribute__((diagnose_if(x == x, "system header warning",
>> >> "warning"))) // expected-note {{from 'diagnose_if' attribute}}
>> >> +{
>> >> +  return 0;
>> >> +}
>> >> +
>> >> +void test_system_header() {
>> >> +  system_header_func(0); // expected-warning {{system header
>> >> warning}}
>> >> +}
>> >>
>> >> Modified: cfe/trunk/test/Sema/diagnose_if.c
>> >> URL:
>> >>
>> >> 

[PATCH] D29350: clang-cl: Evaluate arguments left-to-right in constructor call with initializer list (PR31831)

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

In https://reviews.llvm.org/D29350#662525, @rnk wrote:

> Can you check if we got this wrong in clang 3.8 and 3.9? I'm wondering if 
> this was a regression, or if we always got this wrong.


I tried back to 3.5, and it seems we just always got it wrong.


https://reviews.llvm.org/D29350



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

[PATCH] D29350: clang-cl: Evaluate arguments left-to-right in constructor call with initializer list (PR31831)

[Reid Kleckner via Phabricator via cfe-commits]

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

lgtm

Can you check if we got this wrong in clang 3.8 and 3.9? I'm wondering if this 
was a regression, or if we always got this wrong.


https://reviews.llvm.org/D29350



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

[PATCH] D29151: [clang-tidy] Add misc-invalidated-iterators check.

[Anna Zaks via Phabricator via cfe-commits]

zaks.anna added a comment.

Before clang-tidy came into existence the guidelines were very clear. One 
should write a clang warning if the diagnostic:

- can be implemented without path-insensitive analysis (including 
flow-sensitive)
- is checking for language rules (not specific API misuse)

In my view, this should still be the rule going forward because compiler 
warnings are most effective in reaching users.

The Clang Static Analyzer used to be the place for all other diagnostics. Most 
of the checkers it contains rely on path-sensitive analysis. Note that one 
might catch the same bug with flow-sensitive as well as path-sensitive 
analysis. So in some of those cases, we have both warnings as well as analyzer 
checkers finding the same type of user error. However, the checkers can catch 
more bugs since they are path-sensitive. The analyzer also has several 
flow-sensitive/ AST matching checkers. Those checks could not have been written 
as warnings mainly because they check for APIs, which are not part of the 
language.

My understanding is that clang-tidy supports fixits, which the clang static 
analyzer currently does not support. However, there could be other benefits to 
placing not path-sensitive checks there as well. I am not sure. I've also heard 
opinions that it's more of a linter tool, so the user expectations could be 
different.

> Even right now there are clang-tidy checks that finds subset of alpha checks, 
> but probably having lower false positive rate.

Again, alpha checks are unfinished work, so we should not use them as examples 
of checkers that have false positives. Some of them are research projects and 
should probably be deleted.


Repository:
  rL LLVM

https://reviews.llvm.org/D29151



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

[PATCH] D29358: [OpenMP] Report an error for -faltivec on anything other than PowerPC.

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea created this revision.
Herald added a subscriber: nemanjai.

When performing an offload, throw an error if the host tool chain uses the 
altivec flag on an architecture other than PowerPC.


Repository:
  rL LLVM

https://reviews.llvm.org/D29358

Files:
  lib/Driver/Tools.cpp
  test/OpenMP/offloading_defects.cpp


Index: test/OpenMP/offloading_defects.cpp
===
--- /dev/null
+++ test/OpenMP/offloading_defects.cpp
@@ -0,0 +1,73 @@
+// 
#
+// RUN:   %clangxx -DCK1 -target powerpc64le-ibm-linux-gnu -mcpu=pwr8 
-maltivec -O0 -S -emit-llvm %s -o - \
+// RUN:   | FileCheck --check-prefix CK1 %s
+#ifdef CK1
+
+static __inline__ vector float __attribute__((__overloadable__, 
__always_inline__)) vec_ld(int __a,
+   const vector float *__b) {
+  return (vector float)__builtin_altivec_lvx(__a, __b);
+}
+
+static __inline__ vector float __attribute__((__overloadable__, 
__always_inline__)) vec_ld(int __a, const float *__b) {
+  return (vector float)__builtin_altivec_lvx(__a, __b);
+}
+
+static __inline__ vector double __attribute__((__overloadable__, 
__always_inline__)) __builtin_vec_splats(double __a) {
+  return (vector double)(__a);
+}
+
+static __inline__ vector double __attribute__((__overloadable__, 
__always_inline__)) vec_abs(vector double __a) {
+  return __builtin_vsx_xvabsdp(__a);
+}
+
+static __inline__ vector bool long long __attribute__((__overloadable__, 
__always_inline__))
+vec_cmpgt(vector double __a, vector double __b) {
+  return (vector bool long long)__builtin_vsx_xvcmpgtdp(__a, __b);
+}
+
+static __inline__ vector double __attribute__((__overloadable__, 
__always_inline__))
+vec_sel(vector double __a, vector double __b, vector bool long long __c) {
+  vector long long __res = ((vector long long)__a & ~(vector long long)__c) |
+   ((vector long long)__b & (vector long long)__c);
+  return (vector double)__res;
+}
+
+static __inline__ vector double __attribute__((__overloadable__, 
__always_inline__))
+vec_sel(vector double __a, vector double __b, vector unsigned long long __c) {
+  vector long long __res = ((vector long long)__a & ~(vector long long)__c) |
+   ((vector long long)__b & (vector long long)__c);
+  return (vector double)__res;
+}
+
+static double x[8];
+
+static inline __vector double vec_ld_dbl(int offset, double const *x) {
+
+return (__vector double)vec_ld(offset,
+  reinterpret_cast(const_cast(x)));
+}
+
+// CK1: declare <4 x i32> @llvm.ppc.altivec.lvx(i8*)
+// CK1: declare <2 x i64> @llvm.ppc.vsx.xvcmpgtdp(<2 x double>, <2 x double>)
+// CK1: declare <2 x double> @llvm.fabs.v2f64(<2 x double>)
+
+int main(int argc, char **argv)
+{
+  double A[1] = {-1.0};
+
+  // FIXME: Test with OpenMP offloading support too.
+  #pragma omp target
+  {
+A[0] += 1.0;
+  }
+
+  __vector double vsum = __builtin_vec_splats(A[0]);
+  __vector double verror = __builtin_vec_splats((double) 0.0);
+  __vector double v = vec_ld_dbl(0, x);
+  __vector bool long long sel = vec_cmpgt( vec_abs(v), vec_abs(vsum) );
+  __vector double a = vec_sel(vsum, v, sel);
+
+
+  return 0;
+}
+#endif
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -5432,7 +5432,10 @@
 
   // Report an error for -faltivec on anything other than PowerPC.
   if (const Arg *A = Args.getLastArg(options::OPT_faltivec)) {
-const llvm::Triple::ArchType Arch = getToolChain().getArch();
+const llvm::Triple::ArchType Arch =
+JA.isDeviceOffloading(Action::OFK_None)
+? getToolChain().getArch()
+: C.getSingleOffloadToolChain()->getArch();
 if (!(Arch == llvm::Triple::ppc || Arch == llvm::Triple::ppc64 ||
   Arch == llvm::Triple::ppc64le))
   D.Diag(diag::err_drv_argument_only_allowed_with) << A->getAsString(Args)


Index: test/OpenMP/offloading_defects.cpp
===
--- /dev/null
+++ test/OpenMP/offloading_defects.cpp
@@ -0,0 +1,73 @@
+// #
+// RUN:   %clangxx -DCK1 -target powerpc64le-ibm-linux-gnu -mcpu=pwr8 -maltivec -O0 -S -emit-llvm %s -o - \
+// RUN:   | FileCheck --check-prefix CK1 %s
+#ifdef CK1
+
+static __inline__ vector float __attribute__((__overloadable__, __always_inline__)) vec_ld(int __a,
+   const vector float *__b) {
+  return (vector float)__builtin_altivec_lvx(__a, __b);
+}
+
+static __inline__ vector float __attribute__((__overloadable__, __always_inline__)) vec_ld(int __a, const float *__b) {
+  return (vector float)__builtin_altivec_lvx(__a, __b);
+}
+
+static __inline__ vector double __attribute__((__overloadable__, 

[PATCH] D29151: [clang-tidy] Add misc-invalidated-iterators check.

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

> I think we need some sort of clear guidelines on where what functionality 
> should be added. Even right now there are clang-tidy checks that finds subset 
> of alpha checks, but probably having lower false positive rate. The other 
> example is Use after move, that is doing similar thing as uninitialized 
> values analysis in clang.

I agree with this and could add that we also need similar guidelines for Clang 
diagnostics too. Some of Clang-tidy and Static Analyzer checks are more suited 
for Clang diagnostics (unused constructs, deadcode.DeadStores, etc). It will be 
reasonable if we have discussions to decide better place for particular check 
instead of just swallowing patches.


Repository:
  rL LLVM

https://reviews.llvm.org/D29151



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

[PATCH] D29151: [clang-tidy] Add misc-invalidated-iterators check.

[Piotr Padlewski via Phabricator via cfe-commits]

Prazek added a comment.

In https://reviews.llvm.org/D29151#658484, @zaks.anna wrote:

> The static analyzer is definitely the place to go for bug detection that 
> requires path sensitivity. It's also reasonably good for anything that needs 
> flow-sensitivity. Although, most flow-sensitive analysis (such as liveness) 
> now live in lib/Analysis/, which is used by both Sema and the Clang Static 
> Analyzer. Both path-sensitive and flow-sensitive analysis are based on 
> clang's CFG. I do not know of clang-tidy uses CFG or lib/Analysis/.
>
> Here are the wikipedia definitions of sensitivity I am referring to:
>  //- A flow-sensitive analysis takes into account the order of statements in 
> a program. For example, a flow-insensitive pointer alias analysis may 
> determine "variables x and y may refer to the same location", while a 
> flow-sensitive analysis may determine "after statement 20, variables x and y 
> may refer to the same location".
>
> - A path-sensitive analysis computes different pieces of analysis information 
> dependent on the predicates at conditional branch instructions. For instance, 
> if a branch contains a condition x>0, then on the fall-through path, the 
> analysis would assume that x<=0 and on the target of the branch it would 
> assume that indeed x>0 holds. //
>
>   There is work underway in the analyzer for adding IteratorPastEnd checker. 
> The first commit was rather large and has been reviewed here 
> https://reviews.llvm.org/D25660. That checker is very close to be moved out 
> of alpha. Moving it out of alpha is pending evaluation on real codebases to 
> ensure that the false positive rates are low and enhancements to error 
> reporting.
>
> > Other problem is that it would be probably a part 
> >  of one of the alpha checks, that are not developed and I don't know if 
> > they will ever be fully supported.
>
> There seems to be a misconception about what the alpha checkers are. All 
> checkers start in alpha package to allow in-tree incremental development. 
> Once the checkers are complete, they should move out of alpha. The criteria 
> is that the code should pass the review, the checker needs to have low false 
> positive rates, finally, the error reports should be of good quality (some 
> checks greatly benefit from extra path hints that can be implemented with a 
> visitor). These are motivated by providing a good user experience to end 
> users. (We do have several checkers that are "stuck in alpha". A lot of them 
> are abandoned experiments that just need to be deleted; others could probably 
> be made production quality with some extra effort.)


I think we need some sort of clear guidelines on where what functionality 
should be added. Even right now there are clang-tidy checks that finds subset 
of alpha checks, but probably having lower false positive rate. The other 
example is Use after move, that is doing similar thing as uninitialized values 
analysis in clang.


Repository:
  rL LLVM

https://reviews.llvm.org/D29151



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

[PATCH] D28365: [Driver] Updated for Visual Studio 2017

[Hamza Sood via Phabricator via cfe-commits]

hamzasood updated this revision to Diff 86502.
hamzasood added a comment.

Added a FIXME comment about configuring use of the Setup Configuration API.
In its current form (RC3) the header is distributed in a nuget package, so it's 
installed per-project instead of being in a system location that can be 
automatically detected. This could change for the final release, so it probably 
isn't worth thinking about it too much yet.


https://reviews.llvm.org/D28365

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  lib/Driver/MSVCToolChain.cpp
  lib/Driver/ToolChains.h
  lib/Driver/Tools.cpp

Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -10888,19 +10888,12 @@
 // making sure that whatever executable that's found is not a same-named exe
 // from clang itself to prevent clang from falling back to itself.
 static std::string FindVisualStudioExecutable(const ToolChain ,
-  const char *Exe,
-  const char *ClangProgramPath) {
+  const char *Exe) {
   const auto  = static_cast(TC);
-  std::string visualStudioBinDir;
-  if (MSVC.getVisualStudioBinariesFolder(ClangProgramPath,
- visualStudioBinDir)) {
-SmallString<128> FilePath(visualStudioBinDir);
-llvm::sys::path::append(FilePath, Exe);
-if (llvm::sys::fs::can_execute(FilePath.c_str()))
-  return FilePath.str();
-  }
-
-  return Exe;
+  SmallString<128> FilePath(MSVC.getSubDirectoryPath(toolchains::MSVCToolChain
+ ::SubDirectoryType::Bin));
+  llvm::sys::path::append(FilePath, Exe);
+  return (llvm::sys::fs::can_execute(FilePath) ? FilePath.str() : Exe);
 }
 
 void visualstudio::Linker::ConstructJob(Compilation , const JobAction ,
@@ -10909,7 +10902,7 @@
 const ArgList ,
 const char *LinkingOutput) const {
   ArgStringList CmdArgs;
-  const ToolChain  = getToolChain();
+  auto  = static_cast(getToolChain());
 
   assert((Output.isFilename() || Output.isNothing()) && "invalid output");
   if (Output.isFilename())
@@ -10925,37 +10918,20 @@
 // did not run vcvarsall), try to build a consistent link environment.  If
 // the environment variable is set however, assume the user knows what
 // they're doing.
-std::string VisualStudioDir;
-const auto  = static_cast(TC);
-if (MSVC.getVisualStudioInstallDir(VisualStudioDir)) {
-  SmallString<128> LibDir(VisualStudioDir);
-  llvm::sys::path::append(LibDir, "VC", "lib");
-  switch (MSVC.getArch()) {
-  case llvm::Triple::x86:
-// x86 just puts the libraries directly in lib
-break;
-  case llvm::Triple::x86_64:
-llvm::sys::path::append(LibDir, "amd64");
-break;
-  case llvm::Triple::arm:
-llvm::sys::path::append(LibDir, "arm");
-break;
-  default:
-break;
-  }
-  CmdArgs.push_back(
-  Args.MakeArgString(std::string("-libpath:") + LibDir.c_str()));
+CmdArgs.push_back(Args.MakeArgString(
+  std::string("-libpath:")
+  + TC.getSubDirectoryPath(toolchains::MSVCToolChain
+   ::SubDirectoryType::Lib)));
 
-  if (MSVC.useUniversalCRT(VisualStudioDir)) {
-std::string UniversalCRTLibPath;
-if (MSVC.getUniversalCRTLibraryPath(UniversalCRTLibPath))
-  CmdArgs.push_back(Args.MakeArgString(std::string("-libpath:") +
-   UniversalCRTLibPath));
-  }
+if (TC.useUniversalCRT()) {
+  std::string UniversalCRTLibPath;
+  if (TC.getUniversalCRTLibraryPath(UniversalCRTLibPath))
+CmdArgs.push_back(Args.MakeArgString(std::string("-libpath:")
+ + UniversalCRTLibPath));
 }
 
 std::string WindowsSdkLibPath;
-if (MSVC.getWindowsSDKLibraryPath(WindowsSdkLibPath))
+if (TC.getWindowsSDKLibraryPath(WindowsSdkLibPath))
   CmdArgs.push_back(
   Args.MakeArgString(std::string("-libpath:") + WindowsSdkLibPath));
   }
@@ -11079,8 +11055,7 @@
 // If we're using the MSVC linker, it's not sufficient to just use link
 // from the program PATH, because other environments like GnuWin32 install
 // their own link.exe which may come first.
-linkPath = FindVisualStudioExecutable(TC, "link.exe",
-  C.getDriver().getClangProgramPath());
+linkPath = FindVisualStudioExecutable(TC, "link.exe");
   } else {
 linkPath = Linker;
 llvm::sys::path::replace_extension(linkPath, "exe");
@@ -11213,9 +11188,7 @@
   Args.MakeArgString(std::string("/Fo") + Output.getFilename());
   CmdArgs.push_back(Fo);
 
-  const 

Re: [PATCH] D24969: [Sema] Use the instantiated name of destructors in FindInstantiatedDecl and RebuildMemberExpr

[Richard Smith via cfe-commits]

On 31 January 2017 at 14:49, Hans Wennborg  wrote:

> Richard, what do you think? I don't believe this fixes a 3.9
> regression, but the fix looks small, so it might be worth it.
>

I think on the whole the benefit of this change outweighs the risk of it
regressing something.


> On Tue, Jan 31, 2017 at 12:07 PM, Akira Hatanaka 
> wrote:
> > Thanks for the review. r293678.
> >
> > Should this be merged to 4.0?
> >
> >> On Jan 31, 2017, at 12:04 PM, Akira Hatanaka via Phabricator via
> cfe-commits  wrote:
> >>
> >> This revision was automatically updated to reflect the committed
> changes.
> >> Closed by commit rL293678: [Sema] Transform a templated name before
> looking it up in (authored by ahatanak).
> >>
> >> Changed prior to commit:
> >>  https://reviews.llvm.org/D24969?vs=82134=86474#toc
> >>
> >> Repository:
> >>  rL LLVM
> >>
> >> https://reviews.llvm.org/D24969
> >>
> >> Files:
> >>  cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
> >>  cfe/trunk/lib/Sema/TreeTransform.h
> >>  cfe/trunk/test/SemaCXX/destructor.cpp
> >>
> >>
> >> Index: cfe/trunk/test/SemaCXX/destructor.cpp
> >> ===
> >> --- cfe/trunk/test/SemaCXX/destructor.cpp
> >> +++ cfe/trunk/test/SemaCXX/destructor.cpp
> >> @@ -431,3 +431,23 @@
> >>
> >> // The constructor definition should not have errors
> >> Invalid::~Invalid() {}
> >> +
> >> +namespace PR30361 {
> >> +template 
> >> +struct C1 {
> >> +  ~C1() {}
> >> +  operator C1* () { return nullptr; }
> >> +  void foo1();
> >> +};
> >> +
> >> +template
> >> +void C1::foo1() {
> >> +  C1::operator C1*();
> >> +  C1::~C1();
> >> +}
> >> +
> >> +void foo1() {
> >> +  C1 x;
> >> +  x.foo1();
> >> +}
> >> +}
> >> Index: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
> >> ===
> >> --- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
> >> +++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
> >> @@ -4990,8 +4990,12 @@
> >> NamedDecl *Result = nullptr;
> >> // FIXME: If the name is a dependent name, this lookup won't
> necessarily
> >> // find it. Does that ever matter?
> >> -if (D->getDeclName()) {
> >> -  DeclContext::lookup_result Found = ParentDC->lookup(D->
> getDeclName());
> >> +if (auto Name = D->getDeclName()) {
> >> +  DeclarationNameInfo NameInfo(Name, D->getLocation());
> >> +  Name = SubstDeclarationNameInfo(NameInfo,
> TemplateArgs).getName();
> >> +  if (!Name)
> >> +return nullptr;
> >> +  DeclContext::lookup_result Found = ParentDC->lookup(Name);
> >>   Result = findInstantiationOf(Context, D, Found.begin(),
> Found.end());
> >> } else {
> >>   // Since we don't have a name for the entity we're looking for,
> >> Index: cfe/trunk/lib/Sema/TreeTransform.h
> >> ===
> >> --- cfe/trunk/lib/Sema/TreeTransform.h
> >> +++ cfe/trunk/lib/Sema/TreeTransform.h
> >> @@ -8966,12 +8966,18 @@
> >>   // base (and therefore couldn't do the check) and a
> >>   // nested-name-qualifier (and therefore could do the lookup).
> >>   NamedDecl *FirstQualifierInScope = nullptr;
> >> +  DeclarationNameInfo MemberNameInfo = E->getMemberNameInfo();
> >> +  if (MemberNameInfo.getName()) {
> >> +MemberNameInfo = getDerived().TransformDeclarationNameInfo(
> MemberNameInfo);
> >> +if (!MemberNameInfo.getName())
> >> +  return ExprError();
> >> +  }
> >>
> >>   return getDerived().RebuildMemberExpr(Base.get(), FakeOperatorLoc,
> >> E->isArrow(),
> >> QualifierLoc,
> >> TemplateKWLoc,
> >> -E->getMemberNameInfo(),
> >> +MemberNameInfo,
> >> Member,
> >> FoundDecl,
> >> (E->hasExplicitTemplateArgs()
> >>
> >>
> >> ___
> >> 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] D24969: [Sema] Use the instantiated name of destructors in FindInstantiatedDecl and RebuildMemberExpr

[Hans Wennborg via cfe-commits]

Richard, what do you think? I don't believe this fixes a 3.9
regression, but the fix looks small, so it might be worth it.

On Tue, Jan 31, 2017 at 12:07 PM, Akira Hatanaka  wrote:
> Thanks for the review. r293678.
>
> Should this be merged to 4.0?
>
>> On Jan 31, 2017, at 12:04 PM, Akira Hatanaka via Phabricator via cfe-commits 
>>  wrote:
>>
>> This revision was automatically updated to reflect the committed changes.
>> Closed by commit rL293678: [Sema] Transform a templated name before looking 
>> it up in (authored by ahatanak).
>>
>> Changed prior to commit:
>>  https://reviews.llvm.org/D24969?vs=82134=86474#toc
>>
>> Repository:
>>  rL LLVM
>>
>> https://reviews.llvm.org/D24969
>>
>> Files:
>>  cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
>>  cfe/trunk/lib/Sema/TreeTransform.h
>>  cfe/trunk/test/SemaCXX/destructor.cpp
>>
>>
>> Index: cfe/trunk/test/SemaCXX/destructor.cpp
>> ===
>> --- cfe/trunk/test/SemaCXX/destructor.cpp
>> +++ cfe/trunk/test/SemaCXX/destructor.cpp
>> @@ -431,3 +431,23 @@
>>
>> // The constructor definition should not have errors
>> Invalid::~Invalid() {}
>> +
>> +namespace PR30361 {
>> +template 
>> +struct C1 {
>> +  ~C1() {}
>> +  operator C1* () { return nullptr; }
>> +  void foo1();
>> +};
>> +
>> +template
>> +void C1::foo1() {
>> +  C1::operator C1*();
>> +  C1::~C1();
>> +}
>> +
>> +void foo1() {
>> +  C1 x;
>> +  x.foo1();
>> +}
>> +}
>> Index: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
>> ===
>> --- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
>> +++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
>> @@ -4990,8 +4990,12 @@
>> NamedDecl *Result = nullptr;
>> // FIXME: If the name is a dependent name, this lookup won't necessarily
>> // find it. Does that ever matter?
>> -if (D->getDeclName()) {
>> -  DeclContext::lookup_result Found = ParentDC->lookup(D->getDeclName());
>> +if (auto Name = D->getDeclName()) {
>> +  DeclarationNameInfo NameInfo(Name, D->getLocation());
>> +  Name = SubstDeclarationNameInfo(NameInfo, TemplateArgs).getName();
>> +  if (!Name)
>> +return nullptr;
>> +  DeclContext::lookup_result Found = ParentDC->lookup(Name);
>>   Result = findInstantiationOf(Context, D, Found.begin(), Found.end());
>> } else {
>>   // Since we don't have a name for the entity we're looking for,
>> Index: cfe/trunk/lib/Sema/TreeTransform.h
>> ===
>> --- cfe/trunk/lib/Sema/TreeTransform.h
>> +++ cfe/trunk/lib/Sema/TreeTransform.h
>> @@ -8966,12 +8966,18 @@
>>   // base (and therefore couldn't do the check) and a
>>   // nested-name-qualifier (and therefore could do the lookup).
>>   NamedDecl *FirstQualifierInScope = nullptr;
>> +  DeclarationNameInfo MemberNameInfo = E->getMemberNameInfo();
>> +  if (MemberNameInfo.getName()) {
>> +MemberNameInfo = 
>> getDerived().TransformDeclarationNameInfo(MemberNameInfo);
>> +if (!MemberNameInfo.getName())
>> +  return ExprError();
>> +  }
>>
>>   return getDerived().RebuildMemberExpr(Base.get(), FakeOperatorLoc,
>> E->isArrow(),
>> QualifierLoc,
>> TemplateKWLoc,
>> -E->getMemberNameInfo(),
>> +MemberNameInfo,
>> Member,
>> FoundDecl,
>> (E->hasExplicitTemplateArgs()
>>
>>
>> ___
>> 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: r292194 - [AST] AttributedType should derive type properties from the EquivalentType

[Hans Wennborg via cfe-commits]

r293702.

On Tue, Jan 31, 2017 at 11:12 AM, Richard Smith  wrote:
> Sure, let's take this for Clang 4.
>
> On 31 January 2017 at 10:39, Hans Wennborg  wrote:
>>
>> Ping?
>>
>> On Thu, Jan 26, 2017 at 3:42 PM, Hans Wennborg  wrote:
>> > Should we merge this to the release branch?
>> >
>> > On Mon, Jan 16, 2017 at 8:14 PM, David Majnemer via cfe-commits
>> >  wrote:
>> >> Author: majnemer
>> >> Date: Mon Jan 16 22:14:25 2017
>> >> New Revision: 292194
>> >>
>> >> URL: http://llvm.org/viewvc/llvm-project?rev=292194=rev
>> >> Log:
>> >> [AST] AttributedType should derive type properties from the
>> >> EquivalentType
>> >>
>> >> Using the canonical type instead of the equivalent type can result in
>> >> insufficient template instantiations.
>> >>
>> >> This fixes PR31656.
>> >>
>> >> Differential Revision: https://reviews.llvm.org/D28788
>> >>
>> >> Modified:
>> >> cfe/trunk/include/clang/AST/Type.h
>> >> cfe/trunk/test/CodeGenCXX/microsoft-abi-default-cc.cpp
>> >>
>> >> Modified: cfe/trunk/include/clang/AST/Type.h
>> >> URL:
>> >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Type.h?rev=292194=292193=292194=diff
>> >>
>> >> ==
>> >> --- cfe/trunk/include/clang/AST/Type.h (original)
>> >> +++ cfe/trunk/include/clang/AST/Type.h Mon Jan 16 22:14:25 2017
>> >> @@ -3832,13 +3832,13 @@ private:
>> >>
>> >>friend class ASTContext; // creates these
>> >>
>> >> -  AttributedType(QualType canon, Kind attrKind,
>> >> - QualType modified, QualType equivalent)
>> >> -: Type(Attributed, canon, canon->isDependentType(),
>> >> -   canon->isInstantiationDependentType(),
>> >> -   canon->isVariablyModifiedType(),
>> >> -   canon->containsUnexpandedParameterPack()),
>> >> -  ModifiedType(modified), EquivalentType(equivalent) {
>> >> +  AttributedType(QualType canon, Kind attrKind, QualType modified,
>> >> + QualType equivalent)
>> >> +  : Type(Attributed, canon, equivalent->isDependentType(),
>> >> + equivalent->isInstantiationDependentType(),
>> >> + equivalent->isVariablyModifiedType(),
>> >> + equivalent->containsUnexpandedParameterPack()),
>> >> +ModifiedType(modified), EquivalentType(equivalent) {
>> >>  AttributedTypeBits.AttrKind = attrKind;
>> >>}
>> >>
>> >>
>> >> Modified: cfe/trunk/test/CodeGenCXX/microsoft-abi-default-cc.cpp
>> >> URL:
>> >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/microsoft-abi-default-cc.cpp?rev=292194=292193=292194=diff
>> >>
>> >> ==
>> >> --- cfe/trunk/test/CodeGenCXX/microsoft-abi-default-cc.cpp (original)
>> >> +++ cfe/trunk/test/CodeGenCXX/microsoft-abi-default-cc.cpp Mon Jan 16
>> >> 22:14:25 2017
>> >> @@ -45,3 +45,12 @@ void __cdecl static_baz() {}
>> >>  void static_qux() {}
>> >>  // GCABI-LABEL: define void @_Z10static_quxv
>> >>  // MSABI: define void @"\01?static_qux@@YAXXZ"
>> >> +
>> >> +namespace PR31656 {
>> >> +template 
>> >> +void __cdecl callee(int args[I]);
>> >> +// GCABI-LABEL: declare void @_ZN7PR316566calleeILi1EEEvPi(
>> >> +// MSABI: declare void @"\01??$callee@$00@PR31656@@YAXQAH@Z"(
>> >> +
>> >> +void caller() { callee<1>(0); }
>> >> +}
>> >>
>> >>
>> >> ___
>> >> 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] D29350: clang-cl: Evaluate arguments left-to-right in constructor call with initializer list (PR31831)

[Hans Wennborg via Phabricator via cfe-commits]

hans created this revision.

clang-cl would evaluate the arguments right-to-left (see PR), and for 
non-Windows targets I suppose we only got it because we were already emitting 
left-to-right in CodeGenFunction::EmitCallArgs.


https://reviews.llvm.org/D29350

Files:
  lib/CodeGen/CGClass.cpp
  test/CodeGenCXX/initializer-list-ctor-order.cpp


Index: test/CodeGenCXX/initializer-list-ctor-order.cpp
===
--- /dev/null
+++ test/CodeGenCXX/initializer-list-ctor-order.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -std=c++11 %s -emit-llvm -o - -triple %itanium_abi_triple | 
FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-ITANIUM
+// RUN: %clang_cc1 -std=c++11 %s -emit-llvm -o - -triple i686-windows| 
FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-MS
+
+extern "C" {
+int f();
+int g();
+}
+
+struct A {
+  A(int, int);
+};
+
+
+void foo() {
+  A a{f(), g()};
+}
+// CHECK-ITANIUM-LABEL: define void @_Z3foov
+// CHECK-MS-LABEL: define void @"\01?foo@@YAXXZ"
+// CHECK: call i32 @f()
+// CHECK: call i32 @g()
+
+struct B : A {
+  B();
+};
+B::B() : A{f(), g()} {}
+// CHECK-ITANIUM-LABEL: define void @_ZN1BC2Ev
+// CHECK-MS-LABEL: define x86_thiscallcc %struct.B* @"\01??0B@@QAE@XZ"
+// CHECK: call i32 @f()
+// CHECK: call i32 @g()
Index: lib/CodeGen/CGClass.cpp
===
--- lib/CodeGen/CGClass.cpp
+++ lib/CodeGen/CGClass.cpp
@@ -1950,7 +1950,11 @@
 
   // Add the rest of the user-supplied arguments.
   const FunctionProtoType *FPT = D->getType()->castAs();
-  EmitCallArgs(Args, FPT, E->arguments(), E->getConstructor());
+  EvaluationOrder Order = E->isListInitialization()
+  ? EvaluationOrder::ForceLeftToRight
+  : EvaluationOrder::Default;
+  EmitCallArgs(Args, FPT, E->arguments(), E->getConstructor(),
+   /*ParamsToSkip*/ 0, Order);
 
   EmitCXXConstructorCall(D, Type, ForVirtualBase, Delegating, This, Args);
 }


Index: test/CodeGenCXX/initializer-list-ctor-order.cpp
===
--- /dev/null
+++ test/CodeGenCXX/initializer-list-ctor-order.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -std=c++11 %s -emit-llvm -o - -triple %itanium_abi_triple | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-ITANIUM
+// RUN: %clang_cc1 -std=c++11 %s -emit-llvm -o - -triple i686-windows| FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-MS
+
+extern "C" {
+int f();
+int g();
+}
+
+struct A {
+  A(int, int);
+};
+
+
+void foo() {
+  A a{f(), g()};
+}
+// CHECK-ITANIUM-LABEL: define void @_Z3foov
+// CHECK-MS-LABEL: define void @"\01?foo@@YAXXZ"
+// CHECK: call i32 @f()
+// CHECK: call i32 @g()
+
+struct B : A {
+  B();
+};
+B::B() : A{f(), g()} {}
+// CHECK-ITANIUM-LABEL: define void @_ZN1BC2Ev
+// CHECK-MS-LABEL: define x86_thiscallcc %struct.B* @"\01??0B@@QAE@XZ"
+// CHECK: call i32 @f()
+// CHECK: call i32 @g()
Index: lib/CodeGen/CGClass.cpp
===
--- lib/CodeGen/CGClass.cpp
+++ lib/CodeGen/CGClass.cpp
@@ -1950,7 +1950,11 @@
 
   // Add the rest of the user-supplied arguments.
   const FunctionProtoType *FPT = D->getType()->castAs();
-  EmitCallArgs(Args, FPT, E->arguments(), E->getConstructor());
+  EvaluationOrder Order = E->isListInitialization()
+  ? EvaluationOrder::ForceLeftToRight
+  : EvaluationOrder::Default;
+  EmitCallArgs(Args, FPT, E->arguments(), E->getConstructor(),
+   /*ParamsToSkip*/ 0, Order);
 
   EmitCXXConstructorCall(D, Type, ForVirtualBase, Delegating, This, Args);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r293692 - Fix modules codegen to be compatible with modules-ts

[David Blaikie via cfe-commits]

Author: dblaikie
Date: Tue Jan 31 15:28:19 2017
New Revision: 293692

URL: http://llvm.org/viewvc/llvm-project?rev=293692=rev
Log:
Fix modules codegen to be compatible with modules-ts

The Module::WithCodegen flag was only being set when the module was
parsed from a ModuleMap. Instead set it late, in the ASTWriter to match
the layer where the MODULAR_CODEGEN_DECLs list is determined (the
WithCodegen flag essentially means "are this module's decls in
MODULAR_CODEGEN_DECLs").

When simultaneous emission of AST file and modular object is implemented
this may need to change - the Module::WithCodegen flag will need to be
set earlier, and ideally the MODULAR_CODEGEN_DECLs gathering will
consult this flag (that's not possible right now since Decls destined
for an AST File don't have a Module - only if they're /read/ from a
Module is that true - I expect that would need to change as well).

Modified:
cfe/trunk/lib/Lex/ModuleMap.cpp
cfe/trunk/lib/Serialization/ASTWriter.cpp
cfe/trunk/test/CodeGenCXX/modules-ts.cppm

Modified: cfe/trunk/lib/Lex/ModuleMap.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/ModuleMap.cpp?rev=293692=293691=293692=diff
==
--- cfe/trunk/lib/Lex/ModuleMap.cpp (original)
+++ cfe/trunk/lib/Lex/ModuleMap.cpp Tue Jan 31 15:28:19 2017
@@ -1501,7 +1501,6 @@ void ModuleMapParser::parseModuleDecl()
   (!ActiveModule->Parent && ModuleName == "Darwin"))
 ActiveModule->NoUndeclaredIncludes = true;
   ActiveModule->Directory = Directory;
-  ActiveModule->WithCodegen = L.getLangOpts().ModularCodegen;
 
   if (!ActiveModule->Parent) {
 StringRef MapFileName(ModuleMapFile->getName());

Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=293692=293691=293692=diff
==
--- cfe/trunk/lib/Serialization/ASTWriter.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTWriter.cpp Tue Jan 31 15:28:19 2017
@@ -2590,7 +2590,7 @@ void ASTWriter::WriteSubmodules(Module *
   Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // InferExplicit...
   Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // 
InferExportWild...
   Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // 
ConfigMacrosExh...
-  Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 2)); // WithCodegen
+  Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // WithCodegen
   Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob)); // Name
   unsigned DefinitionAbbrev = Stream.EmitAbbrev(std::move(Abbrev));
 
@@ -2690,7 +2690,7 @@ void ASTWriter::WriteSubmodules(Module *
  Mod->InferExplicitSubmodules,
  Mod->InferExportWildcard,
  Mod->ConfigMacrosExhaustive,
- Mod->WithCodegen};
+ Context->getLangOpts().ModularCodegen 
&& WritingModule};
   Stream.EmitRecordWithBlob(DefinitionAbbrev, Record, Mod->Name);
 }
 

Modified: cfe/trunk/test/CodeGenCXX/modules-ts.cppm
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/modules-ts.cppm?rev=293692=293691=293692=diff
==
--- cfe/trunk/test/CodeGenCXX/modules-ts.cppm (original)
+++ cfe/trunk/test/CodeGenCXX/modules-ts.cppm Tue Jan 31 15:28:19 2017
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fmodules-ts -std=c++1z -triple=x86_64-linux-gnu 
-emit-module-interface %s -o %t.pcm
+// RUN: %clang_cc1 -fmodules-ts -std=c++1z -triple=x86_64-linux-gnu 
-fmodules-codegen -emit-module-interface %s -o %t.pcm
 // RUN: %clang_cc1 -fmodules-ts -std=c++1z -triple=x86_64-linux-gnu %t.pcm 
-emit-llvm -o - | FileCheck %s
 
 module FooBar;
@@ -8,6 +8,9 @@ export {
   int f() { return 0; }
 }
 
+// CHECK-LABEL: define weak_odr void @_Z2f2v(
+inline void f2() { }
+
 // FIXME: Emit global variables and their initializers with this TU.
 // Emit an initialization function that other TUs can call, with guard 
variable.
 


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

[PATCH] D16135: Macro Debug Info support in Clang

[Richard Smith via Phabricator via cfe-commits]

rsmith added inline comments.



Comment at: include/clang/CodeGen/ModuleBuilder.h:71
+  ///
+  /// This methods can be called before initializing the CGDebugInfo calss.
+  /// But the returned value should not be used until after initialization.

Typo 'calss'



Comment at: include/clang/CodeGen/ModuleBuilder.h:72
+  /// This methods can be called before initializing the CGDebugInfo calss.
+  /// But the returned value should not be used until after initialization.
+  /// It is caller responsibility to validate that the place holder was

What is the returned value in that case? A reference to a null pointer?

Generally, I'm not particularly happy with this approach of exposing a 
reference to an internal pointer as the way of propagating the `CGDebugInfo` to 
the macro generator as needed. Instead, how about giving the `MacroPPCallbacks` 
object a pointer to the `CodeGenerator` itself, and having it ask the 
`CodeGenModule` for the debug info object when it needs it?



Comment at: include/clang/CodeGen/ModuleBuilder.h:73
+  /// But the returned value should not be used until after initialization.
+  /// It is caller responsibility to validate that the place holder was
+  /// initialized before start using it.

caller -> the caller's



Comment at: include/clang/CodeGen/ModuleBuilder.h:74
+  /// It is caller responsibility to validate that the place holder was
+  /// initialized before start using it.
+  CodeGen::CGDebugInfo *::getModuleDebugInfoRef();

start using -> starting to use



Comment at: lib/CodeGen/MacroPPCallbacks.cpp:1
+//===--- MacroPPCallbacks.h -*- C++ 
-*-===//
+//

Wrong file header (should be .cpp, and you don't need the "-*- C++ -*-" in a 
.cpp file.


https://reviews.llvm.org/D16135



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

Re: r290539 - [inline-asm]No error for conflict between inputs\outputs and clobber list

[Dimitry Andric via cfe-commits]

On 26 Dec 2016, at 13:23, Marina Yatsina via cfe-commits 
 wrote:
> 
> Author: myatsina
> Date: Mon Dec 26 06:23:42 2016
> New Revision: 290539
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=290539=rev
> Log:
> [inline-asm]No error for conflict between inputs\outputs and clobber list
> 
> According to extended asm syntax, a case where the clobber list includes a 
> variable from the inputs or outputs should be an error - conflict.
> for example:
> 
> const long double a = 0.0;
> int main()
> {
> 
> char b;
> double t1 = a;
> __asm__ ("fucompp": "=a" (b) : "u" (t1), "t" (t1) : "cc", "st", "st(1)");
> 
> return 0;
> }
> 
> This should conflict with the output - t1 which is st, and st which is st 
> aswell.

In FreeBSD ports, we are now getting this new error message when compiling 
svgalib (see https://bugs.freebsd.org/216154), when compiling with clang 4.0.0.

While fixing it, we noticed something strange (or interesting, depending on 
your point of view), namely that it does not seem to handle the first input or 
output operand, e.g. "0".  For example, svgalib has this inline function:

/* Always 32-bit align destination, even for a small number of bytes. */
static inline void *
 __memcpy_aligndest(void *dest, const void *src, int n)
{
__asm__ __volatile__("cmpl $3, %%ecx\n\t"
 "ja 1f\n\t"
 "call * __memcpy_jumptable (, %%ecx, 4)\n\t"
 "jmp 2f\n\t"
 "1:call __memcpyasm_regargs\n\t"
 "2:":
 :"S"(dest), "d"(src), "c"(n)
 :"ax", "0", "1", "2");
return dest;
}

The error produced for this is:

svgalib-1.4.3/gl/inlstring.h:281:17: error: asm-specifier for input or output 
variable conflicts with asm clobber list
 :"ax", "0", "1", "2");
 ^

And indeed, removing the "1" and "2" input operands fixes the error.  However, 
by definition, "0" is always an input or output operand, so should it not 
produce an error too?

-Dimitry



signature.asc
Description: Message signed with OpenPGP using GPGMail
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24969: [Sema] Use the instantiated name of destructors in FindInstantiatedDecl and RebuildMemberExpr

[Akira Hatanaka via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL293678: [Sema] Transform a templated name before looking it 
up in (authored by ahatanak).

Changed prior to commit:
  https://reviews.llvm.org/D24969?vs=82134=86474#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D24969

Files:
  cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
  cfe/trunk/lib/Sema/TreeTransform.h
  cfe/trunk/test/SemaCXX/destructor.cpp


Index: cfe/trunk/test/SemaCXX/destructor.cpp
===
--- cfe/trunk/test/SemaCXX/destructor.cpp
+++ cfe/trunk/test/SemaCXX/destructor.cpp
@@ -431,3 +431,23 @@
 
 // The constructor definition should not have errors
 Invalid::~Invalid() {}
+
+namespace PR30361 {
+template 
+struct C1 {
+  ~C1() {}
+  operator C1* () { return nullptr; }
+  void foo1();
+};
+
+template
+void C1::foo1() {
+  C1::operator C1*();
+  C1::~C1();
+}
+
+void foo1() {
+  C1 x;
+  x.foo1();
+}
+}
Index: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -4990,8 +4990,12 @@
 NamedDecl *Result = nullptr;
 // FIXME: If the name is a dependent name, this lookup won't necessarily
 // find it. Does that ever matter?
-if (D->getDeclName()) {
-  DeclContext::lookup_result Found = ParentDC->lookup(D->getDeclName());
+if (auto Name = D->getDeclName()) {
+  DeclarationNameInfo NameInfo(Name, D->getLocation());
+  Name = SubstDeclarationNameInfo(NameInfo, TemplateArgs).getName();
+  if (!Name)
+return nullptr;
+  DeclContext::lookup_result Found = ParentDC->lookup(Name);
   Result = findInstantiationOf(Context, D, Found.begin(), Found.end());
 } else {
   // Since we don't have a name for the entity we're looking for,
Index: cfe/trunk/lib/Sema/TreeTransform.h
===
--- cfe/trunk/lib/Sema/TreeTransform.h
+++ cfe/trunk/lib/Sema/TreeTransform.h
@@ -8966,12 +8966,18 @@
   // base (and therefore couldn't do the check) and a
   // nested-name-qualifier (and therefore could do the lookup).
   NamedDecl *FirstQualifierInScope = nullptr;
+  DeclarationNameInfo MemberNameInfo = E->getMemberNameInfo();
+  if (MemberNameInfo.getName()) {
+MemberNameInfo = getDerived().TransformDeclarationNameInfo(MemberNameInfo);
+if (!MemberNameInfo.getName())
+  return ExprError();
+  }
 
   return getDerived().RebuildMemberExpr(Base.get(), FakeOperatorLoc,
 E->isArrow(),
 QualifierLoc,
 TemplateKWLoc,
-E->getMemberNameInfo(),
+MemberNameInfo,
 Member,
 FoundDecl,
 (E->hasExplicitTemplateArgs()


Index: cfe/trunk/test/SemaCXX/destructor.cpp
===
--- cfe/trunk/test/SemaCXX/destructor.cpp
+++ cfe/trunk/test/SemaCXX/destructor.cpp
@@ -431,3 +431,23 @@
 
 // The constructor definition should not have errors
 Invalid::~Invalid() {}
+
+namespace PR30361 {
+template 
+struct C1 {
+  ~C1() {}
+  operator C1* () { return nullptr; }
+  void foo1();
+};
+
+template
+void C1::foo1() {
+  C1::operator C1*();
+  C1::~C1();
+}
+
+void foo1() {
+  C1 x;
+  x.foo1();
+}
+}
Index: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -4990,8 +4990,12 @@
 NamedDecl *Result = nullptr;
 // FIXME: If the name is a dependent name, this lookup won't necessarily
 // find it. Does that ever matter?
-if (D->getDeclName()) {
-  DeclContext::lookup_result Found = ParentDC->lookup(D->getDeclName());
+if (auto Name = D->getDeclName()) {
+  DeclarationNameInfo NameInfo(Name, D->getLocation());
+  Name = SubstDeclarationNameInfo(NameInfo, TemplateArgs).getName();
+  if (!Name)
+return nullptr;
+  DeclContext::lookup_result Found = ParentDC->lookup(Name);
   Result = findInstantiationOf(Context, D, Found.begin(), Found.end());
 } else {
   // Since we don't have a name for the entity we're looking for,
Index: cfe/trunk/lib/Sema/TreeTransform.h
===
--- cfe/trunk/lib/Sema/TreeTransform.h
+++ cfe/trunk/lib/Sema/TreeTransform.h
@@ -8966,12 +8966,18 @@
   // base (and therefore couldn't do the check) and a
   // nested-name-qualifier (and therefore could do the lookup).
   NamedDecl *FirstQualifierInScope = nullptr;
+  

r293678 - [Sema] Transform a templated name before looking it up in

[Akira Hatanaka via cfe-commits]

Author: ahatanak
Date: Tue Jan 31 13:53:32 2017
New Revision: 293678

URL: http://llvm.org/viewvc/llvm-project?rev=293678=rev
Log:
[Sema] Transform a templated name before looking it up in
FindInstantiatedDecl or passing it to RebuildMemberExpr.

This fixes PR30361.

rdar://problem/17341274

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

Modified:
cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
cfe/trunk/lib/Sema/TreeTransform.h
cfe/trunk/test/SemaCXX/destructor.cpp

Modified: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp?rev=293678=293677=293678=diff
==
--- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp Tue Jan 31 13:53:32 2017
@@ -4990,8 +4990,12 @@ NamedDecl *Sema::FindInstantiatedDecl(So
 NamedDecl *Result = nullptr;
 // FIXME: If the name is a dependent name, this lookup won't necessarily
 // find it. Does that ever matter?
-if (D->getDeclName()) {
-  DeclContext::lookup_result Found = ParentDC->lookup(D->getDeclName());
+if (auto Name = D->getDeclName()) {
+  DeclarationNameInfo NameInfo(Name, D->getLocation());
+  Name = SubstDeclarationNameInfo(NameInfo, TemplateArgs).getName();
+  if (!Name)
+return nullptr;
+  DeclContext::lookup_result Found = ParentDC->lookup(Name);
   Result = findInstantiationOf(Context, D, Found.begin(), Found.end());
 } else {
   // Since we don't have a name for the entity we're looking for,

Modified: cfe/trunk/lib/Sema/TreeTransform.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/TreeTransform.h?rev=293678=293677=293678=diff
==
--- cfe/trunk/lib/Sema/TreeTransform.h (original)
+++ cfe/trunk/lib/Sema/TreeTransform.h Tue Jan 31 13:53:32 2017
@@ -8966,12 +8966,18 @@ TreeTransform::TransformMemberE
   // base (and therefore couldn't do the check) and a
   // nested-name-qualifier (and therefore could do the lookup).
   NamedDecl *FirstQualifierInScope = nullptr;
+  DeclarationNameInfo MemberNameInfo = E->getMemberNameInfo();
+  if (MemberNameInfo.getName()) {
+MemberNameInfo = getDerived().TransformDeclarationNameInfo(MemberNameInfo);
+if (!MemberNameInfo.getName())
+  return ExprError();
+  }
 
   return getDerived().RebuildMemberExpr(Base.get(), FakeOperatorLoc,
 E->isArrow(),
 QualifierLoc,
 TemplateKWLoc,
-E->getMemberNameInfo(),
+MemberNameInfo,
 Member,
 FoundDecl,
 (E->hasExplicitTemplateArgs()

Modified: cfe/trunk/test/SemaCXX/destructor.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/destructor.cpp?rev=293678=293677=293678=diff
==
--- cfe/trunk/test/SemaCXX/destructor.cpp (original)
+++ cfe/trunk/test/SemaCXX/destructor.cpp Tue Jan 31 13:53:32 2017
@@ -431,3 +431,23 @@ class Invalid {
 
 // The constructor definition should not have errors
 Invalid::~Invalid() {}
+
+namespace PR30361 {
+template 
+struct C1 {
+  ~C1() {}
+  operator C1* () { return nullptr; }
+  void foo1();
+};
+
+template
+void C1::foo1() {
+  C1::operator C1*();
+  C1::~C1();
+}
+
+void foo1() {
+  C1 x;
+  x.foo1();
+}
+}


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

r293677 - Extend -Wcast-calling-convention to warn on declarations as well as definitions

[Reid Kleckner via cfe-commits]

Author: rnk
Date: Tue Jan 31 13:37:45 2017
New Revision: 293677

URL: http://llvm.org/viewvc/llvm-project?rev=293677=rev
Log:
Extend -Wcast-calling-convention to warn on declarations as well as definitions

My original warning was very conservative and I never revisited the
heuristics that were used.

This would have caught http://crbug.com/687251 at compile time.

Modified:
cfe/trunk/lib/Sema/SemaCast.cpp
cfe/trunk/test/Sema/callingconv-cast.c

Modified: cfe/trunk/lib/Sema/SemaCast.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCast.cpp?rev=293677=293676=293677=diff
==
--- cfe/trunk/lib/Sema/SemaCast.cpp (original)
+++ cfe/trunk/lib/Sema/SemaCast.cpp Tue Jan 31 13:37:45 2017
@@ -1763,13 +1763,12 @@ static void DiagnoseCallingConvCast(Sema
   if (!DRE)
 return;
   auto *FD = dyn_cast(DRE->getDecl());
-  const FunctionDecl *Definition;
-  if (!FD || !FD->hasBody(Definition))
+  if (!FD)
 return;
 
   // Only warn if we are casting from the default convention to a non-default
   // convention. This can happen when the programmer forgot to apply the 
calling
-  // convention to the function definition and then inserted this cast to
+  // convention to the function declaration and then inserted this cast to
   // satisfy the type system.
   CallingConv DefaultCC = Self.getASTContext().getDefaultCallingConvention(
   FD->isVariadic(), FD->isCXXInstanceMember());
@@ -1792,7 +1791,7 @@ static void DiagnoseCallingConvCast(Sema
   // whose address was taken. Try to use the latest macro for the convention.
   // For example, users probably want to write "WINAPI" instead of "__stdcall"
   // to match the Windows header declarations.
-  SourceLocation NameLoc = Definition->getNameInfo().getLoc();
+  SourceLocation NameLoc = FD->getFirstDecl()->getNameInfo().getLoc();
   Preprocessor  = Self.getPreprocessor();
   SmallVector AttrTokens;
   SmallString<64> CCAttrText;

Modified: cfe/trunk/test/Sema/callingconv-cast.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/callingconv-cast.c?rev=293677=293676=293677=diff
==
--- cfe/trunk/test/Sema/callingconv-cast.c (original)
+++ cfe/trunk/test/Sema/callingconv-cast.c Tue Jan 31 13:37:45 2017
@@ -15,6 +15,13 @@ void mismatched_before_winapi(int x) {}
 // expected-note@+1 3 {{consider defining 'mismatched' with the 'stdcall' 
calling convention}}
 void mismatched(int x) {}
 
+// expected-note@+1 {{consider defining 'mismatched_declaration' with the 
'stdcall' calling convention}}
+void mismatched_declaration(int x);
+
+// expected-note@+1 {{consider defining 'suggest_fix_first_redecl' with the 
'stdcall' calling convention}}
+void suggest_fix_first_redecl(int x);
+void suggest_fix_first_redecl(int x);
+
 typedef void (WINAPI *callback_t)(int);
 void take_callback(callback_t callback);
 
@@ -46,6 +53,12 @@ int main() {
   // Another way to suppress the warning.
   take_callback((callback_t)(void*)mismatched);
 
+  // Warn on declarations as well as definitions.
+  // expected-warning@+1 {{cast between incompatible calling conventions 
'cdecl' and 'stdcall'}}
+  take_callback((callback_t)mismatched_declaration);
+  // expected-warning@+1 {{cast between incompatible calling conventions 
'cdecl' and 'stdcall'}}
+  take_callback((callback_t)suggest_fix_first_redecl);
+
   // Don't warn, because we're casting from stdcall to cdecl. Usually that 
means
   // the programmer is rinsing the function pointer through some kind of opaque
   // API.


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

Re: r291963 - [clang] Emit `diagnose_if` warnings from system headers

[George Burgess IV via cfe-commits]

> If not, perhaps we should disable the attribute for the Clang 4 release
instead

FWIW, I'd strongly prefer to do this over letting diagnose_if go into Clang
4 unpatched. So, if my patch does feel too big, I'm happy to let
diagnose_if be a new-in-clang-5 attribute. :)

On Tue, Jan 31, 2017 at 11:11 AM, Richard Smith 
wrote:

> Yes, this makes sense. We should also decide what we're going to do about
> the larger diagnose_if patch that George has asked to be ported to Clang 4.
> Are you comfortable taking a patch of that size? If not, perhaps we should
> disable the attribute for the Clang 4 release instead.
>
>
> On 31 January 2017 at 10:36, Hans Wennborg  wrote:
>
>> Ping?
>>
>> On Thu, Jan 26, 2017 at 10:21 AM, Hans Wennborg 
>> wrote:
>> > Ping?
>> >
>> > On Mon, Jan 23, 2017 at 4:27 PM, Hans Wennborg 
>> wrote:
>> >> Ping?
>> >>
>> >> On Tue, Jan 17, 2017 at 4:16 PM, Hans Wennborg 
>> wrote:
>> >>> Richard, what do you think?
>> >>>
>> >>> On Fri, Jan 13, 2017 at 3:16 PM, Eric Fiselier  wrote:
>>  I would love to see this merged. It would make it easier to write
>> libc++
>>  tests if the tests didn't have to worry about the old 4.0 behavior.
>> 
>>  CC'ing Richard: Would merging this be OK?
>> 
>>  On Fri, Jan 13, 2017 at 3:46 PM, George Burgess IV
>>   wrote:
>> >
>> > Do we want to consider merging this into the release branch? Seems
>> like
>> > more of a bugfix than a feature to me.
>> >
>> > On Fri, Jan 13, 2017 at 2:11 PM, Eric Fiselier via cfe-commits
>> >  wrote:
>> >>
>> >> Author: ericwf
>> >> Date: Fri Jan 13 16:11:40 2017
>> >> New Revision: 291963
>> >>
>> >> URL: http://llvm.org/viewvc/llvm-project?rev=291963=rev
>> >> Log:
>> >> [clang] Emit `diagnose_if` warnings from system headers
>> >>
>> >> Summary: In order for libc++ to meaningfully use `diagnose_if`
>> warnings
>> >> they need to be emitted from system headers by default. This patch
>> changes
>> >> the `diagnose_if` warning diagnostic to be shown in system headers.
>> >>
>> >> Reviewers: george.burgess.iv, rsmith, aaron.ballman
>> >>
>> >> Subscribers: cfe-commits
>> >>
>> >> Differential Revision: https://reviews.llvm.org/D28703
>> >>
>> >> Added:
>> >> cfe/trunk/test/Sema/Inputs/diagnose-if-warn-system-header.h
>> >> Modified:
>> >> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> >> cfe/trunk/test/Sema/diagnose_if.c
>> >>
>> >> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> >> URL:
>> >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/
>> Basic/DiagnosticSemaKinds.td?rev=291963=291962=291963=diff
>> >>
>> >> 
>> ==
>> >> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> (original)
>> >> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Jan
>> 13
>> >> 16:11:40 2017
>> >> @@ -3380,7 +3380,8 @@ def note_ovl_candidate_has_pass_object_s
>> >>  "candidate address cannot be taken because parameter %0 has "
>> >>  "pass_object_size attribute">;
>> >>  def err_diagnose_if_succeeded : Error<"%0">;
>> >> -def warn_diagnose_if_succeeded : Warning<"%0">,
>> >> InGroup;
>> >> +def warn_diagnose_if_succeeded : Warning<"%0">,
>> >> InGroup,
>> >> +ShowInSystemHeader;
>> >>  def note_ovl_candidate_disabled_by_function_cond_attr : Note<
>> >>  "candidate disabled: %0">;
>> >>  def note_ovl_candidate_disabled_by_extension : Note<
>> >>
>> >> Added: cfe/trunk/test/Sema/Inputs/diagnose-if-warn-system-header.h
>> >> URL:
>> >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/Inpu
>> ts/diagnose-if-warn-system-header.h?rev=291963=auto
>> >>
>> >> 
>> ==
>> >> --- cfe/trunk/test/Sema/Inputs/diagnose-if-warn-system-header.h
>> (added)
>> >> +++ cfe/trunk/test/Sema/Inputs/diagnose-if-warn-system-header.h
>> Fri Jan
>> >> 13 16:11:40 2017
>> >> @@ -0,0 +1,11 @@
>> >> +#pragma GCC system_header
>> >> +
>> >> +inline int system_header_func(int x)
>> >> +  __attribute__((diagnose_if(x == x, "system header warning",
>> >> "warning"))) // expected-note {{from 'diagnose_if' attribute}}
>> >> +{
>> >> +  return 0;
>> >> +}
>> >> +
>> >> +void test_system_header() {
>> >> +  system_header_func(0); // expected-warning {{system header
>> warning}}
>> >> +}
>> >>
>> >> Modified: cfe/trunk/test/Sema/diagnose_if.c
>> >> URL:
>> >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/diag
>> 

r293676 - [windows] [asan] Fix tests failing after 293668.

[Marcos Pividori via cfe-commits]

Author: mpividori
Date: Tue Jan 31 13:05:05 2017
New Revision: 293676

URL: http://llvm.org/viewvc/llvm-project?rev=293676=rev
Log:
[windows] [asan] Fix tests failing after 293668.

Modified:
cfe/trunk/test/Driver/cl-link.c

Modified: cfe/trunk/test/Driver/cl-link.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/cl-link.c?rev=293676=293675=293676=diff
==
--- cfe/trunk/test/Driver/cl-link.c (original)
+++ cfe/trunk/test/Driver/cl-link.c Tue Jan 31 13:05:05 2017
@@ -13,8 +13,9 @@
 // ASAN: link.exe
 // ASAN: "-debug"
 // ASAN: "-incremental:no"
-// ASAN: "{{.*}}clang_rt.asan-i386.lib"
+// ASAN: "{{[^"]*}}clang_rt.asan-i386.lib"
 // ASAN: "{{.*}}clang_rt.asan_cxx-i386.lib"
+// ASAN: "-wholearchive:{{.*}}clang_rt.asan-i386.lib"
 // ASAN: "{{.*}}cl-link{{.*}}.obj"
 
 // RUN: %clang_cl -m32 -arch:IA32 --target=i386-pc-win32 /MD /Tc%s -### 
-fsanitize=address 2>&1 | FileCheck --check-prefix=ASAN-MD %s


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

Re: r292194 - [AST] AttributedType should derive type properties from the EquivalentType

[Richard Smith via cfe-commits]

Sure, let's take this for Clang 4.

On 31 January 2017 at 10:39, Hans Wennborg  wrote:

> Ping?
>
> On Thu, Jan 26, 2017 at 3:42 PM, Hans Wennborg  wrote:
> > Should we merge this to the release branch?
> >
> > On Mon, Jan 16, 2017 at 8:14 PM, David Majnemer via cfe-commits
> >  wrote:
> >> Author: majnemer
> >> Date: Mon Jan 16 22:14:25 2017
> >> New Revision: 292194
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=292194=rev
> >> Log:
> >> [AST] AttributedType should derive type properties from the
> EquivalentType
> >>
> >> Using the canonical type instead of the equivalent type can result in
> >> insufficient template instantiations.
> >>
> >> This fixes PR31656.
> >>
> >> Differential Revision: https://reviews.llvm.org/D28788
> >>
> >> Modified:
> >> cfe/trunk/include/clang/AST/Type.h
> >> cfe/trunk/test/CodeGenCXX/microsoft-abi-default-cc.cpp
> >>
> >> Modified: cfe/trunk/include/clang/AST/Type.h
> >> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/
> clang/AST/Type.h?rev=292194=292193=292194=diff
> >> 
> ==
> >> --- cfe/trunk/include/clang/AST/Type.h (original)
> >> +++ cfe/trunk/include/clang/AST/Type.h Mon Jan 16 22:14:25 2017
> >> @@ -3832,13 +3832,13 @@ private:
> >>
> >>friend class ASTContext; // creates these
> >>
> >> -  AttributedType(QualType canon, Kind attrKind,
> >> - QualType modified, QualType equivalent)
> >> -: Type(Attributed, canon, canon->isDependentType(),
> >> -   canon->isInstantiationDependentType(),
> >> -   canon->isVariablyModifiedType(),
> >> -   canon->containsUnexpandedParameterPack()),
> >> -  ModifiedType(modified), EquivalentType(equivalent) {
> >> +  AttributedType(QualType canon, Kind attrKind, QualType modified,
> >> + QualType equivalent)
> >> +  : Type(Attributed, canon, equivalent->isDependentType(),
> >> + equivalent->isInstantiationDependentType(),
> >> + equivalent->isVariablyModifiedType(),
> >> + equivalent->containsUnexpandedParameterPack()),
> >> +ModifiedType(modified), EquivalentType(equivalent) {
> >>  AttributedTypeBits.AttrKind = attrKind;
> >>}
> >>
> >>
> >> Modified: cfe/trunk/test/CodeGenCXX/microsoft-abi-default-cc.cpp
> >> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/
> CodeGenCXX/microsoft-abi-default-cc.cpp?rev=292194=
> 292193=292194=diff
> >> 
> ==
> >> --- cfe/trunk/test/CodeGenCXX/microsoft-abi-default-cc.cpp (original)
> >> +++ cfe/trunk/test/CodeGenCXX/microsoft-abi-default-cc.cpp Mon Jan 16
> 22:14:25 2017
> >> @@ -45,3 +45,12 @@ void __cdecl static_baz() {}
> >>  void static_qux() {}
> >>  // GCABI-LABEL: define void @_Z10static_quxv
> >>  // MSABI: define void @"\01?static_qux@@YAXXZ"
> >> +
> >> +namespace PR31656 {
> >> +template 
> >> +void __cdecl callee(int args[I]);
> >> +// GCABI-LABEL: declare void @_ZN7PR316566calleeILi1EEEvPi(
> >> +// MSABI: declare void @"\01??$callee@$00@PR31656@@YAXQAH@Z"(
> >> +
> >> +void caller() { callee<1>(0); }
> >> +}
> >>
> >>
> >> ___
> >> 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: r291963 - [clang] Emit `diagnose_if` warnings from system headers

[Richard Smith via cfe-commits]

Yes, this makes sense. We should also decide what we're going to do about
the larger diagnose_if patch that George has asked to be ported to Clang 4.
Are you comfortable taking a patch of that size? If not, perhaps we should
disable the attribute for the Clang 4 release instead.

On 31 January 2017 at 10:36, Hans Wennborg  wrote:

> Ping?
>
> On Thu, Jan 26, 2017 at 10:21 AM, Hans Wennborg  wrote:
> > Ping?
> >
> > On Mon, Jan 23, 2017 at 4:27 PM, Hans Wennborg 
> wrote:
> >> Ping?
> >>
> >> On Tue, Jan 17, 2017 at 4:16 PM, Hans Wennborg 
> wrote:
> >>> Richard, what do you think?
> >>>
> >>> On Fri, Jan 13, 2017 at 3:16 PM, Eric Fiselier  wrote:
>  I would love to see this merged. It would make it easier to write
> libc++
>  tests if the tests didn't have to worry about the old 4.0 behavior.
> 
>  CC'ing Richard: Would merging this be OK?
> 
>  On Fri, Jan 13, 2017 at 3:46 PM, George Burgess IV
>   wrote:
> >
> > Do we want to consider merging this into the release branch? Seems
> like
> > more of a bugfix than a feature to me.
> >
> > On Fri, Jan 13, 2017 at 2:11 PM, Eric Fiselier via cfe-commits
> >  wrote:
> >>
> >> Author: ericwf
> >> Date: Fri Jan 13 16:11:40 2017
> >> New Revision: 291963
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=291963=rev
> >> Log:
> >> [clang] Emit `diagnose_if` warnings from system headers
> >>
> >> Summary: In order for libc++ to meaningfully use `diagnose_if`
> warnings
> >> they need to be emitted from system headers by default. This patch
> changes
> >> the `diagnose_if` warning diagnostic to be shown in system headers.
> >>
> >> Reviewers: george.burgess.iv, rsmith, aaron.ballman
> >>
> >> Subscribers: cfe-commits
> >>
> >> Differential Revision: https://reviews.llvm.org/D28703
> >>
> >> Added:
> >> cfe/trunk/test/Sema/Inputs/diagnose-if-warn-system-header.h
> >> Modified:
> >> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> >> cfe/trunk/test/Sema/diagnose_if.c
> >>
> >> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> >> URL:
> >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/
> DiagnosticSemaKinds.td?rev=291963=291962=291963=diff
> >>
> >> 
> ==
> >> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> >> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Jan 13
> >> 16:11:40 2017
> >> @@ -3380,7 +3380,8 @@ def note_ovl_candidate_has_pass_object_s
> >>  "candidate address cannot be taken because parameter %0 has "
> >>  "pass_object_size attribute">;
> >>  def err_diagnose_if_succeeded : Error<"%0">;
> >> -def warn_diagnose_if_succeeded : Warning<"%0">,
> >> InGroup;
> >> +def warn_diagnose_if_succeeded : Warning<"%0">,
> >> InGroup,
> >> +ShowInSystemHeader;
> >>  def note_ovl_candidate_disabled_by_function_cond_attr : Note<
> >>  "candidate disabled: %0">;
> >>  def note_ovl_candidate_disabled_by_extension : Note<
> >>
> >> Added: cfe/trunk/test/Sema/Inputs/diagnose-if-warn-system-header.h
> >> URL:
> >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/
> Inputs/diagnose-if-warn-system-header.h?rev=291963=auto
> >>
> >> 
> ==
> >> --- cfe/trunk/test/Sema/Inputs/diagnose-if-warn-system-header.h
> (added)
> >> +++ cfe/trunk/test/Sema/Inputs/diagnose-if-warn-system-header.h
> Fri Jan
> >> 13 16:11:40 2017
> >> @@ -0,0 +1,11 @@
> >> +#pragma GCC system_header
> >> +
> >> +inline int system_header_func(int x)
> >> +  __attribute__((diagnose_if(x == x, "system header warning",
> >> "warning"))) // expected-note {{from 'diagnose_if' attribute}}
> >> +{
> >> +  return 0;
> >> +}
> >> +
> >> +void test_system_header() {
> >> +  system_header_func(0); // expected-warning {{system header
> warning}}
> >> +}
> >>
> >> Modified: cfe/trunk/test/Sema/diagnose_if.c
> >> URL:
> >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/
> diagnose_if.c?rev=291963=291962=291963=diff
> >>
> >> 
> ==
> >> --- cfe/trunk/test/Sema/diagnose_if.c (original)
> >> +++ cfe/trunk/test/Sema/diagnose_if.c Fri Jan 13 16:11:40 2017
> >> @@ -150,3 +150,6 @@ void alwaysWarnWithArg(int a) _diagnose_
> >>  void runAlwaysWarnWithArg(int a) {
> >>alwaysWarnWithArg(a); // expected-warning{{alwaysWarn}}
> >>  }
> >> +
> >> +// Test that diagnose_if 

r293675 - Keep Chromium ObjC column limit at 80 for consistency with C++

[Nico Weber via cfe-commits]

Author: nico
Date: Tue Jan 31 12:42:05 2017
New Revision: 293675

URL: http://llvm.org/viewvc/llvm-project?rev=293675=rev
Log:
Keep Chromium ObjC column limit at 80 for consistency with C++

https://reviews.llvm.org/D29337
Patch from Dan Beam !

Modified:
cfe/trunk/lib/Format/Format.cpp

Modified: cfe/trunk/lib/Format/Format.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=293675=293674=293675=diff
==
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Tue Jan 31 12:42:05 2017
@@ -653,6 +653,8 @@ FormatStyle getChromiumStyle(FormatStyle
 ChromiumStyle.AllowShortLoopsOnASingleLine = false;
 ChromiumStyle.BinPackParameters = false;
 ChromiumStyle.DerivePointerAlignment = false;
+if (Language == FormatStyle::LK_ObjC)
+  ChromiumStyle.ColumnLimit = 80;
   }
   ChromiumStyle.SortIncludes = false;
   return ChromiumStyle;


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

[PATCH] D29339: [OpenMP] Add support for auxiliary triple specification

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea created this revision.

Device offloading requires the specification of an additional flag containing 
the triple of the //other// architecture the code is being compiled on if such 
an architecture exists. If compiling for the host, the auxiliary triple flag 
will contain the triple describing the device and vice versa.


Repository:
  rL LLVM

https://reviews.llvm.org/D29339

Files:
  lib/Driver/Tools.cpp
  lib/Frontend/CompilerInstance.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Frontend/InitPreprocessor.cpp
  test/Driver/openmp-offload.c

Index: test/Driver/openmp-offload.c
===
--- test/Driver/openmp-offload.c
+++ test/Driver/openmp-offload.c
@@ -248,32 +248,32 @@
 //
 // Compile for the powerpc device.
 //
-// CHK-COMMANDS: clang{{.*}}" "-cc1" "-triple" "powerpc64le-ibm-linux-gnu" "-emit-obj" {{.*}}"-pic-level" "2" {{.*}}"-fopenmp" {{.*}}"-o" "
+// CHK-COMMANDS: clang{{.*}}" "-cc1" "-triple" "powerpc64le-ibm-linux-gnu" "-aux-triple" "powerpc64le--linux" "-emit-obj" {{.*}}"-pic-level" "2" {{.*}}"-fopenmp" {{.*}}"-o" "
 // CHK-COMMANDS-SAME: [[T1OBJ:[^\\/]+\.o]]" "-x" "c" "{{.*}}[[INPUT]]" "-fopenmp-is-device" "-fopenmp-host-ir-file-path" "{{.*}}[[HOSTBC]]"
 // CHK-COMMANDS: ld{{(\.exe)?}}" {{.*}}"-o" "
 // CHK-COMMANDS-SAME: [[T1BIN:[^\\/]+\.out]]" {{.*}}"{{.*}}[[T1OBJ]]"
-// CHK-COMMANDS-ST: clang{{.*}}" "-cc1" "-triple" "powerpc64le-ibm-linux-gnu" "-E" {{.*}}"-fopenmp" {{.*}}"-o" "
+// CHK-COMMANDS-ST: clang{{.*}}" "-cc1" "-triple" "powerpc64le-ibm-linux-gnu" "-aux-triple" "powerpc64le--linux" "-E" {{.*}}"-fopenmp" {{.*}}"-o" "
 // CHK-COMMANDS-ST-SAME: [[T1PP:[^\\/]+\.i]]" "-x" "c" "{{.*}}[[INPUT]]"
-// CHK-COMMANDS-ST: clang{{.*}}" "-cc1" "-triple" "powerpc64le-ibm-linux-gnu" "-emit-llvm-bc" {{.*}}"-pic-level" "2" {{.*}}"-fopenmp" {{.*}}"-o" "
+// CHK-COMMANDS-ST: clang{{.*}}" "-cc1" "-triple" "powerpc64le-ibm-linux-gnu" "-aux-triple" "powerpc64le--linux" "-emit-llvm-bc" {{.*}}"-pic-level" "2" {{.*}}"-fopenmp" {{.*}}"-o" "
 // CHK-COMMANDS-ST-SAME: [[T1BC:[^\\/]+\.bc]]" "-x" "cpp-output" "{{.*}}[[T1PP]]" "-fopenmp-is-device" "-fopenmp-host-ir-file-path" "{{.*}}[[HOSTBC]]"
-// CHK-COMMANDS-ST: clang{{.*}}" "-cc1" "-triple" "powerpc64le-ibm-linux-gnu" "-S" {{.*}}"-fopenmp" {{.*}}"-o" "
+// CHK-COMMANDS-ST: clang{{.*}}" "-cc1" "-triple" "powerpc64le-ibm-linux-gnu" "-aux-triple" "powerpc64le--linux" "-S" {{.*}}"-fopenmp" {{.*}}"-o" "
 // CHK-COMMANDS-ST-SAME: [[T1ASM:[^\\/]+\.s]]" "-x" "ir" "{{.*}}[[T1BC]]"
 // CHK-COMMANDS-ST: clang{{.*}}" "-cc1as" "-triple" "powerpc64le-ibm-linux-gnu" "-filetype" "obj" {{.*}}"-o" "
 // CHK-COMMANDS-ST-SAME: [[T1OBJ:[^\\/]+\.o]]" "{{.*}}[[T1ASM]]"
 // CHK-COMMANDS-ST: ld{{(\.exe)?}}" {{.*}}"-shared" {{.*}}"-o" "
 // CHK-COMMANDS-ST-SAME: [[T1BIN:[^\\/]+\.out-openmp-powerpc64le-ibm-linux-gnu]]" {{.*}}"{{.*}}[[T1OBJ]]"
 //
 // Compile for the x86 device.
 //
-// CHK-COMMANDS: clang{{.*}}" "-cc1" "-triple" "x86_64-pc-linux-gnu" "-emit-obj" {{.*}}"-pic-level" "2" {{.*}}"-fopenmp"  {{.*}}"-o" "
+// CHK-COMMANDS: clang{{.*}}" "-cc1" "-triple" "x86_64-pc-linux-gnu" "-aux-triple" "powerpc64le--linux" "-emit-obj" {{.*}}"-pic-level" "2" {{.*}}"-fopenmp"  {{.*}}"-o" "
 // CHK-COMMANDS-SAME: [[T2OBJ:[^\\/]+\.o]]" "-x" "c" "{{.*}}[[INPUT]]" "-fopenmp-is-device" "-fopenmp-host-ir-file-path" "{{.*}}[[HOSTBC]]"
 // CHK-COMMANDS: ld{{(\.exe)?}}" {{.*}}"-o" "
 // CHK-COMMANDS-SAME: [[T2BIN:[^\\/]+\.out]]" {{.*}}"{{.*}}[[T2OBJ]]"
-// CHK-COMMANDS-ST: clang{{.*}}" "-cc1" "-triple" "x86_64-pc-linux-gnu" "-E" {{.*}}"-fopenmp" {{.*}}"-o" "
+// CHK-COMMANDS-ST: clang{{.*}}" "-cc1" "-triple" "x86_64-pc-linux-gnu" "-aux-triple" "powerpc64le--linux" "-E" {{.*}}"-fopenmp" {{.*}}"-o" "
 // CHK-COMMANDS-ST-SAME: [[T2PP:[^\\/]+\.i]]" "-x" "c" "{{.*}}[[INPUT]]"
-// CHK-COMMANDS-ST: clang{{.*}}" "-cc1" "-triple" "x86_64-pc-linux-gnu" "-emit-llvm-bc" {{.*}}"-pic-level" "2" {{.*}}"-fopenmp" {{.*}}"-o" "
+// CHK-COMMANDS-ST: clang{{.*}}" "-cc1" "-triple" "x86_64-pc-linux-gnu" "-aux-triple" "powerpc64le--linux" "-emit-llvm-bc" {{.*}}"-pic-level" "2" {{.*}}"-fopenmp" {{.*}}"-o" "
 // CHK-COMMANDS-ST-SAME: [[T2BC:[^\\/]+\.bc]]" "-x" "cpp-output" "{{.*}}[[T2PP]]" "-fopenmp-is-device" "-fopenmp-host-ir-file-path" "{{.*}}[[HOSTBC]]"
-// CHK-COMMANDS-ST: clang{{.*}}" "-cc1" "-triple" "x86_64-pc-linux-gnu" "-S" {{.*}}"-fopenmp" {{.*}}"-o" "
+// CHK-COMMANDS-ST: clang{{.*}}" "-cc1" "-triple" "x86_64-pc-linux-gnu" "-aux-triple" "powerpc64le--linux" "-S" {{.*}}"-fopenmp" {{.*}}"-o" "
 // CHK-COMMANDS-ST-SAME: [[T2ASM:[^\\/]+\.s]]" "-x" "ir" "{{.*}}[[T2BC]]"
 // CHK-COMMANDS-ST: clang{{.*}}" "-cc1as" "-triple" "x86_64-pc-linux-gnu" "-filetype" "obj" {{.*}}"-o" "
 // CHK-COMMANDS-ST-SAME: [[T2OBJ:[^\\/]+\.o]]" "{{.*}}[[T2ASM]]"
@@ -396,25 +396,25 @@
 // CHK-BUJOBS-ST-SAME: [[HOSTBC:[^\\/]+\.bc]]" "-x" "cpp-output" "{{.*}}[[HOSTPP]]" "-fopenmp-targets=powerpc64le-ibm-linux-gnu,x86_64-pc-linux-gnu"
 
 // Create target 1 object.
-// 

r293668 - [windows] [asan] Add wholearchive flag when including static lib asan.

[Marcos Pividori via cfe-commits]

Author: mpividori
Date: Tue Jan 31 12:31:38 2017
New Revision: 293668

URL: http://llvm.org/viewvc/llvm-project?rev=293668=rev
Log:
[windows] [asan] Add wholearchive flag when including static lib asan.

In Windows, when the sanitizer is implemented as a static library, we use
auxiliary static library dll_thunk that will be linked to the dlls that have
instrumentation, so they can refer to the runtime in the main executable.
It uses interception to get a pointer the function in the main executable and
override its function with that pointer.
Because of that, we need to ensure that the main executable exports all the
sanitizers' interface, otherwise the initialization in dll_thunk will fail.

In this commit we add the flag -wholearchive to clang driver to ensure that
the linker does not omit any object files from asan library.

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

Modified:
cfe/trunk/lib/Driver/Tools.cpp

Modified: cfe/trunk/lib/Driver/Tools.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=293668=293667=293668=diff
==
--- cfe/trunk/lib/Driver/Tools.cpp (original)
+++ cfe/trunk/lib/Driver/Tools.cpp Tue Jan 31 12:31:38 2017
@@ -11002,6 +11002,11 @@ void visualstudio::Linker::ConstructJob(
 } else {
   for (const auto  : {"asan", "asan_cxx"})
 CmdArgs.push_back(TC.getCompilerRTArgString(Args, Lib));
+  // Make sure the linker consider all object files from the static 
library.
+  // This is necessary because instrumented dlls need access to all the
+  // interface exported by the static lib in the main executable.
+  CmdArgs.push_back(Args.MakeArgString(std::string("-wholearchive:") +
+  TC.getCompilerRT(Args, "asan")));
 }
   }
 


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

Re: r292194 - [AST] AttributedType should derive type properties from the EquivalentType

[Hans Wennborg via cfe-commits]

Ping?

On Thu, Jan 26, 2017 at 3:42 PM, Hans Wennborg  wrote:
> Should we merge this to the release branch?
>
> On Mon, Jan 16, 2017 at 8:14 PM, David Majnemer via cfe-commits
>  wrote:
>> Author: majnemer
>> Date: Mon Jan 16 22:14:25 2017
>> New Revision: 292194
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=292194=rev
>> Log:
>> [AST] AttributedType should derive type properties from the EquivalentType
>>
>> Using the canonical type instead of the equivalent type can result in
>> insufficient template instantiations.
>>
>> This fixes PR31656.
>>
>> Differential Revision: https://reviews.llvm.org/D28788
>>
>> Modified:
>> cfe/trunk/include/clang/AST/Type.h
>> cfe/trunk/test/CodeGenCXX/microsoft-abi-default-cc.cpp
>>
>> Modified: cfe/trunk/include/clang/AST/Type.h
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Type.h?rev=292194=292193=292194=diff
>> ==
>> --- cfe/trunk/include/clang/AST/Type.h (original)
>> +++ cfe/trunk/include/clang/AST/Type.h Mon Jan 16 22:14:25 2017
>> @@ -3832,13 +3832,13 @@ private:
>>
>>friend class ASTContext; // creates these
>>
>> -  AttributedType(QualType canon, Kind attrKind,
>> - QualType modified, QualType equivalent)
>> -: Type(Attributed, canon, canon->isDependentType(),
>> -   canon->isInstantiationDependentType(),
>> -   canon->isVariablyModifiedType(),
>> -   canon->containsUnexpandedParameterPack()),
>> -  ModifiedType(modified), EquivalentType(equivalent) {
>> +  AttributedType(QualType canon, Kind attrKind, QualType modified,
>> + QualType equivalent)
>> +  : Type(Attributed, canon, equivalent->isDependentType(),
>> + equivalent->isInstantiationDependentType(),
>> + equivalent->isVariablyModifiedType(),
>> + equivalent->containsUnexpandedParameterPack()),
>> +ModifiedType(modified), EquivalentType(equivalent) {
>>  AttributedTypeBits.AttrKind = attrKind;
>>}
>>
>>
>> Modified: cfe/trunk/test/CodeGenCXX/microsoft-abi-default-cc.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/microsoft-abi-default-cc.cpp?rev=292194=292193=292194=diff
>> ==
>> --- cfe/trunk/test/CodeGenCXX/microsoft-abi-default-cc.cpp (original)
>> +++ cfe/trunk/test/CodeGenCXX/microsoft-abi-default-cc.cpp Mon Jan 16 
>> 22:14:25 2017
>> @@ -45,3 +45,12 @@ void __cdecl static_baz() {}
>>  void static_qux() {}
>>  // GCABI-LABEL: define void @_Z10static_quxv
>>  // MSABI: define void @"\01?static_qux@@YAXXZ"
>> +
>> +namespace PR31656 {
>> +template 
>> +void __cdecl callee(int args[I]);
>> +// GCABI-LABEL: declare void @_ZN7PR316566calleeILi1EEEvPi(
>> +// MSABI: declare void @"\01??$callee@$00@PR31656@@YAXQAH@Z"(
>> +
>> +void caller() { callee<1>(0); }
>> +}
>>
>>
>> ___
>> 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: [libcxxabi] r292638 - Fix catch_reference_nullptr.pass.cpp test for GCC.

[Hans Wennborg via cfe-commits]

On Thu, Jan 26, 2017 at 10:23 AM, Hans Wennborg  wrote:
> On Fri, Jan 20, 2017 at 1:52 PM, Renato Golin  wrote:
>> On 20 January 2017 at 19:46, Eric Fiselier  wrote:
>>> This patch fixes a libc++abi test failure when compiled with GCC 5, 6, or 7.
>>> We should merge this into 4.0 to help get `check-all` clean.
>>
>> Hi Eric,
>>
>> All good on my side, pass with GCC 5.4 and 6.3.
>>
>> Can you attach all commits that need to be backported into a bug
>> against PR31622?
>>
>> We should probably squash them all before merge.
>
> What's the status here?

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

Re: r291963 - [clang] Emit `diagnose_if` warnings from system headers

[Hans Wennborg via cfe-commits]

Ping?

On Thu, Jan 26, 2017 at 10:21 AM, Hans Wennborg  wrote:
> Ping?
>
> On Mon, Jan 23, 2017 at 4:27 PM, Hans Wennborg  wrote:
>> Ping?
>>
>> On Tue, Jan 17, 2017 at 4:16 PM, Hans Wennborg  wrote:
>>> Richard, what do you think?
>>>
>>> On Fri, Jan 13, 2017 at 3:16 PM, Eric Fiselier  wrote:
 I would love to see this merged. It would make it easier to write libc++
 tests if the tests didn't have to worry about the old 4.0 behavior.

 CC'ing Richard: Would merging this be OK?

 On Fri, Jan 13, 2017 at 3:46 PM, George Burgess IV
  wrote:
>
> Do we want to consider merging this into the release branch? Seems like
> more of a bugfix than a feature to me.
>
> On Fri, Jan 13, 2017 at 2:11 PM, Eric Fiselier via cfe-commits
>  wrote:
>>
>> Author: ericwf
>> Date: Fri Jan 13 16:11:40 2017
>> New Revision: 291963
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=291963=rev
>> Log:
>> [clang] Emit `diagnose_if` warnings from system headers
>>
>> Summary: In order for libc++ to meaningfully use `diagnose_if` warnings
>> they need to be emitted from system headers by default. This patch 
>> changes
>> the `diagnose_if` warning diagnostic to be shown in system headers.
>>
>> Reviewers: george.burgess.iv, rsmith, aaron.ballman
>>
>> Subscribers: cfe-commits
>>
>> Differential Revision: https://reviews.llvm.org/D28703
>>
>> Added:
>> cfe/trunk/test/Sema/Inputs/diagnose-if-warn-system-header.h
>> Modified:
>> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> cfe/trunk/test/Sema/diagnose_if.c
>>
>> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=291963=291962=291963=diff
>>
>> ==
>> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
>> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Jan 13
>> 16:11:40 2017
>> @@ -3380,7 +3380,8 @@ def note_ovl_candidate_has_pass_object_s
>>  "candidate address cannot be taken because parameter %0 has "
>>  "pass_object_size attribute">;
>>  def err_diagnose_if_succeeded : Error<"%0">;
>> -def warn_diagnose_if_succeeded : Warning<"%0">,
>> InGroup;
>> +def warn_diagnose_if_succeeded : Warning<"%0">,
>> InGroup,
>> +ShowInSystemHeader;
>>  def note_ovl_candidate_disabled_by_function_cond_attr : Note<
>>  "candidate disabled: %0">;
>>  def note_ovl_candidate_disabled_by_extension : Note<
>>
>> Added: cfe/trunk/test/Sema/Inputs/diagnose-if-warn-system-header.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/Inputs/diagnose-if-warn-system-header.h?rev=291963=auto
>>
>> ==
>> --- cfe/trunk/test/Sema/Inputs/diagnose-if-warn-system-header.h (added)
>> +++ cfe/trunk/test/Sema/Inputs/diagnose-if-warn-system-header.h Fri Jan
>> 13 16:11:40 2017
>> @@ -0,0 +1,11 @@
>> +#pragma GCC system_header
>> +
>> +inline int system_header_func(int x)
>> +  __attribute__((diagnose_if(x == x, "system header warning",
>> "warning"))) // expected-note {{from 'diagnose_if' attribute}}
>> +{
>> +  return 0;
>> +}
>> +
>> +void test_system_header() {
>> +  system_header_func(0); // expected-warning {{system header warning}}
>> +}
>>
>> Modified: cfe/trunk/test/Sema/diagnose_if.c
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/diagnose_if.c?rev=291963=291962=291963=diff
>>
>> ==
>> --- cfe/trunk/test/Sema/diagnose_if.c (original)
>> +++ cfe/trunk/test/Sema/diagnose_if.c Fri Jan 13 16:11:40 2017
>> @@ -150,3 +150,6 @@ void alwaysWarnWithArg(int a) _diagnose_
>>  void runAlwaysWarnWithArg(int a) {
>>alwaysWarnWithArg(a); // expected-warning{{alwaysWarn}}
>>  }
>> +
>> +// Test that diagnose_if warnings generated in system headers are not
>> ignored.
>> +#include "Inputs/diagnose-if-warn-system-header.h"
>>
>>
>> ___
>> 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: r293416 - [c-index-test] Provide capability for 'c-index-test core' to dump symbol information from a PCH/module file.

[Argyrios Kyrtzidis via cfe-commits]

Hi Hans,

> On Jan 31, 2017, at 9:08 AM, Hans Wennborg  wrote:
> 
> Hi Argyrios,
> 
> Can you provide more background on what this does. Is it fixing an
> existing issue or doing something new?

On second thought, let’s not get this into the stable branch, this is 
introducing something new, not fixing an existing issue.

> 
> r293461 is making changes beyond Index/ so I'd like Richard to take a look 
> too.
> 
> Thanks,
> Hans
> 
> On Mon, Jan 30, 2017 at 8:18 AM, Argyrios Kyrtzidis  wrote:
>> Hi Hans,
>> 
>> Could this go into the stable branch, along with the follow-ups:
>> r293461
>> r293463
>> r293466
>> 
>>> On Jan 28, 2017, at 8:50 PM, Argyrios Kyrtzidis via cfe-commits 
>>>  wrote:
>>> 
>>> Author: akirtzidis
>>> Date: Sat Jan 28 22:50:35 2017
>>> New Revision: 293416
>>> 
>>> URL: http://llvm.org/viewvc/llvm-project?rev=293416=rev
>>> Log:
>>> [c-index-test] Provide capability for 'c-index-test core' to dump symbol 
>>> information from a PCH/module file.
>>> 
>>> Added:
>>>   cfe/trunk/test/Index/Core/index-pch.c
>>> Modified:
>>>   cfe/trunk/tools/c-index-test/CMakeLists.txt
>>>   cfe/trunk/tools/c-index-test/core_main.cpp
>>> 
>>> Added: cfe/trunk/test/Index/Core/index-pch.c
>>> URL: 
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/Core/index-pch.c?rev=293416=auto
>>> ==
>>> --- cfe/trunk/test/Index/Core/index-pch.c (added)
>>> +++ cfe/trunk/test/Index/Core/index-pch.c Sat Jan 28 22:50:35 2017
>>> @@ -0,0 +1,13 @@
>>> +// RUN: c-index-test core -print-source-symbols -- %s | FileCheck %s
>>> +// RUN: %clang_cc1 -emit-pch %s -o %t.pch
>>> +// RUN: c-index-test core -print-source-symbols -module-file %t.pch | 
>>> FileCheck %s
>>> +
>>> +// CHECK: [[@LINE+1]]:6 | function/C | test1 | [[TEST1_USR:.*]] | 
>>> [[TEST1_CG:.*]] | Decl | rel: 0
>>> +void test1();
>>> +
>>> +// CHECK: [[@LINE+1]]:20 | function/C | test2 | [[TEST2_USR:.*]] | {{.*}} 
>>> | Def | rel: 0
>>> +static inline void test2() {
>>> +  // CHECK: [[@LINE+2]]:3 | function/C | test1 | [[TEST1_USR]] | 
>>> [[TEST1_CG]] | Ref,Call,RelCall,RelCont | rel: 1
>>> +  // CHECK-NEXT: RelCall,RelCont | test2 | [[TEST2_USR]]
>>> +  test1();
>>> +}
>>> 
>>> Modified: cfe/trunk/tools/c-index-test/CMakeLists.txt
>>> URL: 
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/c-index-test/CMakeLists.txt?rev=293416=293415=293416=diff
>>> ==
>>> --- cfe/trunk/tools/c-index-test/CMakeLists.txt (original)
>>> +++ cfe/trunk/tools/c-index-test/CMakeLists.txt Sat Jan 28 22:50:35 2017
>>> @@ -24,6 +24,7 @@ else()
>>>libclang
>>>clangAST
>>>clangBasic
>>> +clangCodeGen
>>>clangFrontend
>>>clangIndex
>>>  )
>>> 
>>> Modified: cfe/trunk/tools/c-index-test/core_main.cpp
>>> URL: 
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/c-index-test/core_main.cpp?rev=293416=293415=293416=diff
>>> ==
>>> --- cfe/trunk/tools/c-index-test/core_main.cpp (original)
>>> +++ cfe/trunk/tools/c-index-test/core_main.cpp Sat Jan 28 22:50:35 2017
>>> @@ -7,6 +7,7 @@
>>> //
>>> //===--===//
>>> 
>>> +#include "clang/CodeGen/ObjectFilePCHContainerOperations.h"
>>> #include "clang/Frontend/ASTUnit.h"
>>> #include "clang/Frontend/CompilerInstance.h"
>>> #include "clang/Frontend/CompilerInvocation.h"
>>> @@ -49,6 +50,13 @@ static cl::extrahelp MoreHelp(
>>>  "invocation\n"
>>> );
>>> 
>>> +static cl::opt
>>> +ModuleFilePath("module-file",
>>> +   cl::desc("Path to module file to print symbols from"));
>>> +static cl::opt
>>> +  ModuleFormat("fmodule-format", cl::init("raw"),
>>> +cl::desc("Container format for clang modules and PCH, 'raw' or 
>>> 'obj'"));
>>> +
>>> }
>>> } // anonymous namespace
>>> 
>>> @@ -160,6 +168,39 @@ static bool printSourceSymbols(ArrayRef<
>>>  return false;
>>> }
>>> 
>>> +static bool printSourceSymbolsFromModule(StringRef modulePath,
>>> + StringRef format) {
>>> +  FileSystemOptions FileSystemOpts;
>>> +  auto pchContOps = std::make_shared();
>>> +  // Register the support for object-file-wrapped Clang modules.
>>> +  
>>> pchContOps->registerReader(llvm::make_unique());
>>> +  auto pchRdr = pchContOps->getReaderOrNull(format);
>>> +  if (!pchRdr) {
>>> +errs() << "unknown module format: " << format << '\n';
>>> +return true;
>>> +  }
>>> +
>>> +  IntrusiveRefCntPtr Diags =
>>> +  CompilerInstance::createDiagnostics(new DiagnosticOptions());
>>> +  std::unique_ptr AU = ASTUnit::LoadFromASTFile(
>>> +  modulePath, *pchRdr, Diags,
>>> +  FileSystemOpts, /*UseDebugInfo=*/false,
>>> +  /*OnlyLocalDecls=*/true, None,
>>> +  /*CaptureDiagnostics=*/false,
>>> +  

[PATCH] D27985: Add demangling support for C++11 thread_local variables

[Dave Bozier via Phabricator via cfe-commits]

davidb closed this revision.
davidb added a comment.

Closed by revision r293638.


Repository:
  rL LLVM

https://reviews.llvm.org/D27985



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

[PATCH] D16135: Macro Debug Info support in Clang

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added inline comments.



Comment at: lib/CodeGen/MacroPPCallbacks.cpp:62
+: DebugInfo(DI), PP(PP), FirstInclude(false), FirstIncludeDone(false),
+  CommandIncludeFiles(0), SkipFiles(2) {
+  Parents.push_back(nullptr);

aprantl wrote:
> Comment why SkipFiles is initialized to 2?
Thanks. I think this comment should now go into the header and be promoted to 
the Docygen comment for the CommandIncludeFiles. Also consider doing 
`CommandIncludeFiles = 0` in the class definition.



Comment at: lib/CodeGen/MacroPPCallbacks.cpp:31
+  MacroInfo::arg_iterator AI = MI.arg_begin(), E = MI.arg_end();
+  for (; AI + 1 != E; ++AI) {
+Name << (*AI)->getName();

std::next() ?



Comment at: lib/CodeGen/MacroPPCallbacks.cpp:44
+if (MI.isGNUVarargs())
+  Name << "..."; // #define foo(x...)
+

LLVM style prefers comments to be full sentences and above the code.



Comment at: lib/CodeGen/MacroPPCallbacks.cpp:83
+  break;
+  // Fall though!
+  case MainFileScope:

I think there is an LLVM_FALLTHROUGH macro (or something to that end)



Comment at: lib/CodeGen/MacroPPCallbacks.cpp:154
+StringRef SearchPath, StringRef RelativePath, const Module *Imported) {
+  // Only care about "include" directives.
+  if (!IncludeTok.is(tok::identifier))

#include as opposed to #import? If so, why?



Comment at: lib/CodeGen/ModuleBuilder.cpp:68
 SmallVector DeferredInlineMethodDefinitions;
+CGDebugInfo *DebugInfoRef;
 

What does Ref stand for?


https://reviews.llvm.org/D16135



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

Re: r293596 - Handle ObjCEncodeExpr in extractStringLiteralCharacter.

[Hans Wennborg via cfe-commits]

Merged in r293653.

Thanks,
Hans

On Mon, Jan 30, 2017 at 7:16 PM, Akira Hatanaka  wrote:
> Hans, can this be merged to 4.0? This fixes a regression from 3.9 (the part I 
> touched was committed a few years ago, but didn’t cause an assert until 
> recently).
>
>> On Jan 30, 2017, at 6:31 PM, Akira Hatanaka via cfe-commits 
>>  wrote:
>>
>> Author: ahatanak
>> Date: Mon Jan 30 20:31:39 2017
>> New Revision: 293596
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=293596=rev
>> Log:
>> Handle ObjCEncodeExpr in extractStringLiteralCharacter.
>>
>> This fixes an assertion failure that occurs later in the function when
>> an ObjCEncodeExpr is cast to StringLiteral.
>>
>> rdar://problem/30111207
>>
>> Modified:
>>cfe/trunk/lib/AST/ExprConstant.cpp
>>cfe/trunk/test/CodeGenObjC/encode-test.m
>>
>> Modified: cfe/trunk/lib/AST/ExprConstant.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=293596=293595=293596=diff
>> ==
>> --- cfe/trunk/lib/AST/ExprConstant.cpp (original)
>> +++ cfe/trunk/lib/AST/ExprConstant.cpp Mon Jan 30 20:31:39 2017
>> @@ -2394,7 +2394,14 @@ static unsigned getBaseIndex(const CXXRe
>> /// Extract the value of a character from a string literal.
>> static APSInt extractStringLiteralCharacter(EvalInfo , const Expr *Lit,
>> uint64_t Index) {
>> -  // FIXME: Support ObjCEncodeExpr, MakeStringConstant
>> +  // FIXME: Support MakeStringConstant
>> +  if (const auto *ObjCEnc = dyn_cast(Lit)) {
>> +std::string Str;
>> +Info.Ctx.getObjCEncodingForType(ObjCEnc->getEncodedType(), Str);
>> +assert(Index <= Str.size() && "Index too large");
>> +return APSInt::getUnsigned(Str.c_str()[Index]);
>> +  }
>> +
>>   if (auto PE = dyn_cast(Lit))
>> Lit = PE->getFunctionName();
>>   const StringLiteral *S = cast(Lit);
>>
>> Modified: cfe/trunk/test/CodeGenObjC/encode-test.m
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/encode-test.m?rev=293596=293595=293596=diff
>> ==
>> --- cfe/trunk/test/CodeGenObjC/encode-test.m (original)
>> +++ cfe/trunk/test/CodeGenObjC/encode-test.m Mon Jan 30 20:31:39 2017
>> @@ -180,3 +180,14 @@ const char g14[] = @encode(__typeof__(*t
>>
>> // CHECK: @g15 = constant [2 x i8] c":\00"
>> const char g15[] = @encode(SEL);
>> +
>> +typedef typeof(sizeof(int)) size_t;
>> +size_t strlen(const char *s);
>> +
>> +// CHECK-LABEL: @test_strlen(
>> +// CHECK: %[[i:.*]] = alloca i32
>> +// CHECK: store i32 1, i32* %[[i]]
>> +void test_strlen() {
>> +  const char array[] = @encode(int);
>> +  int i = strlen(array);
>> +}
>>
>>
>> ___
>> 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] D28001: [X86] Teach Clang about -mfentry flag

[Nirav Dave via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL293649: [X86] Teach Clang about -mfentry flag (authored by 
niravd).

Changed prior to commit:
  https://reviews.llvm.org/D28001?vs=83850=86442#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D28001

Files:
  cfe/trunk/include/clang/Driver/Options.td
  cfe/trunk/include/clang/Frontend/CodeGenOptions.def
  cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
  cfe/trunk/lib/Driver/Tools.cpp
  cfe/trunk/lib/Frontend/CompilerInvocation.cpp
  cfe/trunk/test/CodeGen/fentry.c


Index: cfe/trunk/test/CodeGen/fentry.c
===
--- cfe/trunk/test/CodeGen/fentry.c
+++ cfe/trunk/test/CodeGen/fentry.c
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -pg -mfentry -triple i386-unknown-unknown -emit-llvm -o - 
%s | FileCheck  %s
+// RUN: %clang_cc1 -pg -mfentry -triple x86_64-unknown-linux-gnu -emit-llvm -o 
- %s | FileCheck %s
+// RUN: %clang_cc1 -mfentry -triple i386-unknown-unknown -emit-llvm -o - %s | 
FileCheck -check-prefix=NOPG %s
+// RUN: %clang_cc1 -mfentry -triple x86_64-unknown-linux-gnu -emit-llvm -o - 
%s | FileCheck -check-prefix=NOPG %s
+
+int foo(void) {
+  return 0;
+}
+
+//CHECK: attributes #{{[0-9]+}} = { {{.*}}"fentry-call"="true"{{.*}} }
+//NOPG-NOT: attributes #{{[0-9]+}} = { {{.*}}"fentry-call"{{.*}} }
Index: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
===
--- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
@@ -860,8 +860,12 @@
   // inlining, we just add an attribute to insert a mcount call in backend.
   // The attribute "counting-function" is set to mcount function name which is
   // architecture dependent.
-  if (CGM.getCodeGenOpts().InstrumentForProfiling)
-Fn->addFnAttr("counting-function", getTarget().getMCountName());
+  if (CGM.getCodeGenOpts().InstrumentForProfiling) {
+if (CGM.getCodeGenOpts().CallFEntry)
+  Fn->addFnAttr("fentry-call", "true");
+else
+  Fn->addFnAttr("counting-function", getTarget().getMCountName());
+  }
 
   if (RetTy->isVoidType()) {
 // Void type; nothing to return.
Index: cfe/trunk/lib/Driver/Tools.cpp
===
--- cfe/trunk/lib/Driver/Tools.cpp
+++ cfe/trunk/lib/Driver/Tools.cpp
@@ -5483,6 +5483,9 @@
   if (getToolChain().SupportsProfiling())
 Args.AddLastArg(CmdArgs, options::OPT_pg);
 
+  if (getToolChain().SupportsProfiling())
+Args.AddLastArg(CmdArgs, options::OPT_mfentry);
+
   // -flax-vector-conversions is default.
   if (!Args.hasFlag(options::OPT_flax_vector_conversions,
 options::OPT_fno_lax_vector_conversions))
Index: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
===
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp
@@ -717,6 +717,7 @@
   Opts.XRayInstructionThreshold =
   getLastArgIntValue(Args, OPT_fxray_instruction_threshold_, 200, Diags);
   Opts.InstrumentForProfiling = Args.hasArg(OPT_pg);
+  Opts.CallFEntry = Args.hasArg(OPT_mfentry);
   Opts.EmitOpenCLArgMetadata = Args.hasArg(OPT_cl_kernel_arg_info);
   Opts.CompressDebugSections = Args.hasArg(OPT_compress_debug_sections);
   Opts.RelaxELFRelocations = Args.hasArg(OPT_mrelax_relocations);
Index: cfe/trunk/include/clang/Frontend/CodeGenOptions.def
===
--- cfe/trunk/include/clang/Frontend/CodeGenOptions.def
+++ cfe/trunk/include/clang/Frontend/CodeGenOptions.def
@@ -83,6 +83,7 @@
 VALUE_CODEGENOPT(XRayInstructionThreshold , 32, 200)
 
 CODEGENOPT(InstrumentForProfiling , 1, 0) ///< Set when -pg is enabled.
+CODEGENOPT(CallFEntry , 1, 0) ///< Set when -mfentry is enabled.
 CODEGENOPT(LessPreciseFPMAD  , 1, 0) ///< Enable less precise MAD instructions 
to
  ///< be generated.
 CODEGENOPT(PrepareForLTO , 1, 0) ///< Set when -flto is enabled on the
Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -1858,6 +1858,8 @@
   Flags<[CC1Option]>,
   HelpText<"Use copy relocations support for PIE builds">;
 def mno_pie_copy_relocations : Flag<["-"], "mno-pie-copy-relocations">, 
Group;
+def mfentry : Flag<["-"], "mfentry">, HelpText<"insert calls to fentry at 
function entry (x86 only)">,
+  Flags<[CC1Option]>, Group;
 def mx87 : Flag<["-"], "mx87">, Group;
 def m80387 : Flag<["-"], "m80387">, Alias;
 def msse2 : Flag<["-"], "msse2">, Group;


Index: cfe/trunk/test/CodeGen/fentry.c
===
--- cfe/trunk/test/CodeGen/fentry.c
+++ cfe/trunk/test/CodeGen/fentry.c
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -pg -mfentry -triple 

r293649 - [X86] Teach Clang about -mfentry flag

[Nirav Dave via cfe-commits]

Author: niravd
Date: Tue Jan 31 11:00:35 2017
New Revision: 293649

URL: http://llvm.org/viewvc/llvm-project?rev=293649=rev
Log:
[X86] Teach Clang about -mfentry flag

Replace mcount calls with calls to fentry.

Reviewers: hfinkel, craig.topper

Subscribers: llvm-commits

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

Added:
cfe/trunk/test/CodeGen/fentry.c
Modified:
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/include/clang/Frontend/CodeGenOptions.def
cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
cfe/trunk/lib/Driver/Tools.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=293649=293648=293649=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Tue Jan 31 11:00:35 2017
@@ -1858,6 +1858,8 @@ def mpie_copy_relocations : Flag<["-"],
   Flags<[CC1Option]>,
   HelpText<"Use copy relocations support for PIE builds">;
 def mno_pie_copy_relocations : Flag<["-"], "mno-pie-copy-relocations">, 
Group;
+def mfentry : Flag<["-"], "mfentry">, HelpText<"insert calls to fentry at 
function entry (x86 only)">,
+  Flags<[CC1Option]>, Group;
 def mx87 : Flag<["-"], "mx87">, Group;
 def m80387 : Flag<["-"], "m80387">, Alias;
 def msse2 : Flag<["-"], "msse2">, Group;

Modified: cfe/trunk/include/clang/Frontend/CodeGenOptions.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/CodeGenOptions.def?rev=293649=293648=293649=diff
==
--- cfe/trunk/include/clang/Frontend/CodeGenOptions.def (original)
+++ cfe/trunk/include/clang/Frontend/CodeGenOptions.def Tue Jan 31 11:00:35 2017
@@ -83,6 +83,7 @@ CODEGENOPT(XRayInstrumentFunctions , 1,
 VALUE_CODEGENOPT(XRayInstructionThreshold , 32, 200)
 
 CODEGENOPT(InstrumentForProfiling , 1, 0) ///< Set when -pg is enabled.
+CODEGENOPT(CallFEntry , 1, 0) ///< Set when -mfentry is enabled.
 CODEGENOPT(LessPreciseFPMAD  , 1, 0) ///< Enable less precise MAD instructions 
to
  ///< be generated.
 CODEGENOPT(PrepareForLTO , 1, 0) ///< Set when -flto is enabled on the

Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.cpp?rev=293649=293648=293649=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Tue Jan 31 11:00:35 2017
@@ -860,8 +860,12 @@ void CodeGenFunction::StartFunction(Glob
   // inlining, we just add an attribute to insert a mcount call in backend.
   // The attribute "counting-function" is set to mcount function name which is
   // architecture dependent.
-  if (CGM.getCodeGenOpts().InstrumentForProfiling)
-Fn->addFnAttr("counting-function", getTarget().getMCountName());
+  if (CGM.getCodeGenOpts().InstrumentForProfiling) {
+if (CGM.getCodeGenOpts().CallFEntry)
+  Fn->addFnAttr("fentry-call", "true");
+else
+  Fn->addFnAttr("counting-function", getTarget().getMCountName());
+  }
 
   if (RetTy->isVoidType()) {
 // Void type; nothing to return.

Modified: cfe/trunk/lib/Driver/Tools.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=293649=293648=293649=diff
==
--- cfe/trunk/lib/Driver/Tools.cpp (original)
+++ cfe/trunk/lib/Driver/Tools.cpp Tue Jan 31 11:00:35 2017
@@ -5483,6 +5483,9 @@ void Clang::ConstructJob(Compilation ,
   if (getToolChain().SupportsProfiling())
 Args.AddLastArg(CmdArgs, options::OPT_pg);
 
+  if (getToolChain().SupportsProfiling())
+Args.AddLastArg(CmdArgs, options::OPT_mfentry);
+
   // -flax-vector-conversions is default.
   if (!Args.hasFlag(options::OPT_flax_vector_conversions,
 options::OPT_fno_lax_vector_conversions))

Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=293649=293648=293649=diff
==
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Tue Jan 31 11:00:35 2017
@@ -717,6 +717,7 @@ static bool ParseCodeGenArgs(CodeGenOpti
   Opts.XRayInstructionThreshold =
   getLastArgIntValue(Args, OPT_fxray_instruction_threshold_, 200, Diags);
   Opts.InstrumentForProfiling = Args.hasArg(OPT_pg);
+  Opts.CallFEntry = Args.hasArg(OPT_mfentry);
   Opts.EmitOpenCLArgMetadata = Args.hasArg(OPT_cl_kernel_arg_info);
   Opts.CompressDebugSections = Args.hasArg(OPT_compress_debug_sections);
   Opts.RelaxELFRelocations = 

Re: r293416 - [c-index-test] Provide capability for 'c-index-test core' to dump symbol information from a PCH/module file.

[Hans Wennborg via cfe-commits]

Hi Argyrios,

Can you provide more background on what this does. Is it fixing an
existing issue or doing something new?

r293461 is making changes beyond Index/ so I'd like Richard to take a look too.

Thanks,
Hans

On Mon, Jan 30, 2017 at 8:18 AM, Argyrios Kyrtzidis  wrote:
> Hi Hans,
>
> Could this go into the stable branch, along with the follow-ups:
> r293461
> r293463
> r293466
>
>> On Jan 28, 2017, at 8:50 PM, Argyrios Kyrtzidis via cfe-commits 
>>  wrote:
>>
>> Author: akirtzidis
>> Date: Sat Jan 28 22:50:35 2017
>> New Revision: 293416
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=293416=rev
>> Log:
>> [c-index-test] Provide capability for 'c-index-test core' to dump symbol 
>> information from a PCH/module file.
>>
>> Added:
>>cfe/trunk/test/Index/Core/index-pch.c
>> Modified:
>>cfe/trunk/tools/c-index-test/CMakeLists.txt
>>cfe/trunk/tools/c-index-test/core_main.cpp
>>
>> Added: cfe/trunk/test/Index/Core/index-pch.c
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/Core/index-pch.c?rev=293416=auto
>> ==
>> --- cfe/trunk/test/Index/Core/index-pch.c (added)
>> +++ cfe/trunk/test/Index/Core/index-pch.c Sat Jan 28 22:50:35 2017
>> @@ -0,0 +1,13 @@
>> +// RUN: c-index-test core -print-source-symbols -- %s | FileCheck %s
>> +// RUN: %clang_cc1 -emit-pch %s -o %t.pch
>> +// RUN: c-index-test core -print-source-symbols -module-file %t.pch | 
>> FileCheck %s
>> +
>> +// CHECK: [[@LINE+1]]:6 | function/C | test1 | [[TEST1_USR:.*]] | 
>> [[TEST1_CG:.*]] | Decl | rel: 0
>> +void test1();
>> +
>> +// CHECK: [[@LINE+1]]:20 | function/C | test2 | [[TEST2_USR:.*]] | {{.*}} | 
>> Def | rel: 0
>> +static inline void test2() {
>> +  // CHECK: [[@LINE+2]]:3 | function/C | test1 | [[TEST1_USR]] | 
>> [[TEST1_CG]] | Ref,Call,RelCall,RelCont | rel: 1
>> +  // CHECK-NEXT: RelCall,RelCont | test2 | [[TEST2_USR]]
>> +  test1();
>> +}
>>
>> Modified: cfe/trunk/tools/c-index-test/CMakeLists.txt
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/c-index-test/CMakeLists.txt?rev=293416=293415=293416=diff
>> ==
>> --- cfe/trunk/tools/c-index-test/CMakeLists.txt (original)
>> +++ cfe/trunk/tools/c-index-test/CMakeLists.txt Sat Jan 28 22:50:35 2017
>> @@ -24,6 +24,7 @@ else()
>> libclang
>> clangAST
>> clangBasic
>> +clangCodeGen
>> clangFrontend
>> clangIndex
>>   )
>>
>> Modified: cfe/trunk/tools/c-index-test/core_main.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/c-index-test/core_main.cpp?rev=293416=293415=293416=diff
>> ==
>> --- cfe/trunk/tools/c-index-test/core_main.cpp (original)
>> +++ cfe/trunk/tools/c-index-test/core_main.cpp Sat Jan 28 22:50:35 2017
>> @@ -7,6 +7,7 @@
>> //
>> //===--===//
>>
>> +#include "clang/CodeGen/ObjectFilePCHContainerOperations.h"
>> #include "clang/Frontend/ASTUnit.h"
>> #include "clang/Frontend/CompilerInstance.h"
>> #include "clang/Frontend/CompilerInvocation.h"
>> @@ -49,6 +50,13 @@ static cl::extrahelp MoreHelp(
>>   "invocation\n"
>> );
>>
>> +static cl::opt
>> +ModuleFilePath("module-file",
>> +   cl::desc("Path to module file to print symbols from"));
>> +static cl::opt
>> +  ModuleFormat("fmodule-format", cl::init("raw"),
>> +cl::desc("Container format for clang modules and PCH, 'raw' or 
>> 'obj'"));
>> +
>> }
>> } // anonymous namespace
>>
>> @@ -160,6 +168,39 @@ static bool printSourceSymbols(ArrayRef<
>>   return false;
>> }
>>
>> +static bool printSourceSymbolsFromModule(StringRef modulePath,
>> + StringRef format) {
>> +  FileSystemOptions FileSystemOpts;
>> +  auto pchContOps = std::make_shared();
>> +  // Register the support for object-file-wrapped Clang modules.
>> +  
>> pchContOps->registerReader(llvm::make_unique());
>> +  auto pchRdr = pchContOps->getReaderOrNull(format);
>> +  if (!pchRdr) {
>> +errs() << "unknown module format: " << format << '\n';
>> +return true;
>> +  }
>> +
>> +  IntrusiveRefCntPtr Diags =
>> +  CompilerInstance::createDiagnostics(new DiagnosticOptions());
>> +  std::unique_ptr AU = ASTUnit::LoadFromASTFile(
>> +  modulePath, *pchRdr, Diags,
>> +  FileSystemOpts, /*UseDebugInfo=*/false,
>> +  /*OnlyLocalDecls=*/true, None,
>> +  /*CaptureDiagnostics=*/false,
>> +  /*AllowPCHWithCompilerErrors=*/true,
>> +  /*UserFilesAreVolatile=*/false);
>> +  if (!AU) {
>> +errs() << "failed to create TU for: " << modulePath << '\n';
>> +return true;
>> +  }
>> +
>> +  auto DataConsumer = std::make_shared(outs());
>> +  IndexingOptions IndexOpts;
>> +  indexASTUnit(*AU, DataConsumer, IndexOpts);
>> +
>> +  return false;
>> +}
>> +

Re: [libcxx] r293581 - Revert "Adorn __call_once_proxy with `inline` and `_LIBCPP_INLINE_VISIBILITY`"

[Hans Wennborg via cfe-commits]

Merged to 4.0 as discussed on PR31782.

On Mon, Jan 30, 2017 at 5:26 PM, Justin Bogner via cfe-commits
 wrote:
> Author: bogner
> Date: Mon Jan 30 19:26:09 2017
> New Revision: 293581
>
> URL: http://llvm.org/viewvc/llvm-project?rev=293581=rev
> Log:
> Revert "Adorn __call_once_proxy with `inline` and `_LIBCPP_INLINE_VISIBILITY`"
>
> While this change didn't really hurt, it does lead to spurious
> warnings about not being able to override weak symbols if you end up
> linking objects built with this change to ones built without it.
> Furthermore, since __call_once_proxy is called indirectly anyway it
> doesn't actually inline ever.
>
> Longer term, it would probably make sense to give this symbol internal
> visibility instead.
>
> This reverts r291497
>
> Modified:
> libcxx/trunk/include/mutex
>
> Modified: libcxx/trunk/include/mutex
> URL: 
> http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/mutex?rev=293581=293580=293581=diff
> ==
> --- libcxx/trunk/include/mutex (original)
> +++ libcxx/trunk/include/mutex Mon Jan 30 19:26:09 2017
> @@ -559,7 +559,6 @@ public:
>  #endif
>
>  template 
> -inline _LIBCPP_INLINE_VISIBILITY
>  void
>  __call_once_proxy(void* __vp)
>  {
>
>
> ___
> 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

[libcxx] r293645 - Merging r293581:

[Hans Wennborg via cfe-commits]

Author: hans
Date: Tue Jan 31 10:40:22 2017
New Revision: 293645

URL: http://llvm.org/viewvc/llvm-project?rev=293645=rev
Log:
Merging r293581:

r293581 | bogner | 2017-01-30 17:26:09 -0800 (Mon, 30 Jan 2017) | 12 lines

Revert "Adorn __call_once_proxy with `inline` and `_LIBCPP_INLINE_VISIBILITY`"

While this change didn't really hurt, it does lead to spurious
warnings about not being able to override weak symbols if you end up
linking objects built with this change to ones built without it.
Furthermore, since __call_once_proxy is called indirectly anyway it
doesn't actually inline ever.

Longer term, it would probably make sense to give this symbol internal
visibility instead.

This reverts r291497


Modified:
libcxx/branches/release_40/   (props changed)
libcxx/branches/release_40/include/mutex

Propchange: libcxx/branches/release_40/
--
--- svn:mergeinfo (original)
+++ svn:mergeinfo Tue Jan 31 10:40:22 2017
@@ -1,2 +1,2 @@
 /libcxx/branches/apple:136569-137939
-/libcxx/trunk:292013,292091,292607,292990,293154,293197
+/libcxx/trunk:292013,292091,292607,292990,293154,293197,293581

Modified: libcxx/branches/release_40/include/mutex
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/branches/release_40/include/mutex?rev=293645=293644=293645=diff
==
--- libcxx/branches/release_40/include/mutex (original)
+++ libcxx/branches/release_40/include/mutex Tue Jan 31 10:40:22 2017
@@ -559,7 +559,6 @@ public:
 #endif
 
 template 
-inline _LIBCPP_INLINE_VISIBILITY
 void
 __call_once_proxy(void* __vp)
 {


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

r293641 - [clang-format] Fix regression about adding leading whitespace to the content of line comments

[Krasimir Georgiev via cfe-commits]

Author: krasimir
Date: Tue Jan 31 09:40:15 2017
New Revision: 293641

URL: http://llvm.org/viewvc/llvm-project?rev=293641=rev
Log:
[clang-format] Fix regression about adding leading whitespace to the content of 
line comments

Summary:
The reflower didn't measure precisely the line column of a line in the middle of
a line comment section that has a prefix that needs to be adapted.

source:
```
/// a
//b
```

format before:
```
/// a
 //b
```

format after:
```
/// a
// b
```

Reviewers: djasper

Reviewed By: djasper

Subscribers: sammccall, cfe-commits, klimek

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

Modified:
cfe/trunk/lib/Format/BreakableToken.cpp
cfe/trunk/unittests/Format/FormatTest.cpp

Modified: cfe/trunk/lib/Format/BreakableToken.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/BreakableToken.cpp?rev=293641=293640=293641=diff
==
--- cfe/trunk/lib/Format/BreakableToken.cpp (original)
+++ cfe/trunk/lib/Format/BreakableToken.cpp Tue Jan 31 09:40:15 2017
@@ -796,10 +796,13 @@ void BreakableLineCommentSection::replac
 } else {
   // This is the first line for the current token, but no reflow with the
   // previous token is necessary. However, we still may need to adjust the
-  // start column.
+  // start column. Note that ContentColumn[LineIndex] is the expected
+  // content column after a possible update to the prefix, hence the prefix
+  // length change is included.
   unsigned LineColumn =
   ContentColumn[LineIndex] -
-  (Content[LineIndex].data() - Lines[LineIndex].data());
+  (Content[LineIndex].data() - Lines[LineIndex].data()) +
+  (OriginalPrefix[LineIndex].size() - Prefix[LineIndex].size());
   if (tokenAt(LineIndex).OriginalColumn != LineColumn) {
 Whitespaces.replaceWhitespace(*Tokens[LineIndex],
   /*Newlines=*/1,
@@ -813,13 +816,14 @@ void BreakableLineCommentSection::replac
 /*InPPDirective=*/false);
   }
 }
-  } else if (OriginalPrefix[LineIndex] != Prefix[LineIndex]) {
-// This is not the first line of the token. Adjust the prefix if necessary.
+  }
+  if (OriginalPrefix[LineIndex] != Prefix[LineIndex]) {
+// Adjust the prefix if necessary.
 
 // Take care of the space possibly introduced after a decoration.
 assert(Prefix[LineIndex] == (OriginalPrefix[LineIndex] + " ").str() &&
-   "Expecting a block comment decoration to differ from original by "
-   "at most a space");
+   "Expecting a line comment prefix to differ from original by at most 
"
+   "a space");
 Whitespaces.replaceWhitespaceInToken(
 tokenAt(LineIndex), OriginalPrefix[LineIndex].size(), 0, "", "",
 /*InPPDirective=*/false, /*Newlines=*/0, /*Spaces=*/1);

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=293641=293640=293641=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Tue Jan 31 09:40:15 2017
@@ -1371,6 +1371,25 @@ TEST_F(FormatTest, SplitsLongCxxComments
 format("//Even if it makes the line exceed the column limit",
getLLVMStyleWithColumns(51)));
   EXPECT_EQ("//--But not here", format("//--But not here", getLLVMStyle()));
+  EXPECT_EQ("/// line 1\n"
+"// add leading whitespace",
+format("/// line 1\n"
+   "//add leading whitespace",
+   getLLVMStyleWithColumns(30)));
+  EXPECT_EQ("/// line 1\n"
+"/// line 2\n"
+"//! line 3\n"
+"//! line 4\n"
+"//! line 5\n"
+"// line 6\n"
+"// line 7",
+format("///line 1\n"
+   "///line 2\n"
+   "//! line 3\n"
+   "//!line 4\n"
+   "//!line 5\n"
+   "// line 6\n"
+   "//line 7", getLLVMStyleWithColumns(20)));
 
   EXPECT_EQ("// aa bb cc dd",
 format("// aa bb cc dd   ",


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

[PATCH] D29329: [clang-format] Fix regression about adding leading whitespace to the content of line comments

[Krasimir Georgiev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL293641: [clang-format] Fix regression about adding leading 
whitespace to the content of… (authored by krasimir).

Changed prior to commit:
  https://reviews.llvm.org/D29329?vs=86435=86437#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D29329

Files:
  cfe/trunk/lib/Format/BreakableToken.cpp
  cfe/trunk/unittests/Format/FormatTest.cpp


Index: cfe/trunk/lib/Format/BreakableToken.cpp
===
--- cfe/trunk/lib/Format/BreakableToken.cpp
+++ cfe/trunk/lib/Format/BreakableToken.cpp
@@ -796,10 +796,13 @@
 } else {
   // This is the first line for the current token, but no reflow with the
   // previous token is necessary. However, we still may need to adjust the
-  // start column.
+  // start column. Note that ContentColumn[LineIndex] is the expected
+  // content column after a possible update to the prefix, hence the prefix
+  // length change is included.
   unsigned LineColumn =
   ContentColumn[LineIndex] -
-  (Content[LineIndex].data() - Lines[LineIndex].data());
+  (Content[LineIndex].data() - Lines[LineIndex].data()) +
+  (OriginalPrefix[LineIndex].size() - Prefix[LineIndex].size());
   if (tokenAt(LineIndex).OriginalColumn != LineColumn) {
 Whitespaces.replaceWhitespace(*Tokens[LineIndex],
   /*Newlines=*/1,
@@ -813,13 +816,14 @@
 /*InPPDirective=*/false);
   }
 }
-  } else if (OriginalPrefix[LineIndex] != Prefix[LineIndex]) {
-// This is not the first line of the token. Adjust the prefix if necessary.
+  }
+  if (OriginalPrefix[LineIndex] != Prefix[LineIndex]) {
+// Adjust the prefix if necessary.
 
 // Take care of the space possibly introduced after a decoration.
 assert(Prefix[LineIndex] == (OriginalPrefix[LineIndex] + " ").str() &&
-   "Expecting a block comment decoration to differ from original by "
-   "at most a space");
+   "Expecting a line comment prefix to differ from original by at most 
"
+   "a space");
 Whitespaces.replaceWhitespaceInToken(
 tokenAt(LineIndex), OriginalPrefix[LineIndex].size(), 0, "", "",
 /*InPPDirective=*/false, /*Newlines=*/0, /*Spaces=*/1);
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -1371,6 +1371,25 @@
 format("//Even if it makes the line exceed the column limit",
getLLVMStyleWithColumns(51)));
   EXPECT_EQ("//--But not here", format("//--But not here", getLLVMStyle()));
+  EXPECT_EQ("/// line 1\n"
+"// add leading whitespace",
+format("/// line 1\n"
+   "//add leading whitespace",
+   getLLVMStyleWithColumns(30)));
+  EXPECT_EQ("/// line 1\n"
+"/// line 2\n"
+"//! line 3\n"
+"//! line 4\n"
+"//! line 5\n"
+"// line 6\n"
+"// line 7",
+format("///line 1\n"
+   "///line 2\n"
+   "//! line 3\n"
+   "//!line 4\n"
+   "//!line 5\n"
+   "// line 6\n"
+   "//line 7", getLLVMStyleWithColumns(20)));
 
   EXPECT_EQ("// aa bb cc dd",
 format("// aa bb cc dd   ",


Index: cfe/trunk/lib/Format/BreakableToken.cpp
===
--- cfe/trunk/lib/Format/BreakableToken.cpp
+++ cfe/trunk/lib/Format/BreakableToken.cpp
@@ -796,10 +796,13 @@
 } else {
   // This is the first line for the current token, but no reflow with the
   // previous token is necessary. However, we still may need to adjust the
-  // start column.
+  // start column. Note that ContentColumn[LineIndex] is the expected
+  // content column after a possible update to the prefix, hence the prefix
+  // length change is included.
   unsigned LineColumn =
   ContentColumn[LineIndex] -
-  (Content[LineIndex].data() - Lines[LineIndex].data());
+  (Content[LineIndex].data() - Lines[LineIndex].data()) +
+  (OriginalPrefix[LineIndex].size() - Prefix[LineIndex].size());
   if (tokenAt(LineIndex).OriginalColumn != LineColumn) {
 Whitespaces.replaceWhitespace(*Tokens[LineIndex],
   /*Newlines=*/1,
@@ -813,13 +816,14 @@
 /*InPPDirective=*/false);
   }
 }
-  } else if (OriginalPrefix[LineIndex] != Prefix[LineIndex]) {
-// This is not the first line of the token. Adjust the prefix if necessary.
+  }
+  if (OriginalPrefix[LineIndex] != Prefix[LineIndex]) {
+// 

[PATCH] D29329: [clang-format] Fix regression about adding leading whitespace to the content of line comments

[Daniel Jasper via Phabricator via cfe-commits]

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

Looks good.


https://reviews.llvm.org/D29329



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

[PATCH] D28771: [Analyzer] Various fixes for the IteratorPastEnd checker

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/IteratorPastEndChecker.cpp:530
+  auto value = RVal;
+  if (auto loc = value.getAs()) {
+value = State->getRawSVal(*loc);

NoQ wrote:
> Is there a test case for this hack?
> 
> I'd also consider inspecting the AST (probably before passing the values to 
> `handleRandomIncrOrDecr()`) and making the decision based on that. Because 
> even though this pattern ("if a value is a loc and we expect a nonloc, do an 
> extra dereference") is present in many places in the analyzer, in most of 
> these places it doesn't work correctly (what if we try to discriminate 
> between `int*` and `int*&`?).
I just want to get the sign of the integer value (if it is available). It 
turned out that I cannot do comparison between loc and nonloc. (Strange, 
because I can do anything else). After I created this hack, the Analyzer did 
not crash anymore on the llvm/clang code.

I do not fully understand what I should fix here and how? In this particular 
place we expect some integer, thus no int* or int*&.



Comment at: lib/StaticAnalyzer/Checkers/IteratorPastEndChecker.cpp:553
+
+  // When increasing by positive or decreasing by negative an iterator past its
+  // end, then it is a bug. We check for bugs before the operator call.

NoQ wrote:
> I think incrementing `end()` by `0` is not a bug (?)
I think it is not a bug, but how to solve it properly? If I chose just 
greaterThanZero, then we have the same problem for decrementing end() by 0. Is 
it worth to create three states here?


https://reviews.llvm.org/D28771



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

[libcxxabi] r293638 - Add support for demangling C++11 thread_local variables. In clang, the grammar for mangling for these names are " ::= TW " for wrapper variables or "

[David Bozier via cfe-commits]

Author: davidb
Date: Tue Jan 31 09:18:56 2017
New Revision: 293638

URL: http://llvm.org/viewvc/llvm-project?rev=293638=rev
Log:
Add support for demangling C++11 thread_local variables. In clang, the grammar 
for mangling for these names are " ::= TW " for 
wrapper variables or " ::= TH " for initialization 
variables.

Modified:
libcxxabi/trunk/src/cxa_demangle.cpp
libcxxabi/trunk/test/test_demangle.pass.cpp

Modified: libcxxabi/trunk/src/cxa_demangle.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxxabi/trunk/src/cxa_demangle.cpp?rev=293638=293637=293638=diff
==
--- libcxxabi/trunk/src/cxa_demangle.cpp (original)
+++ libcxxabi/trunk/src/cxa_demangle.cpp Tue Jan 31 09:18:56 2017
@@ -4345,6 +4345,8 @@ parse_call_offset(const char* first, con
 //# base is the nominal target function of thunk
 //::= GV  # Guard variable for one-time 
initialization
 // # No 
+//::= TW  # Thread-local wrapper
+//::= TH  # Thread-local initialization
 //  extension ::= TC   _  # construction 
vtable for second-in-first
 //  extension ::= GR  # reference temporary for object
 
@@ -4448,6 +4450,28 @@ parse_special_name(const char* first, co
 }
 }
 break;
+case 'W':
+// TW  # Thread-local wrapper
+t = parse_name(first + 2, last, db);
+if (t != first + 2) 
+{
+if (db.names.empty())
+return first;
+db.names.back().first.insert(0, "thread-local wrapper 
routine for ");
+first = t;
+}
+break;
+case 'H':
+//TH  # Thread-local initialization
+t = parse_name(first + 2, last, db);
+if (t != first + 2) 
+{
+if (db.names.empty())
+return first;
+db.names.back().first.insert(0, "thread-local 
initialization routine for ");
+first = t;
+}
+break;
 default:
 // T  
 {

Modified: libcxxabi/trunk/test/test_demangle.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxxabi/trunk/test/test_demangle.pass.cpp?rev=293638=293637=293638=diff
==
--- libcxxabi/trunk/test/test_demangle.pass.cpp (original)
+++ libcxxabi/trunk/test/test_demangle.pass.cpp Tue Jan 31 09:18:56 2017
@@ -29602,6 +29602,8 @@ const char* cases[][2] =
 // FIXME(compnerd) pretty print this as void (*)(unsigned long &) volatile 
&&
 {"PVFvRmOE", "void (*)(unsigned long&)  volatile&&"},
 {"PFvRmOE", "void (*)(unsigned long&) &&"},
+{"_ZTW1x", "thread-local wrapper routine for x"},
+{"_ZTHN3fooE", "thread-local initialization routine for foo"},
 };
 
 const unsigned N = sizeof(cases) / sizeof(cases[0]);


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

[PATCH] D28771: [Analyzer] Various fixes for the IteratorPastEnd checker

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 86428.
baloghadamsoftware added a comment.

Updates based on the comments.


https://reviews.llvm.org/D28771

Files:
  lib/StaticAnalyzer/Checkers/IteratorPastEndChecker.cpp
  test/Analysis/Inputs/system-header-simulator-cxx.h
  test/Analysis/diagnostics/explicit-suppression.cpp
  test/Analysis/iterator-past-end.cpp

Index: test/Analysis/iterator-past-end.cpp
===
--- test/Analysis/iterator-past-end.cpp
+++ test/Analysis/iterator-past-end.cpp
@@ -203,3 +203,42 @@
 start = ++item; // no-warning
   }
 }
+
+void good_overwrite(std::vector ) {
+  auto i = vec.end();
+  i = vec.begin();
+  *i; // no-warning
+}
+
+void good_overwrite_find(std::vector , int e) {
+  auto i = std::find(vec.begin(), vec.end(), e);
+  if(i == vec.end()) {
+i = vec.begin();
+  }
+  *i; // no-warning
+}
+
+void bad_overwrite(std::vector ) {
+  auto i = vec.begin();
+  i = vec.end();
+  *i; // expected-warning{{Iterator accessed past its end}}
+}
+
+void bad_overwrite_find(std::vector , int e) {
+  auto i = std::find(vec.begin(), vec.end(), e);
+  if(i != vec.end()) {
+i = vec.begin();
+  }
+  *i; // expected-warning{{Iterator accessed past its end}}
+}
+
+void good_advance(std::vector ) {
+  auto i = vec.end();
+  std::advance(i, -1);
+  *i; // no-warning
+}
+
+void good_prev(std::vector ) {
+  auto i = std::prev(vec.end());
+  *i; // no-warning
+}
Index: test/Analysis/diagnostics/explicit-suppression.cpp
===
--- test/Analysis/diagnostics/explicit-suppression.cpp
+++ test/Analysis/diagnostics/explicit-suppression.cpp
@@ -18,6 +18,6 @@
 void testCopyNull(C *I, C *E) {
   std::copy(I, E, (C *)0);
 #ifndef SUPPRESSED
-  // expected-warning@../Inputs/system-header-simulator-cxx.h:191 {{Called C++ object pointer is null}}
+  // expected-warning@../Inputs/system-header-simulator-cxx.h:293 {{Called C++ object pointer is null}}
 #endif
 }
Index: test/Analysis/Inputs/system-header-simulator-cxx.h
===
--- test/Analysis/Inputs/system-header-simulator-cxx.h
+++ test/Analysis/Inputs/system-header-simulator-cxx.h
@@ -8,18 +8,60 @@
 typedef unsigned char uint8_t;
 
 typedef __typeof__(sizeof(int)) size_t;
+typedef __typeof__((char*)0-(char*)0) ptrdiff_t;
 void *memmove(void *s1, const void *s2, size_t n);
 
-template  struct __iterator {
-  typedef __iterator iterator;
-  typedef __iterator const_iterator;
+namespace std {
+  struct input_iterator_tag { };
+  struct output_iterator_tag { };
+  struct forward_iterator_tag : public input_iterator_tag { };
+  struct bidirectional_iterator_tag : public forward_iterator_tag { };
+  struct random_access_iterator_tag : public bidirectional_iterator_tag { };
+
+  template  struct iterator_traits {
+typedef typename Iterator::difference_type difference_type;
+typedef typename Iterator::value_type value_type;
+typedef typename Iterator::pointer pointer;
+typedef typename Iterator::reference reference;
+typedef typename Iterator::iterator_category iterator_category;
+  };
+}
 
-  __iterator(const Ptr p) : ptr(p) {}
+template  struct __vector_iterator {
+  typedef __vector_iterator iterator;
+  typedef __vector_iterator const_iterator;
+
+  typedef ptrdiff_t difference_type;
+  typedef T value_type;
+  typedef Ptr pointer;
+  typedef Ref reference;
+  typedef std::random_access_iterator_tag iterator_category;
+
+  __vector_iterator(const Ptr p) : ptr(p) {}
+  __vector_iterator operator++() { ++ ptr; return *this; }
+  __vector_iterator operator++(int) {
+auto tmp = *this;
+++ ptr;
+return tmp;
+  }
+  __vector_iterator operator--() { -- ptr; return *this; }
+  __vector_iterator operator--(int) {
+auto tmp = *this; -- ptr;
+return tmp;
+  }
+  __vector_iterator operator+(difference_type n) {
+return ptr + n;
+  }
+  __vector_iterator operator-(difference_type n) {
+return ptr - n;
+  }
+  __vector_iterator operator+=(difference_type n) {
+return ptr += n;
+  }
+  __vector_iterator operator-=(difference_type n) {
+return ptr -= n;
+  }
 
-  __iterator operator++() { return *this; }
-  __iterator operator++(int) { return *this; }
-  __iterator operator--() { return *this; }
-  __iterator operator--(int) { return *this; }
   Ref operator*() const { return *ptr; }
   Ptr operator->() const { return *ptr; }
 
@@ -33,7 +75,45 @@
   Ptr ptr;
 };
 
+template  struct __list_iterator {
+  typedef __vector_iterator iterator;
+  typedef __vector_iterator const_iterator;
+
+  typedef 

r293636 - clang-format: [JS] Indent expressions in ${} relative to their surrounding

[Daniel Jasper via cfe-commits]

Author: djasper
Date: Tue Jan 31 08:39:33 2017
New Revision: 293636

URL: http://llvm.org/viewvc/llvm-project?rev=293636=rev
Log:
clang-format: [JS] Indent expressions in ${} relative to their surrounding

This only affects expressions inside ${} scopes of template strings.
Here, we want to indent relative to the surrounding template string and
not the surrounding expression. Otherwise, this can create quite a mess.

Before:
  var f = `
aa: ${someFunction(
  a +  //
  )}`;

After:
  var f = `
aa: ${someFunction(
  a +  //
  )}`;

Modified:
cfe/trunk/lib/Format/ContinuationIndenter.cpp
cfe/trunk/unittests/Format/FormatTestJS.cpp

Modified: cfe/trunk/lib/Format/ContinuationIndenter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/ContinuationIndenter.cpp?rev=293636=293635=293636=diff
==
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp (original)
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp Tue Jan 31 08:39:33 2017
@@ -985,12 +985,23 @@ void ContinuationIndenter::moveStatePast
 //   int> v);
 // FIXME: We likely want to do this for more combinations of brackets.
 // Verify that it is wanted for ObjC, too.
-if (Current.Tok.getKind() == tok::less &&
-Current.ParentBracket == tok::l_paren) {
+if (Current.is(tok::less) && Current.ParentBracket == tok::l_paren) {
   NewIndent = std::max(NewIndent, State.Stack.back().Indent);
   LastSpace = std::max(LastSpace, State.Stack.back().Indent);
 }
 
+// JavaScript template strings are special as we always want to indent
+// nested expressions relative to the ${}. Otherwise, this can create quite
+// a mess.
+if (Current.is(TT_TemplateString)) {
+  unsigned Column = Current.IsMultiline
+? Current.LastLineColumnWidth
+: State.Column + Current.ColumnWidth;
+  NewIndent = Column;
+  LastSpace = Column;
+  NestedBlockIndent = Column;
+}
+
 AvoidBinPacking =
 (State.Line->MustBeDeclaration && !Style.BinPackParameters) ||
 (!State.Line->MustBeDeclaration && !Style.BinPackArguments) ||

Modified: cfe/trunk/unittests/Format/FormatTestJS.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestJS.cpp?rev=293636=293635=293636=diff
==
--- cfe/trunk/unittests/Format/FormatTestJS.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestJS.cpp Tue Jan 31 08:39:33 2017
@@ -1401,6 +1401,63 @@ TEST_F(FormatTestJS, TemplateStrings) {
" a:${  aaa. a} `;");
 }
 
+TEST_F(FormatTestJS, TemplateStringMultiLineExpression) {
+  verifyFormat("var f = `aa: ${a +  //\n"
+   "   }`;",
+   "var f = `aa: ${a +  //\n"
+   "   }`;");
+  verifyFormat("var f = `aa: ${ //\n"
+   "   a +  //\n"
+   "   }`;",
+   "var f = `aa: ${ //\n"
+   "   a +  //\n"
+   "   }`;");
+  verifyFormat("var f = `\n"
+   "  aa: ${a +  //\n"
+   "}`;",
+   "var f  =  `\n"
+   "  aa: ${   a  +  //\n"
+   " }`;");
+  verifyFormat("var f = `\n"
+   "  aa: ${ //\n"
+   "a +  //\n"
+   "}`;",
+   "var f  =  `\n"
+   "  aa: ${ //\n"
+   "a  +  //\n"
+   "}`  ;");
+  verifyFormat("var f = `\n"
+   "  aa: ${someFunction(\n"
+   "a +  //\n"
+   ")}`;",
+   "var f  =  `\n"
+   "  aa: ${ someFunction  (\n"
+   "a  +   //\n"
+   ")}`;");
+  verifyFormat("var f = `\n"
+   "  aa: ${  //\n"
+   "someFunction(\n"
+   "a +  //\n"
+   ")}`;",
+   "var f  =  `\n"
+   "  aa: ${  //\n"
+   " 

[PATCH] D29326: [clang-format] Fix reflow in block comment lines with leading whitespace.

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added a comment.

@djasper: I agree with the two spaces rule. Sounds pretty reasonable. Will add 
this feature eventually.


Repository:
  rL LLVM

https://reviews.llvm.org/D29326



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

r293633 - [clang-format] Fix reflow in block comment lines with leading whitespace.

[Krasimir Georgiev via cfe-commits]

Author: krasimir
Date: Tue Jan 31 08:31:44 2017
New Revision: 293633

URL: http://llvm.org/viewvc/llvm-project?rev=293633=rev
Log:
[clang-format] Fix reflow in block comment lines with leading whitespace.

Summary:
The reflower was not taking into account the additional  leading whitespace in 
block comment lines.

source:
```
{
/*
 * long long long long
 *   long
 * long long long long
 */
}
```

format (with column limit 20) before:
```
{
  /*
   * long long long
   * long long long long
   * long long
   */
}
```
format after:
```
{
  /*
   * long long long
   * long long long
   * long long long
   */
}
```

Reviewers: djasper, klimek

Reviewed By: djasper

Subscribers: cfe-commits, sammccall, klimek

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

Modified:
cfe/trunk/lib/Format/BreakableToken.cpp
cfe/trunk/unittests/Format/FormatTest.cpp

Modified: cfe/trunk/lib/Format/BreakableToken.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/BreakableToken.cpp?rev=293633=293632=293633=diff
==
--- cfe/trunk/lib/Format/BreakableToken.cpp (original)
+++ cfe/trunk/lib/Format/BreakableToken.cpp Tue Jan 31 08:31:44 2017
@@ -521,10 +521,15 @@ unsigned BreakableBlockComment::getLineL
 unsigned PreviousEndColumn,
 unsigned ColumnLimit,
 Split SplitBefore) const {
-if (SplitBefore.first == StringRef::npos ||
-SplitBefore.first + SplitBefore.second < Content[LineIndex].size()) {
-  // A piece of line, not the whole, gets reflown.
-  return getLineLengthAfterSplit(LineIndex, TailOffset, StringRef::npos);
+  if (SplitBefore.first == StringRef::npos ||
+  // Block comment line contents contain the trailing whitespace after the
+  // decoration, so the need of left trim. Note that this behavior is
+  // consistent with the breaking of block comments where the indentation 
of
+  // a broken line is uniform across all the lines of the block comment.
+  SplitBefore.first + SplitBefore.second <
+  Content[LineIndex].ltrim().size()) {
+// A piece of line, not the whole, gets reflown.
+return getLineLengthAfterSplit(LineIndex, TailOffset, StringRef::npos);
   } else {
 // The whole line gets reflown, need to check if we need to insert a break
 // for the postfix or not.

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=293633=293632=293633=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Tue Jan 31 08:31:44 2017
@@ -1960,6 +1960,23 @@ TEST_F(FormatTest, ReflowsComments) {
" * longg */",
getLLVMStyleWithColumns(20)));
 
+  // Reflow lines with leading whitespace.
+  EXPECT_EQ("{\n"
+"  /*\n"
+"   * long long long\n"
+"   * long long long\n"
+"   * long long long\n"
+"   */\n"
+"}",
+format("{\n"
+   "/*\n"
+   " * long long long long\n"
+   " *   long\n"
+   " * long long long long\n"
+   " */\n"
+   "}",
+   getLLVMStyleWithColumns(20)));
+
   // Break single line block comments that are first in the line with ' *'
   // decoration.
   EXPECT_EQ("/* long long long\n"


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

[PATCH] D29326: [clang-format] Fix reflow in block comment lines with leading whitespace.

[Krasimir Georgiev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL293633: [clang-format] Fix reflow in block comment lines 
with leading whitespace. (authored by krasimir).

Changed prior to commit:
  https://reviews.llvm.org/D29326?vs=86419=86422#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D29326

Files:
  cfe/trunk/lib/Format/BreakableToken.cpp
  cfe/trunk/unittests/Format/FormatTest.cpp


Index: cfe/trunk/lib/Format/BreakableToken.cpp
===
--- cfe/trunk/lib/Format/BreakableToken.cpp
+++ cfe/trunk/lib/Format/BreakableToken.cpp
@@ -521,10 +521,15 @@
 unsigned PreviousEndColumn,
 unsigned ColumnLimit,
 Split SplitBefore) const {
-if (SplitBefore.first == StringRef::npos ||
-SplitBefore.first + SplitBefore.second < Content[LineIndex].size()) {
-  // A piece of line, not the whole, gets reflown.
-  return getLineLengthAfterSplit(LineIndex, TailOffset, StringRef::npos);
+  if (SplitBefore.first == StringRef::npos ||
+  // Block comment line contents contain the trailing whitespace after the
+  // decoration, so the need of left trim. Note that this behavior is
+  // consistent with the breaking of block comments where the indentation 
of
+  // a broken line is uniform across all the lines of the block comment.
+  SplitBefore.first + SplitBefore.second <
+  Content[LineIndex].ltrim().size()) {
+// A piece of line, not the whole, gets reflown.
+return getLineLengthAfterSplit(LineIndex, TailOffset, StringRef::npos);
   } else {
 // The whole line gets reflown, need to check if we need to insert a break
 // for the postfix or not.
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -1960,6 +1960,23 @@
" * longg */",
getLLVMStyleWithColumns(20)));
 
+  // Reflow lines with leading whitespace.
+  EXPECT_EQ("{\n"
+"  /*\n"
+"   * long long long\n"
+"   * long long long\n"
+"   * long long long\n"
+"   */\n"
+"}",
+format("{\n"
+   "/*\n"
+   " * long long long long\n"
+   " *   long\n"
+   " * long long long long\n"
+   " */\n"
+   "}",
+   getLLVMStyleWithColumns(20)));
+
   // Break single line block comments that are first in the line with ' *'
   // decoration.
   EXPECT_EQ("/* long long long\n"


Index: cfe/trunk/lib/Format/BreakableToken.cpp
===
--- cfe/trunk/lib/Format/BreakableToken.cpp
+++ cfe/trunk/lib/Format/BreakableToken.cpp
@@ -521,10 +521,15 @@
 unsigned PreviousEndColumn,
 unsigned ColumnLimit,
 Split SplitBefore) const {
-if (SplitBefore.first == StringRef::npos ||
-SplitBefore.first + SplitBefore.second < Content[LineIndex].size()) {
-  // A piece of line, not the whole, gets reflown.
-  return getLineLengthAfterSplit(LineIndex, TailOffset, StringRef::npos);
+  if (SplitBefore.first == StringRef::npos ||
+  // Block comment line contents contain the trailing whitespace after the
+  // decoration, so the need of left trim. Note that this behavior is
+  // consistent with the breaking of block comments where the indentation of
+  // a broken line is uniform across all the lines of the block comment.
+  SplitBefore.first + SplitBefore.second <
+  Content[LineIndex].ltrim().size()) {
+// A piece of line, not the whole, gets reflown.
+return getLineLengthAfterSplit(LineIndex, TailOffset, StringRef::npos);
   } else {
 // The whole line gets reflown, need to check if we need to insert a break
 // for the postfix or not.
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -1960,6 +1960,23 @@
" * longg */",
getLLVMStyleWithColumns(20)));
 
+  // Reflow lines with leading whitespace.
+  EXPECT_EQ("{\n"
+"  /*\n"
+"   * long long long\n"
+"   * long long long\n"
+"   * long long long\n"
+"   */\n"
+"}",
+format("{\n"
+   "/*\n"
+   " * long long long long\n"
+   " *   long\n"
+   " * long long long long\n"
+   " */\n"
+   "}",
+   getLLVMStyleWithColumns(20)));
+
   // Break single line block comments that are first in the line with ' *'
   // decoration.
   EXPECT_EQ("/* long long long\n"

[PATCH] D29304: [cmake] Hint find_package() to prefer LLVM installed alongside clang

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rL293632: [cmake] Hint find_package() to prefer LLVM installed 
alongside clang (authored by mgorny).

Changed prior to commit:
  https://reviews.llvm.org/D29304?vs=86339=86421#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D29304

Files:
  cfe/trunk/cmake/modules/CMakeLists.txt
  cfe/trunk/cmake/modules/ClangConfig.cmake.in


Index: cfe/trunk/cmake/modules/ClangConfig.cmake.in
===
--- cfe/trunk/cmake/modules/ClangConfig.cmake.in
+++ cfe/trunk/cmake/modules/ClangConfig.cmake.in
@@ -1,9 +1,10 @@
 # This file allows users to call find_package(Clang) and pick up our targets.
 
-find_package(LLVM REQUIRED CONFIG)
-
 @CLANG_CONFIG_CODE@
 
+find_package(LLVM REQUIRED CONFIG
+ HINTS "@CLANG_CONFIG_LLVM_CMAKE_DIR@")
+
 set(CLANG_EXPORTED_TARGETS "@CLANG_EXPORTS@")
 set(CLANG_CMAKE_DIR "@CLANG_CONFIG_CMAKE_DIR@")
 
Index: cfe/trunk/cmake/modules/CMakeLists.txt
===
--- cfe/trunk/cmake/modules/CMakeLists.txt
+++ cfe/trunk/cmake/modules/CMakeLists.txt
@@ -4,17 +4,23 @@
 set(CLANG_INSTALL_PACKAGE_DIR lib${LLVM_LIBDIR_SUFFIX}/cmake/clang)
 set(clang_cmake_builddir "${CMAKE_BINARY_DIR}/${CLANG_INSTALL_PACKAGE_DIR}")
 
+# Keep this in sync with llvm/cmake/CMakeLists.txt!
+set(LLVM_INSTALL_PACKAGE_DIR lib${LLVM_LIBDIR_SUFFIX}/cmake/llvm)
+set(llvm_cmake_builddir "${LLVM_BINARY_DIR}/${LLVM_INSTALL_PACKAGE_DIR}")
+
 get_property(CLANG_EXPORTS GLOBAL PROPERTY CLANG_EXPORTS)
 export(TARGETS ${CLANG_EXPORTS} FILE 
${clang_cmake_builddir}/ClangTargets.cmake)
 
 # Generate ClangConfig.cmake for the build tree.
 set(CLANG_CONFIG_CMAKE_DIR "${clang_cmake_builddir}")
+set(CLANG_CONFIG_LLVM_CMAKE_DIR "${llvm_cmake_builddir}")
 set(CLANG_CONFIG_EXPORTS_FILE "${clang_cmake_builddir}/ClangTargets.cmake")
 configure_file(
   ${CMAKE_CURRENT_SOURCE_DIR}/ClangConfig.cmake.in
   ${clang_cmake_builddir}/ClangConfig.cmake
   @ONLY)
 set(CLANG_CONFIG_CMAKE_DIR)
+set(CLANG_CONFIG_LLVM_CMAKE_DIR)
 set(CLANG_CONFIG_EXPORTS_FILE)
 
 # Generate ClangConfig.cmake for the install tree.
@@ -29,6 +35,7 @@
 get_filename_component(CLANG_INSTALL_PREFIX \"\${CLANG_INSTALL_PREFIX}\" 
PATH)")
 endforeach(p)
 set(CLANG_CONFIG_CMAKE_DIR 
"\${CLANG_INSTALL_PREFIX}/${CLANG_INSTALL_PACKAGE_DIR}")
+set(CLANG_CONFIG_LLVM_CMAKE_DIR 
"\${CLANG_INSTALL_PREFIX}/${LLVM_INSTALL_PACKAGE_DIR}")
 set(CLANG_CONFIG_EXPORTS_FILE "\${CLANG_CMAKE_DIR}/ClangTargets.cmake")
 configure_file(
   ${CMAKE_CURRENT_SOURCE_DIR}/ClangConfig.cmake.in


Index: cfe/trunk/cmake/modules/ClangConfig.cmake.in
===
--- cfe/trunk/cmake/modules/ClangConfig.cmake.in
+++ cfe/trunk/cmake/modules/ClangConfig.cmake.in
@@ -1,9 +1,10 @@
 # This file allows users to call find_package(Clang) and pick up our targets.
 
-find_package(LLVM REQUIRED CONFIG)
-
 @CLANG_CONFIG_CODE@
 
+find_package(LLVM REQUIRED CONFIG
+ HINTS "@CLANG_CONFIG_LLVM_CMAKE_DIR@")
+
 set(CLANG_EXPORTED_TARGETS "@CLANG_EXPORTS@")
 set(CLANG_CMAKE_DIR "@CLANG_CONFIG_CMAKE_DIR@")
 
Index: cfe/trunk/cmake/modules/CMakeLists.txt
===
--- cfe/trunk/cmake/modules/CMakeLists.txt
+++ cfe/trunk/cmake/modules/CMakeLists.txt
@@ -4,17 +4,23 @@
 set(CLANG_INSTALL_PACKAGE_DIR lib${LLVM_LIBDIR_SUFFIX}/cmake/clang)
 set(clang_cmake_builddir "${CMAKE_BINARY_DIR}/${CLANG_INSTALL_PACKAGE_DIR}")
 
+# Keep this in sync with llvm/cmake/CMakeLists.txt!
+set(LLVM_INSTALL_PACKAGE_DIR lib${LLVM_LIBDIR_SUFFIX}/cmake/llvm)
+set(llvm_cmake_builddir "${LLVM_BINARY_DIR}/${LLVM_INSTALL_PACKAGE_DIR}")
+
 get_property(CLANG_EXPORTS GLOBAL PROPERTY CLANG_EXPORTS)
 export(TARGETS ${CLANG_EXPORTS} FILE ${clang_cmake_builddir}/ClangTargets.cmake)
 
 # Generate ClangConfig.cmake for the build tree.
 set(CLANG_CONFIG_CMAKE_DIR "${clang_cmake_builddir}")
+set(CLANG_CONFIG_LLVM_CMAKE_DIR "${llvm_cmake_builddir}")
 set(CLANG_CONFIG_EXPORTS_FILE "${clang_cmake_builddir}/ClangTargets.cmake")
 configure_file(
   ${CMAKE_CURRENT_SOURCE_DIR}/ClangConfig.cmake.in
   ${clang_cmake_builddir}/ClangConfig.cmake
   @ONLY)
 set(CLANG_CONFIG_CMAKE_DIR)
+set(CLANG_CONFIG_LLVM_CMAKE_DIR)
 set(CLANG_CONFIG_EXPORTS_FILE)
 
 # Generate ClangConfig.cmake for the install tree.
@@ -29,6 +35,7 @@
 get_filename_component(CLANG_INSTALL_PREFIX \"\${CLANG_INSTALL_PREFIX}\" PATH)")
 endforeach(p)
 set(CLANG_CONFIG_CMAKE_DIR "\${CLANG_INSTALL_PREFIX}/${CLANG_INSTALL_PACKAGE_DIR}")
+set(CLANG_CONFIG_LLVM_CMAKE_DIR "\${CLANG_INSTALL_PREFIX}/${LLVM_INSTALL_PACKAGE_DIR}")
 set(CLANG_CONFIG_EXPORTS_FILE "\${CLANG_CMAKE_DIR}/ClangTargets.cmake")
 configure_file(
   ${CMAKE_CURRENT_SOURCE_DIR}/ClangConfig.cmake.in
___
cfe-commits mailing list
cfe-commits@lists.llvm.org

r293632 - [cmake] Hint find_package() to prefer LLVM installed alongside clang

[Michal Gorny via cfe-commits]

Author: mgorny
Date: Tue Jan 31 08:15:40 2017
New Revision: 293632

URL: http://llvm.org/viewvc/llvm-project?rev=293632=rev
Log:
[cmake] Hint find_package() to prefer LLVM installed alongside clang

Include a path hint for find_package() in ClangConfig.cmake to ensure
that CMake prefers LLVM installed alongside clang over the default
search path.

If two versions of LLVM are installed in the system, and one of them is
in PATH, CMake's find_package() magic prefers the CMake directory
alongside that install by default. Adding a relative hint makes it
possible to prioritize to the install from which find_package() is
called.

If you want to build e.g. LLDB against another install of LLVM, you can
pass LLVM_CONFIG override. In this case, LLDB queries the prefix from
llvm-config and uses the CMake files located there. However, when
including ClangConfig, the implicit find_package() nevertheless prefers
PATH-found LLVM over the one used previously by LLDB, and two versions
of LLVMConfig end up being loaded.

This could be fixed on LLDB end up by explicitly forcing custom package
search location. However, it seems simpler and safer to add a hint to
ClangConfig than to track every usage of ClangConfig.

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

Modified:
cfe/trunk/cmake/modules/CMakeLists.txt
cfe/trunk/cmake/modules/ClangConfig.cmake.in

Modified: cfe/trunk/cmake/modules/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/cmake/modules/CMakeLists.txt?rev=293632=293631=293632=diff
==
--- cfe/trunk/cmake/modules/CMakeLists.txt (original)
+++ cfe/trunk/cmake/modules/CMakeLists.txt Tue Jan 31 08:15:40 2017
@@ -4,17 +4,23 @@
 set(CLANG_INSTALL_PACKAGE_DIR lib${LLVM_LIBDIR_SUFFIX}/cmake/clang)
 set(clang_cmake_builddir "${CMAKE_BINARY_DIR}/${CLANG_INSTALL_PACKAGE_DIR}")
 
+# Keep this in sync with llvm/cmake/CMakeLists.txt!
+set(LLVM_INSTALL_PACKAGE_DIR lib${LLVM_LIBDIR_SUFFIX}/cmake/llvm)
+set(llvm_cmake_builddir "${LLVM_BINARY_DIR}/${LLVM_INSTALL_PACKAGE_DIR}")
+
 get_property(CLANG_EXPORTS GLOBAL PROPERTY CLANG_EXPORTS)
 export(TARGETS ${CLANG_EXPORTS} FILE 
${clang_cmake_builddir}/ClangTargets.cmake)
 
 # Generate ClangConfig.cmake for the build tree.
 set(CLANG_CONFIG_CMAKE_DIR "${clang_cmake_builddir}")
+set(CLANG_CONFIG_LLVM_CMAKE_DIR "${llvm_cmake_builddir}")
 set(CLANG_CONFIG_EXPORTS_FILE "${clang_cmake_builddir}/ClangTargets.cmake")
 configure_file(
   ${CMAKE_CURRENT_SOURCE_DIR}/ClangConfig.cmake.in
   ${clang_cmake_builddir}/ClangConfig.cmake
   @ONLY)
 set(CLANG_CONFIG_CMAKE_DIR)
+set(CLANG_CONFIG_LLVM_CMAKE_DIR)
 set(CLANG_CONFIG_EXPORTS_FILE)
 
 # Generate ClangConfig.cmake for the install tree.
@@ -29,6 +35,7 @@ foreach(p ${_count})
 get_filename_component(CLANG_INSTALL_PREFIX \"\${CLANG_INSTALL_PREFIX}\" 
PATH)")
 endforeach(p)
 set(CLANG_CONFIG_CMAKE_DIR 
"\${CLANG_INSTALL_PREFIX}/${CLANG_INSTALL_PACKAGE_DIR}")
+set(CLANG_CONFIG_LLVM_CMAKE_DIR 
"\${CLANG_INSTALL_PREFIX}/${LLVM_INSTALL_PACKAGE_DIR}")
 set(CLANG_CONFIG_EXPORTS_FILE "\${CLANG_CMAKE_DIR}/ClangTargets.cmake")
 configure_file(
   ${CMAKE_CURRENT_SOURCE_DIR}/ClangConfig.cmake.in

Modified: cfe/trunk/cmake/modules/ClangConfig.cmake.in
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/cmake/modules/ClangConfig.cmake.in?rev=293632=293631=293632=diff
==
--- cfe/trunk/cmake/modules/ClangConfig.cmake.in (original)
+++ cfe/trunk/cmake/modules/ClangConfig.cmake.in Tue Jan 31 08:15:40 2017
@@ -1,9 +1,10 @@
 # This file allows users to call find_package(Clang) and pick up our targets.
 
-find_package(LLVM REQUIRED CONFIG)
-
 @CLANG_CONFIG_CODE@
 
+find_package(LLVM REQUIRED CONFIG
+ HINTS "@CLANG_CONFIG_LLVM_CMAKE_DIR@")
+
 set(CLANG_EXPORTED_TARGETS "@CLANG_EXPORTS@")
 set(CLANG_CMAKE_DIR "@CLANG_CONFIG_CMAKE_DIR@")
 


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

[clang-format] add option -fallback-style=fail (fail if -style=file falls back)

[Paul Seyfert via cfe-commits]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256

Hi,
(resend to cfe-commits instead of cfe-dev)

please see my feature proposal for an option -fallback-style=fail below.

Thanks in advance for feedback,
Paul

-  Forwarded Message 
Subject: [cfe-dev] [clang-format] add option -fallback-style=fail (fail if 
-style=file falls back)
Date: Thu, 26 Jan 2017 16:05:32 +0100
From: Paul Seyfert via cfe-dev 
Reply-To: Paul Seyfert 
To: cfe-...@lists.llvm.org

Dear all,

I want to provide information to the clang-format user if the style
is/can be used, or if clang-format sees itself forced to fall back to
the fallback-style. I.e. in contrast to doing nothing, as with
- -fallback-style=none, I want clang-format to fail. Kind of as a reminder
to the user "hey, maybe you forgot to add your .clang-format file to
your project".

I was thinking about doing this through an option
`-fallback-style=fail`. Do you think that is the right approach?

If yes, I gave it a try and wrote the attached one-off patch, which adds
the option `-fallback-style=fail`.

I *briefly* tested it on my system by building `make clang-format`[1] and
tried what happens when no .clang-format file exists and I run
`clang-format -style=file -fallback-style=fail main.cc` on a minimal
hello world c++ file.

NB: the patch does contain an update of the unit test, but I did not
manage to run it (problems with the system I was running on …).

Thanks in advance for feedback,

Paul



For reference:
This addresses what I asked on Stackoverflow
http://stackoverflow.com/questions/41758865/determine-if-clang-format-finds-clang-format-file-in-a-shell-or-cmake-script

and is a duplication of my pull request on github
https://github.com/llvm-mirror/clang/pull/21

[1]: when following steps 1-3 from
https://clang.llvm.org/get_started.html and then running `make
clang-format` in step 7 instead of `make`.
-BEGIN PGP SIGNATURE-
Version: GnuPG v2

iQIcBAEBCAAGBQJYkJhtAAoJEPOmP9OjPXmrVs8QAJKM1RWO2f+w2OOe4EckyPcK
093d7zwr37r21TFGVzLh76JAhhswahB93l4lugoYVDl4n9L2TF5mi/7SMapkxmeo
3VnNN+VlyuCDzIo0Mj8tFCPgTled6TDNKmXbZkxabJ3nyylJXdRAeo4sP1saf5Me
nMVhZPGeOX6VP+9JPuLIs+8qcEFyz90cZIBXUMtxG045ugVQ+DRcrNUAxRSg/c9s
0XL7oCO8D2QDuPdyun9Fe7Btdq+gy8cC32oWkS5rzniY9nfZKJIS1GPzmhJ2YW/w
/moFP4kcrRAKHWDr2CLTAOKS8W3qZzi5RLiCHhHt2Cm5ASsOlzsbf3fz4uCeqbvm
uCFXn7OglJagxpOvXf6oGQKbbLSPZ26BC6eQav3AB4rKlbFNn96B1GxUY3U3IXb4
lhvOyL3AcHS+juxPOnUoRL9cChB2sVPc7ve9Z+yWw6nHwVbG/AnEIYq9BGb9sp5d
9J04XwrjDtx8VU0zjzJTBXxPdeS3aDh+PWcHbe17l2+lKHEFA/Ch+/b0Svbvtmem
DEG1K2cvOSn71cG4fFTl9DR++nR93frij2te5NGHpqDTbQoVb45RhpBViyf5Qhvu
DT/HZHjeQ+FEjnsikgE1q9VJZhp2gUBKssrLBRRavUz5FAe2q/vzk09oiQaWUODU
Iwl0nEJ1u/KQzVfv+yb3
=O68y
-END PGP SIGNATURE-
Index: docs/ClangFormat.rst
===
--- docs/ClangFormat.rst	(Revision 293072)
+++ docs/ClangFormat.rst	(Arbeitskopie)
@@ -42,6 +42,7 @@
 -style=file, but can not find the .clang-format
 file to use.
 Use -fallback-style=none to skip formatting.
+Use -fallback-style=fail to exit with an error.
 -i- Inplace edit s, if specified.
 -length=- Format a range of this length (in bytes).
 Multiple ranges can be formatted by specifying
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp	(Revision 293072)
+++ lib/Format/Format.cpp	(Arbeitskopie)
@@ -737,7 +737,7 @@
 *Style = getWebKitStyle();
   } else if (Name.equals_lower("gnu")) {
 *Style = getGNUStyle();
-  } else if (Name.equals_lower("none")) {
+  } else if (Name.equals_lower("none") || Name.equals_lower("fail")) {
 *Style = getNoStyle();
   } else {
 return false;
@@ -1977,7 +1977,12 @@
 return make_string_error("Configuration file(s) do(es) not support " +
  getLanguageName(Style.Language) + ": " +
  UnsuitableConfigFiles);
-  return FallbackStyle;
+
+  if (!FallbackStyleName.equals_lower("fail")) {
+return FallbackStyle;
+  } else {
+return make_string_error("Configuration file not found.");
+  }
 }
 
 } // namespace format
Index: tools/clang-format/ClangFormat.cpp
===
--- tools/clang-format/ClangFormat.cpp	(Revision 293072)
+++ tools/clang-format/ClangFormat.cpp	(Arbeitskopie)
@@ -68,7 +68,8 @@
"fallback in case clang-format is invoked with\n"
"-style=file, but can not find the .clang-format\n"
"file to use.\n"
-   "Use -fallback-style=none to skip formatting."),
+   "Use -fallback-style=none to skip formatting.\n"
+   "Use 

Re: Add warning for c++ member variable shadowing

[James Sun via cfe-commits]

Hi Saleem

Thanks for the quick response. A test case is added. It covers some ordinary 
cases as well as corner cases like multiple paths to the same base.

Thanks

James

From: Saleem Abdulrasool 
Date: Monday, January 30, 2017 at 6:50 PM
To: James Sun 
Cc: Richard Smith , "cfe-commits@lists.llvm.org" 
, Aaron Ballman 
Subject: Re: Add warning for c++ member variable shadowing

I think that the patch is starting to look pretty good!

Can you add some test cases for the particular cases to diagnose in a separate 
test set to ensure that we have proper coverage of the various cases rather 
than relying on the existing test cases?  Something to make sure that we get 
the simple case right as well as the complex cases (e.g. we don't print 
duplicate warnings for multiple paths).


On Mon, Jan 30, 2017 at 5:50 PM, James Sun 
> wrote:
Hi Richard

Sorry for the late reply. Thank you for giving the feedback! The updated 
version is attached. Please let me know if there is anything improper.

Thanks

James

From: > on behalf of Richard Smith 
>
Date: Friday, January 27, 2017 at 3:03 PM
To: James Sun >
Cc: Saleem Abdulrasool >, 
"cfe-commits@lists.llvm.org" 
>, Aaron Ballman 
>

Subject: Re: Add warning for c++ member variable shadowing

+def warn_shadow_member_variable : Warning<
+  "shadowed variable '%0' in type '%1' inheriting from type '%2'">,

The phrasing of this is incorrect: the things you're warning about are not 
variables, they're non-static data members. Perhaps something like:

  "non-static data member '%0' of '%1' shadows member inherited from type '%2'"

+   InGroup;

Would it make sense to put this in a subgroup of -Wshadow so that it can be 
controlled separately?

+  /// Check if there is a member variable shadowing

Please end comments in a period.

+  void CheckShadowInheritedVariables(const SourceLocation ,

Likewise, 'Variables' is wrong. We would typically use the C term 'Fields' for 
these cases within Clang sources.

+  for (const auto  : DC->bases()) {
+if (const auto *TSI = Base.getTypeSourceInfo())
+  if (const auto *BaseClass = TSI->getType()->getAsCXXRecordDecl()) {
+for (const auto *Field : BaseClass->fields())
+  if (Field->getName() == FieldName)
+Diag(Loc, diag::warn_shadow_member_variable)
+  << FieldName << RD->getName() << BaseClass->getName();
+// Search parent's parents
+CheckShadowInheritedVariables(Loc, FieldName, RD, BaseClass);
+  }
+  }

Maybe we should avoid diagnosing shadowing of members that are inaccessible 
from the derived class? What about if the field name is ambiguous? Also, we 
shouldn't recurse if lookup finds something with the given name in this class, 
and ideally we would only visit each class once, even if it appears multiple 
times in a multiple-inheritance scenario. CXXRecordDecl::lookupInBases can 
handle most of these cases for you automatically, and will also let you build a 
set of paths to problematic base classes in case you want to report those.

On 24 January 2017 at 20:52, James Sun 
> wrote:
Thanks for the comments. The new version is attached.
Wrt two of your questions:

(1)  “The description that you have on CheckShadowInheritedVariables isn't 
really the type of comments that we have in doxygen form.  Im not sure if its 
in line with the rest of the code.”
I’ve read through the doxygen wiki; hopefully it’s fixed; let me know if it’s 
still wrong

(2) “Why are you checking that the DeclContext has a definition rather than the 
record itself?”
There are cases like “struct A; struct B : A {};”, where A does not have a 
definition. The compiler will hit an assertion failure if we call A.bases() 
directly.

Thanks

James


From: Saleem Abdulrasool >
Date: Tuesday, January 24, 2017 at 7:10 PM
To: James Sun >
Cc: "cfe-commits@lists.llvm.org" 
>, Aaron Ballman 
>, Richard Smith 
>
Subject: Re: Add warning for c++ member variable shadowing

Some more stylistic comments:

The description that you have on CheckShadowInheritedVariables isn't really the 
type of comments that we have in doxygen form.  Im not sure if its in line with 
the rest of the code.

The ignore 

r293609 - [lsan] Enable LSan for x86 Linux

[Maxim Ostapenko via cfe-commits]

Author: chefmax
Date: Tue Jan 31 01:00:23 2017
New Revision: 293609

URL: http://llvm.org/viewvc/llvm-project?rev=293609=rev
Log:
[lsan] Enable LSan for x86 Linux

This is a missed part of https://reviews.llvm.org/D28609.
Enable LSan for x86 Linux in clang driver.

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

Modified:
cfe/trunk/lib/Driver/ToolChains.cpp
cfe/trunk/test/Driver/fsanitize.c

Modified: cfe/trunk/lib/Driver/ToolChains.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains.cpp?rev=293609=293608=293609=diff
==
--- cfe/trunk/lib/Driver/ToolChains.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains.cpp Tue Jan 31 01:00:23 2017
@@ -4728,7 +4728,7 @@ SanitizerMask Linux::getSupportedSanitiz
   Res |= SanitizerKind::SafeStack;
   if (IsX86_64 || IsMIPS64 || IsAArch64)
 Res |= SanitizerKind::DataFlow;
-  if (IsX86_64 || IsMIPS64 || IsAArch64)
+  if (IsX86_64 || IsMIPS64 || IsAArch64 || IsX86)
 Res |= SanitizerKind::Leak;
   if (IsX86_64 || IsMIPS64 || IsAArch64 || IsPowerPC64)
 Res |= SanitizerKind::Thread;

Modified: cfe/trunk/test/Driver/fsanitize.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/fsanitize.c?rev=293609=293608=293609=diff
==
--- cfe/trunk/test/Driver/fsanitize.c (original)
+++ cfe/trunk/test/Driver/fsanitize.c Tue Jan 31 01:00:23 2017
@@ -231,6 +231,12 @@
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address,leak 
-fno-sanitize=address %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-SANA-SANL-NO-SANA
 // CHECK-SANA-SANL-NO-SANA: "-fsanitize=leak"
 
+// RUN: %clang -target i686-linux-gnu -fsanitize=leak %s -### 2>&1 | FileCheck 
%s --check-prefix=CHECK-SANL-X86
+// CHECK-SANL-X86: "-fsanitize=leak"
+
+// RUN: %clang -target i686-linux-gnu -fsanitize=address,leak 
-fno-sanitize=address %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-SANA-SANL-NO-SANA-X86
+// CHECK-SANA-SANL-NO-SANA-X86: "-fsanitize=leak"
+
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=memory %s -### 2>&1 | 
FileCheck %s --check-prefix=CHECK-MSAN
 // CHECK-MSAN: "-fno-assume-sane-operator-new"
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address %s -### 2>&1 | 
FileCheck %s --check-prefix=CHECK-ASAN


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

[PATCH] D29322: [clang-format] Fix regression merging comments across newlines.

[Krasimir Georgiev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL293624: [clang-format] Fix regression merging comments 
across newlines. (authored by krasimir).

Changed prior to commit:
  https://reviews.llvm.org/D29322?vs=86412=86413#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D29322

Files:
  cfe/trunk/lib/Format/BreakableToken.cpp
  cfe/trunk/lib/Format/UnwrappedLineParser.cpp
  cfe/trunk/unittests/Format/FormatTest.cpp


Index: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
===
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp
@@ -2124,7 +2124,12 @@
I != E; ++I) {
 // Line comments that belong to the same line comment section are put on 
the
 // same line since later we might want to reflow content between them.
-// See BreakableToken.
+// Additional fine-grained breaking of line comment sections is controlled
+// by the class BreakableLineCommentSection in case it is desirable to keep
+// several line comment sections in the same unwrapped line.
+//
+// FIXME: Consider putting separate line comment sections as children to 
the
+// unwrapped line instead.
 if (isOnNewLine(**I) && JustComments && !continuesLineComment(**I, *Line))
   addUnwrappedLine();
 pushToken(*I);
Index: cfe/trunk/lib/Format/BreakableToken.cpp
===
--- cfe/trunk/lib/Format/BreakableToken.cpp
+++ cfe/trunk/lib/Format/BreakableToken.cpp
@@ -681,6 +681,23 @@
   Content[i] = Content[i].substr(0, EndOfLine);
 }
 LineTok = CurrentTok->Next;
+if (CurrentTok->Next && CurrentTok->Next->NewlinesBefore > 1) {
+  // A line comment section needs to broken by a line comment that is
+  // preceded by at least two newlines. Note that we put this break here
+  // instead of breaking at a previous stage during parsing, since that
+  // would split the contents of the enum into two unwrapped lines in this
+  // example, which is undesirable:
+  // enum A {
+  //   a, // comment about a
+  //
+  //   // comment about b
+  //   b
+  // };
+  //
+  // FIXME: Consider putting separate line comment sections as children to
+  // the unwrapped line instead.
+  break;
+}
   }
 }
 
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -934,6 +934,24 @@
"  VAL_B\n"
"};");
 
+  EXPECT_EQ("enum A {\n"
+"  // line a\n"
+"  a,\n"
+"  b, // line b\n"
+"\n"
+"  // line c\n"
+"  c\n"
+"};",
+format("enum A {\n"
+   "  // line a\n"
+   "  a,\n"
+   "  b, // line b\n"
+   "\n"
+   "  // line c\n"
+   "  c\n"
+   "};",
+   getLLVMStyleWithColumns(20)));
+
   verifyFormat(
   "a =\n"
   "bb; // Trailing comment");


Index: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
===
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp
@@ -2124,7 +2124,12 @@
I != E; ++I) {
 // Line comments that belong to the same line comment section are put on the
 // same line since later we might want to reflow content between them.
-// See BreakableToken.
+// Additional fine-grained breaking of line comment sections is controlled
+// by the class BreakableLineCommentSection in case it is desirable to keep
+// several line comment sections in the same unwrapped line.
+//
+// FIXME: Consider putting separate line comment sections as children to the
+// unwrapped line instead.
 if (isOnNewLine(**I) && JustComments && !continuesLineComment(**I, *Line))
   addUnwrappedLine();
 pushToken(*I);
Index: cfe/trunk/lib/Format/BreakableToken.cpp
===
--- cfe/trunk/lib/Format/BreakableToken.cpp
+++ cfe/trunk/lib/Format/BreakableToken.cpp
@@ -681,6 +681,23 @@
   Content[i] = Content[i].substr(0, EndOfLine);
 }
 LineTok = CurrentTok->Next;
+if (CurrentTok->Next && CurrentTok->Next->NewlinesBefore > 1) {
+  // A line comment section needs to broken by a line comment that is
+  // preceded by at least two newlines. Note that we put this break here
+  // instead of breaking at a previous stage during parsing, since that
+  // would split the contents of the enum into two unwrapped lines in this
+  // example, which is 

r293624 - [clang-format] Fix regression merging comments across newlines.

[Krasimir Georgiev via cfe-commits]

Author: krasimir
Date: Tue Jan 31 07:32:38 2017
New Revision: 293624

URL: http://llvm.org/viewvc/llvm-project?rev=293624=rev
Log:
[clang-format] Fix regression merging comments across newlines.

Summary:
This fixes a regression that causes example:
```
enum A {
  a, // line a

  // line b
  b
};
```
to be formatted as follows:
```
enum A {
  a, // line a
 // line b
  b
};
```

Reviewers: djasper, klimek

Reviewed By: klimek

Subscribers: cfe-commits, sammccall, djasper, klimek

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

Modified:
cfe/trunk/lib/Format/BreakableToken.cpp
cfe/trunk/lib/Format/UnwrappedLineParser.cpp
cfe/trunk/unittests/Format/FormatTest.cpp

Modified: cfe/trunk/lib/Format/BreakableToken.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/BreakableToken.cpp?rev=293624=293623=293624=diff
==
--- cfe/trunk/lib/Format/BreakableToken.cpp (original)
+++ cfe/trunk/lib/Format/BreakableToken.cpp Tue Jan 31 07:32:38 2017
@@ -681,6 +681,23 @@ BreakableLineCommentSection::BreakableLi
   Content[i] = Content[i].substr(0, EndOfLine);
 }
 LineTok = CurrentTok->Next;
+if (CurrentTok->Next && CurrentTok->Next->NewlinesBefore > 1) {
+  // A line comment section needs to broken by a line comment that is
+  // preceded by at least two newlines. Note that we put this break here
+  // instead of breaking at a previous stage during parsing, since that
+  // would split the contents of the enum into two unwrapped lines in this
+  // example, which is undesirable:
+  // enum A {
+  //   a, // comment about a
+  //
+  //   // comment about b
+  //   b
+  // };
+  //
+  // FIXME: Consider putting separate line comment sections as children to
+  // the unwrapped line instead.
+  break;
+}
   }
 }
 

Modified: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineParser.cpp?rev=293624=293623=293624=diff
==
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp (original)
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp Tue Jan 31 07:32:38 2017
@@ -2124,7 +2124,12 @@ void UnwrappedLineParser::flushComments(
I != E; ++I) {
 // Line comments that belong to the same line comment section are put on 
the
 // same line since later we might want to reflow content between them.
-// See BreakableToken.
+// Additional fine-grained breaking of line comment sections is controlled
+// by the class BreakableLineCommentSection in case it is desirable to keep
+// several line comment sections in the same unwrapped line.
+//
+// FIXME: Consider putting separate line comment sections as children to 
the
+// unwrapped line instead.
 if (isOnNewLine(**I) && JustComments && !continuesLineComment(**I, *Line))
   addUnwrappedLine();
 pushToken(*I);

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=293624=293623=293624=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Tue Jan 31 07:32:38 2017
@@ -934,6 +934,24 @@ TEST_F(FormatTest, UnderstandsSingleLine
"  VAL_B\n"
"};");
 
+  EXPECT_EQ("enum A {\n"
+"  // line a\n"
+"  a,\n"
+"  b, // line b\n"
+"\n"
+"  // line c\n"
+"  c\n"
+"};",
+format("enum A {\n"
+   "  // line a\n"
+   "  a,\n"
+   "  b, // line b\n"
+   "\n"
+   "  // line c\n"
+   "  c\n"
+   "};",
+   getLLVMStyleWithColumns(20)));
+
   verifyFormat(
   "a =\n"
   "bb; // Trailing comment");


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

[PATCH] D29322: [clang-format] Fix regression merging comments across newlines.

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added inline comments.



Comment at: lib/Format/UnwrappedLineParser.cpp:2127-2129
+// Additional fine-grained breaking of line comment sections is controlled
+// by the class BreakableLineCommentSection in case it is desirable to keep
+// several line comment sections in the same unwrapped line.

klimek wrote:
> Can you also add a FIXME here (as this is where it'd probably need to happen 
> :)
Done.


https://reviews.llvm.org/D29322



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

[PATCH] D29322: [clang-format] Fix regression merging comments across newlines.

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir updated this revision to Diff 86412.
krasimir marked an inline comment as done.
krasimir added a comment.

- Add a FIXME also in the parser


https://reviews.llvm.org/D29322

Files:
  lib/Format/BreakableToken.cpp
  lib/Format/UnwrappedLineParser.cpp
  unittests/Format/FormatTest.cpp


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -934,6 +934,24 @@
"  VAL_B\n"
"};");
 
+  EXPECT_EQ("enum A {\n"
+"  // line a\n"
+"  a,\n"
+"  b, // line b\n"
+"\n"
+"  // line c\n"
+"  c\n"
+"};",
+format("enum A {\n"
+   "  // line a\n"
+   "  a,\n"
+   "  b, // line b\n"
+   "\n"
+   "  // line c\n"
+   "  c\n"
+   "};",
+   getLLVMStyleWithColumns(20)));
+
   verifyFormat(
   "a =\n"
   "bb; // Trailing comment");
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -2124,7 +2124,12 @@
I != E; ++I) {
 // Line comments that belong to the same line comment section are put on 
the
 // same line since later we might want to reflow content between them.
-// See BreakableToken.
+// Additional fine-grained breaking of line comment sections is controlled
+// by the class BreakableLineCommentSection in case it is desirable to keep
+// several line comment sections in the same unwrapped line.
+//
+// FIXME: Consider putting separate line comment sections as children to 
the
+// unwrapped line instead.
 if (isOnNewLine(**I) && JustComments && !continuesLineComment(**I, *Line))
   addUnwrappedLine();
 pushToken(*I);
Index: lib/Format/BreakableToken.cpp
===
--- lib/Format/BreakableToken.cpp
+++ lib/Format/BreakableToken.cpp
@@ -682,6 +682,23 @@
   Content[i] = Content[i].substr(0, EndOfLine);
 }
 LineTok = CurrentTok->Next;
+if (CurrentTok->Next && CurrentTok->Next->NewlinesBefore > 1) {
+  // A line comment section needs to broken by a line comment that is
+  // preceded by at least two newlines. Note that we put this break here
+  // instead of breaking at a previous stage during parsing, since that
+  // would split the contents of the enum into two unwrapped lines in this
+  // example, which is undesirable:
+  // enum A {
+  //   a, // comment about a
+  //
+  //   // comment about b
+  //   b
+  // };
+  //
+  // FIXME: Consider putting separate line comment sections as children to
+  // the unwrapped line instead.
+  break;
+}
   }
 }
 


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -934,6 +934,24 @@
"  VAL_B\n"
"};");
 
+  EXPECT_EQ("enum A {\n"
+"  // line a\n"
+"  a,\n"
+"  b, // line b\n"
+"\n"
+"  // line c\n"
+"  c\n"
+"};",
+format("enum A {\n"
+   "  // line a\n"
+   "  a,\n"
+   "  b, // line b\n"
+   "\n"
+   "  // line c\n"
+   "  c\n"
+   "};",
+   getLLVMStyleWithColumns(20)));
+
   verifyFormat(
   "a =\n"
   "bb; // Trailing comment");
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -2124,7 +2124,12 @@
I != E; ++I) {
 // Line comments that belong to the same line comment section are put on the
 // same line since later we might want to reflow content between them.
-// See BreakableToken.
+// Additional fine-grained breaking of line comment sections is controlled
+// by the class BreakableLineCommentSection in case it is desirable to keep
+// several line comment sections in the same unwrapped line.
+//
+// FIXME: Consider putting separate line comment sections as children to the
+// unwrapped line instead.
 if (isOnNewLine(**I) && JustComments && !continuesLineComment(**I, *Line))
   addUnwrappedLine();
 pushToken(*I);
Index: lib/Format/BreakableToken.cpp
===
--- 

[PATCH] D29027: [Stack Protection] Add remark for reasons why Stack Protection has been applied

[James Henderson via Phabricator via cfe-commits]

jhenderson added a comment.

Ping


https://reviews.llvm.org/D29027



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

[libcxx] r293623 - Fix up some no-exception compile failures

[Marshall Clow via cfe-commits]

Author: marshall
Date: Tue Jan 31 07:12:32 2017
New Revision: 293623

URL: http://llvm.org/viewvc/llvm-project?rev=293623=rev
Log:
Fix up some no-exception compile failures

Modified:
libcxx/trunk/test/std/strings/basic.string/string.cons/copy_alloc.pass.cpp

Modified: 
libcxx/trunk/test/std/strings/basic.string/string.cons/copy_alloc.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/strings/basic.string/string.cons/copy_alloc.pass.cpp?rev=293623=293622=293623=diff
==
--- libcxx/trunk/test/std/strings/basic.string/string.cons/copy_alloc.pass.cpp 
(original)
+++ libcxx/trunk/test/std/strings/basic.string/string.cons/copy_alloc.pass.cpp 
Tue Jan 31 07:12:32 2017
@@ -18,6 +18,7 @@
 #include "test_allocator.h"
 #include "min_allocator.h"
 
+#ifndef TEST_HAS_NO_EXCEPTIONS
 template 
 struct alloc_imp {
 bool active;
@@ -44,7 +45,7 @@ struct poca_alloc {
 
 alloc_imp *imp;
 
-poca_alloc(alloc_imp *ximp) : imp (ximp) {}
+poca_alloc(alloc_imp *imp_) : imp (imp_) {}
 
 template 
 poca_alloc(const poca_alloc& other) : imp(other.imp) {}
@@ -65,6 +66,15 @@ bool operator!=(const poca_alloc& lhs
 return lhs.imp != rhs.imp;
 }
 
+template 
+void test_assign(S , const S& s2)
+{
+   try { s1 = s2; }
+   catch ( std::bad_alloc &) { return; }
+   assert(false);
+}
+#endif
+
 
 
 template 
@@ -78,16 +88,6 @@ test(S s1, const typename S::allocator_t
 assert(s2.get_allocator() == a);
 }
 
-#ifndef TEST_HAS_NO_EXCEPTIONS
-template 
-void test_assign(S , const S& s2)
-{
-   try { s1 = s2; }
-   catch ( std::bad_alloc &) { return; }
-   assert(false);
-}
-#endif
-
 int main()
 {
 {


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

r293622 - clang-format: [JS] Properly set scopes inside template strings.

[Daniel Jasper via cfe-commits]

Author: djasper
Date: Tue Jan 31 07:03:07 2017
New Revision: 293622

URL: http://llvm.org/viewvc/llvm-project?rev=293622=rev
Log:
clang-format: [JS] Properly set scopes inside template strings.

Before:
  var f = `a:${aaa
  .a} 
   a:${aaa.a} `;

After:
  var f = `a:${aaa.a} 
   a:${aaa.a} `;

Modified:
cfe/trunk/lib/Format/FormatToken.h
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTestJS.cpp

Modified: cfe/trunk/lib/Format/FormatToken.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/FormatToken.h?rev=293622=293621=293622=diff
==
--- cfe/trunk/lib/Format/FormatToken.h (original)
+++ cfe/trunk/lib/Format/FormatToken.h Tue Jan 31 07:03:07 2017
@@ -337,11 +337,15 @@ struct FormatToken {
 
   /// \brief Returns whether \p Tok is ([{ or a template opening <.
   bool opensScope() const {
+if (is(TT_TemplateString) && TokenText.endswith("${"))
+  return true;
 return isOneOf(tok::l_paren, tok::l_brace, tok::l_square,
TT_TemplateOpener);
   }
   /// \brief Returns whether \p Tok is )]} or a template closing >.
   bool closesScope() const {
+if (is(TT_TemplateString) && TokenText.startswith("}"))
+  return true;
 return isOneOf(tok::r_paren, tok::r_brace, tok::r_square,
TT_TemplateCloser);
   }

Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=293622=293621=293622=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Tue Jan 31 07:03:07 2017
@@ -1454,7 +1454,9 @@ public:
 
   // Consume scopes: (), [], <> and {}
   if (Current->opensScope()) {
-while (Current && !Current->closesScope()) {
+// In fragment of a JavaScript template string can look like '}..${' 
and
+// thus close a scope and open a new one at the same time.
+while (Current && (!Current->closesScope() || Current->opensScope())) {
   next();
   parse();
 }

Modified: cfe/trunk/unittests/Format/FormatTestJS.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestJS.cpp?rev=293622=293621=293622=diff
==
--- cfe/trunk/unittests/Format/FormatTestJS.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestJS.cpp Tue Jan 31 07:03:07 2017
@@ -1392,6 +1392,13 @@ TEST_F(FormatTestJS, TemplateStrings) {
   // The token stream can contain two string_literals in sequence, but that
   // doesn't mean that they are implicitly concatenated in JavaScript.
   verifyFormat("var f = ` ${a ? 'a' : 'b'}`;");
+
+  // Ensure that scopes are appropriately set around evaluated expressions in
+  // template strings.
+  verifyFormat("var f = `a:${aaa.a} \n"
+   " a:${aaa.a} `;",
+   "var f = `a:${aaa.  a} \n"
+   " a:${  aaa. a} `;");
 }
 
 TEST_F(FormatTestJS, TemplateStringASI) {


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

[PATCH] D29322: [clang-format] Fix regression merging comments across newlines.

[Manuel Klimek via Phabricator via cfe-commits]

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

lg minus adding the FIXME to the place where we need the change.




Comment at: lib/Format/UnwrappedLineParser.cpp:2127-2129
+// Additional fine-grained breaking of line comment sections is controlled
+// by the class BreakableLineCommentSection in case it is desirable to keep
+// several line comment sections in the same unwrapped line.

Can you also add a FIXME here (as this is where it'd probably need to happen :)


https://reviews.llvm.org/D29322



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

[PATCH] D29322: [clang-format] Fix regression merging comments across newlines.

[Krasimir Georgiev via Phabricator via cfe-commits]

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



Comment at: lib/Format/BreakableToken.cpp:685
 LineTok = CurrentTok->Next;
+if (CurrentTok->Next && CurrentTok->Next->NewlinesBefore > 1) {
+  // A line comment section needs to broken by a line comment that is

krasimir wrote:
> klimek wrote:
> > Could we put them as children into the higher level UnwrappedLine instead?
> Putting them as children looks cleaner; will try that.
For now, I'm adding a FIXME, since this putting the sections into children has 
some undesired ripple effects in other places and needs more in-depth analysis.


https://reviews.llvm.org/D29322



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

[PATCH] D29322: [clang-format] Fix regression merging comments across newlines.

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir updated this revision to Diff 86410.
krasimir added a comment.

- Add a FIXME for separating line comment sections into child lines


https://reviews.llvm.org/D29322

Files:
  lib/Format/BreakableToken.cpp
  lib/Format/UnwrappedLineParser.cpp
  unittests/Format/FormatTest.cpp


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -934,6 +934,24 @@
"  VAL_B\n"
"};");
 
+  EXPECT_EQ("enum A {\n"
+"  // line a\n"
+"  a,\n"
+"  b, // line b\n"
+"\n"
+"  // line c\n"
+"  c\n"
+"};",
+format("enum A {\n"
+   "  // line a\n"
+   "  a,\n"
+   "  b, // line b\n"
+   "\n"
+   "  // line c\n"
+   "  c\n"
+   "};",
+   getLLVMStyleWithColumns(20)));
+
   verifyFormat(
   "a =\n"
   "bb; // Trailing comment");
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -2124,7 +2124,9 @@
I != E; ++I) {
 // Line comments that belong to the same line comment section are put on 
the
 // same line since later we might want to reflow content between them.
-// See BreakableToken.
+// Additional fine-grained breaking of line comment sections is controlled
+// by the class BreakableLineCommentSection in case it is desirable to keep
+// several line comment sections in the same unwrapped line.
 if (isOnNewLine(**I) && JustComments && !continuesLineComment(**I, *Line))
   addUnwrappedLine();
 pushToken(*I);
Index: lib/Format/BreakableToken.cpp
===
--- lib/Format/BreakableToken.cpp
+++ lib/Format/BreakableToken.cpp
@@ -682,6 +682,23 @@
   Content[i] = Content[i].substr(0, EndOfLine);
 }
 LineTok = CurrentTok->Next;
+if (CurrentTok->Next && CurrentTok->Next->NewlinesBefore > 1) {
+  // A line comment section needs to broken by a line comment that is
+  // preceded by at least two newlines. Note that we put this break here
+  // instead of breaking at a previous stage during parsing, since that
+  // would split the contents of the enum into two unwrapped lines in this
+  // example, which is undesirable:
+  // enum A {
+  //   a, // comment about a
+  //
+  //   // comment about b
+  //   b
+  // };
+  //
+  // FIXME: Consider putting separate line comment sections as children to
+  // the unwrapped line instead.
+  break;
+}
   }
 }
 


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -934,6 +934,24 @@
"  VAL_B\n"
"};");
 
+  EXPECT_EQ("enum A {\n"
+"  // line a\n"
+"  a,\n"
+"  b, // line b\n"
+"\n"
+"  // line c\n"
+"  c\n"
+"};",
+format("enum A {\n"
+   "  // line a\n"
+   "  a,\n"
+   "  b, // line b\n"
+   "\n"
+   "  // line c\n"
+   "  c\n"
+   "};",
+   getLLVMStyleWithColumns(20)));
+
   verifyFormat(
   "a =\n"
   "bb; // Trailing comment");
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -2124,7 +2124,9 @@
I != E; ++I) {
 // Line comments that belong to the same line comment section are put on the
 // same line since later we might want to reflow content between them.
-// See BreakableToken.
+// Additional fine-grained breaking of line comment sections is controlled
+// by the class BreakableLineCommentSection in case it is desirable to keep
+// several line comment sections in the same unwrapped line.
 if (isOnNewLine(**I) && JustComments && !continuesLineComment(**I, *Line))
   addUnwrappedLine();
 pushToken(*I);
Index: lib/Format/BreakableToken.cpp
===
--- lib/Format/BreakableToken.cpp
+++ lib/Format/BreakableToken.cpp
@@ -682,6 +682,23 @@
   Content[i] = Content[i].substr(0, EndOfLine);
 }
 LineTok = CurrentTok->Next;
+if (CurrentTok->Next && CurrentTok->Next->NewlinesBefore > 1) {
+  // A line comment 

[libcxx] r293619 - Workaround new -Wshadow warning introduced by r293599

[Alex Lorenz via cfe-commits]

Author: arphaman
Date: Tue Jan 31 06:37:48 2017
New Revision: 293619

URL: http://llvm.org/viewvc/llvm-project?rev=293619=rev
Log:
Workaround new -Wshadow warning introduced by r293599

Modified:
libcxx/trunk/test/std/strings/basic.string/string.cons/copy_alloc.pass.cpp

Modified: 
libcxx/trunk/test/std/strings/basic.string/string.cons/copy_alloc.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/strings/basic.string/string.cons/copy_alloc.pass.cpp?rev=293619=293618=293619=diff
==
--- libcxx/trunk/test/std/strings/basic.string/string.cons/copy_alloc.pass.cpp 
(original)
+++ libcxx/trunk/test/std/strings/basic.string/string.cons/copy_alloc.pass.cpp 
Tue Jan 31 06:37:48 2017
@@ -44,7 +44,7 @@ struct poca_alloc {
 
 alloc_imp *imp;
 
-poca_alloc(alloc_imp *imp) : imp (imp) {}
+poca_alloc(alloc_imp *ximp) : imp (ximp) {}
 
 template 
 poca_alloc(const poca_alloc& other) : imp(other.imp) {}


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

[PATCH] D29300: [clang-format] Refactor WhitespaceManager and friends

[Daniel Jasper via Phabricator via cfe-commits]

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



Comment at: lib/Format/WhitespaceManager.cpp:178
-  LastBlockComment = 
-} else if (Change.Kind == tok::unknown) {
-  if ((Change.StartOfBlockComment = LastBlockComment))

krasimir wrote:
> What happened to the tok::unknown case?
We actually don't set the token back to unknown in replaceWhitespaceInToken 
anymore if this is a comment. So I moved this logic back into the if above.


https://reviews.llvm.org/D29300



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

[PATCH] D29300: [clang-format] Refactor WhitespaceManager and friends

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added a comment.

looks good!


https://reviews.llvm.org/D29300



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

[PATCH] D29300: [clang-format] Refactor WhitespaceManager and friends

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added inline comments.



Comment at: lib/Format/WhitespaceManager.cpp:178
-  LastBlockComment = 
-} else if (Change.Kind == tok::unknown) {
-  if ((Change.StartOfBlockComment = LastBlockComment))

What happened to the tok::unknown case?


https://reviews.llvm.org/D29300



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

r293618 - clang-format: [JS] Fix incorrect line break in template strings.

[Daniel Jasper via cfe-commits]

Author: djasper
Date: Tue Jan 31 06:07:35 2017
New Revision: 293618

URL: http://llvm.org/viewvc/llvm-project?rev=293618=rev
Log:
clang-format: [JS] Fix incorrect line break in template strings.

Before:
  var f = ` ${a ? 'a' : 'b'
}`;

After:
  var f = ` ${a ? 'a' : 'b'}`;

Modified:
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTestJS.cpp

Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=293618=293617=293618=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Tue Jan 31 06:07:35 2017
@@ -2388,6 +2388,11 @@ bool TokenAnnotator::mustBreakBefore(con
 if (Right.is(tok::plus) && Left.is(tok::string_literal) && Right.Next &&
 Right.Next->is(tok::string_literal))
   return true;
+  } else if (Style.Language == FormatStyle::LK_Cpp ||
+ Style.Language == FormatStyle::LK_ObjC) {
+if (Left.isStringLiteral() &&
+(Right.isStringLiteral() || Right.is(TT_ObjCStringLiteral)))
+  return true;
   }
 
   // If the last token before a '}' is a comma or a trailing comment, the
@@ -2411,9 +2416,6 @@ bool TokenAnnotator::mustBreakBefore(con
(Right.NewlinesBefore > 0 && Right.HasUnescapedNewline);
   if (Left.isTrailingComment())
 return true;
-  if (Left.isStringLiteral() &&
-  (Right.isStringLiteral() || Right.is(TT_ObjCStringLiteral)))
-return true;
   if (Right.Previous->IsUnterminatedLiteral)
 return true;
   if (Right.is(tok::lessless) && Right.Next &&

Modified: cfe/trunk/unittests/Format/FormatTestJS.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestJS.cpp?rev=293618=293617=293618=diff
==
--- cfe/trunk/unittests/Format/FormatTestJS.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestJS.cpp Tue Jan 31 06:07:35 2017
@@ -1388,6 +1388,10 @@ TEST_F(FormatTestJS, TemplateStrings) {
"var y;");
   // Escaped dollar.
   verifyFormat("var x = ` \\${foo}`;\n");
+
+  // The token stream can contain two string_literals in sequence, but that
+  // doesn't mean that they are implicitly concatenated in JavaScript.
+  verifyFormat("var f = ` ${a ? 'a' : 'b'}`;");
 }
 
 TEST_F(FormatTestJS, TemplateStringASI) {


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

[PATCH] D21279: Fix some issues in clang-format's AlignConsecutive modes

[Daniel Jasper via Phabricator via cfe-commits]

djasper added a comment.

I apologize in advance if this causes merge conflicts with r293616. However, I 
do hope that that actually makes this patch easier.


https://reviews.llvm.org/D21279



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

[PATCH] D27651: [clang-format] Even with AlignConsecutiveDeclarations, PointerAlignment: Right should keep *s and to the right

[Daniel Jasper via Phabricator via cfe-commits]

djasper added a comment.

I have given stuff in WhitespaceManager access to the actual FormatToken in 
r293616. Hopefully that simplifies this patch.


https://reviews.llvm.org/D27651



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

r293617 - [clang-format] Don't reflow comment lines starting with '@'.

[Krasimir Georgiev via cfe-commits]

Author: krasimir
Date: Tue Jan 31 05:38:02 2017
New Revision: 293617

URL: http://llvm.org/viewvc/llvm-project?rev=293617=rev
Log:
[clang-format] Don't reflow comment lines starting with '@'.

Summary:
This patch stops reflowing comment lines starting with '@', since they commonly
have a special meaning.

Reviewers: djasper

Reviewed By: djasper

Subscribers: cfe-commits, klimek

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

Modified:
cfe/trunk/lib/Format/BreakableToken.cpp
cfe/trunk/unittests/Format/FormatTest.cpp

Modified: cfe/trunk/lib/Format/BreakableToken.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/BreakableToken.cpp?rev=293617=293616=293617=diff
==
--- cfe/trunk/lib/Format/BreakableToken.cpp (original)
+++ cfe/trunk/lib/Format/BreakableToken.cpp Tue Jan 31 05:38:02 2017
@@ -304,7 +304,9 @@ static bool mayReflowContent(StringRef C
   // Simple heuristic for what to reflow: content should contain at least two
   // characters and either the first or second character must be
   // non-punctuation.
-  return Content.size() >= 2 && !Content.endswith("\\") &&
+  return Content.size() >= 2 &&
+ // Lines starting with '@' commonly have special meaning.
+ !Content.startswith("@") && !Content.endswith("\\") &&
  // Note that this is UTF-8 safe, since if isPunctuation(Content[0]) is
  // true, then the first code point must be 1 byte long.
  (!isPunctuation(Content[0]) || !isPunctuation(Content[1]));

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=293617=293616=293617=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Tue Jan 31 05:38:02 2017
@@ -2244,6 +2244,15 @@ TEST_F(FormatTest, ReflowsComments) {
 "// long long long long\n"
 "// ... --- ...",
 getLLVMStyleWithColumns(20)));
+
+  // Don't reflow lines starting with '@'.
+  EXPECT_EQ("// long long long\n"
+"// long\n"
+"// @param arg",
+format("// long long long long\n"
+   "// @param arg",
+   getLLVMStyleWithColumns(20)));
+
   // Reflow lines that have a non-punctuation character among their first 2
   // characters.
   EXPECT_EQ("// long long long\n"


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

[PATCH] D29323: [clang-format] Don't reflow comment lines starting with '@'.

[Krasimir Georgiev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL293617: [clang-format] Don't reflow comment lines starting 
with '@'. (authored by krasimir).

Changed prior to commit:
  https://reviews.llvm.org/D29323?vs=86405=86406#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D29323

Files:
  cfe/trunk/lib/Format/BreakableToken.cpp
  cfe/trunk/unittests/Format/FormatTest.cpp


Index: cfe/trunk/lib/Format/BreakableToken.cpp
===
--- cfe/trunk/lib/Format/BreakableToken.cpp
+++ cfe/trunk/lib/Format/BreakableToken.cpp
@@ -304,7 +304,9 @@
   // Simple heuristic for what to reflow: content should contain at least two
   // characters and either the first or second character must be
   // non-punctuation.
-  return Content.size() >= 2 && !Content.endswith("\\") &&
+  return Content.size() >= 2 &&
+ // Lines starting with '@' commonly have special meaning.
+ !Content.startswith("@") && !Content.endswith("\\") &&
  // Note that this is UTF-8 safe, since if isPunctuation(Content[0]) is
  // true, then the first code point must be 1 byte long.
  (!isPunctuation(Content[0]) || !isPunctuation(Content[1]));
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -2244,6 +2244,15 @@
 "// long long long long\n"
 "// ... --- ...",
 getLLVMStyleWithColumns(20)));
+
+  // Don't reflow lines starting with '@'.
+  EXPECT_EQ("// long long long\n"
+"// long\n"
+"// @param arg",
+format("// long long long long\n"
+   "// @param arg",
+   getLLVMStyleWithColumns(20)));
+
   // Reflow lines that have a non-punctuation character among their first 2
   // characters.
   EXPECT_EQ("// long long long\n"


Index: cfe/trunk/lib/Format/BreakableToken.cpp
===
--- cfe/trunk/lib/Format/BreakableToken.cpp
+++ cfe/trunk/lib/Format/BreakableToken.cpp
@@ -304,7 +304,9 @@
   // Simple heuristic for what to reflow: content should contain at least two
   // characters and either the first or second character must be
   // non-punctuation.
-  return Content.size() >= 2 && !Content.endswith("\\") &&
+  return Content.size() >= 2 &&
+ // Lines starting with '@' commonly have special meaning.
+ !Content.startswith("@") && !Content.endswith("\\") &&
  // Note that this is UTF-8 safe, since if isPunctuation(Content[0]) is
  // true, then the first code point must be 1 byte long.
  (!isPunctuation(Content[0]) || !isPunctuation(Content[1]));
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -2244,6 +2244,15 @@
 "// long long long long\n"
 "// ... --- ...",
 getLLVMStyleWithColumns(20)));
+
+  // Don't reflow lines starting with '@'.
+  EXPECT_EQ("// long long long\n"
+"// long\n"
+"// @param arg",
+format("// long long long long\n"
+   "// @param arg",
+   getLLVMStyleWithColumns(20)));
+
   // Reflow lines that have a non-punctuation character among their first 2
   // characters.
   EXPECT_EQ("// long long long\n"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D29323: [clang-format] Don't reflow comment lines starting with '@'.

[Daniel Jasper via Phabricator via cfe-commits]

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

Looks good.


https://reviews.llvm.org/D29323



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

[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros

[Daniel Jasper via Phabricator via cfe-commits]

djasper added a comment.

Sorry, it took a bit longer, but I have now submitted those changes in r293616.


Repository:
  rL LLVM

https://reviews.llvm.org/D28462



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

[PATCH] D29300: [clang-format] Refactor WhitespaceManager and friends

[Daniel Jasper via Phabricator via cfe-commits]

djasper closed this revision.
djasper added a comment.

Submitted as r293616.


https://reviews.llvm.org/D29300



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

r293616 - [clang-format] Refactor WhitespaceManager and friends

[Daniel Jasper via cfe-commits]

Author: djasper
Date: Tue Jan 31 05:25:01 2017
New Revision: 293616

URL: http://llvm.org/viewvc/llvm-project?rev=293616=rev
Log:
[clang-format] Refactor WhitespaceManager and friends

The main motivation behind this is to cleanup the WhitespaceManager and
make it more extensible for future alignment etc. features.
Specifically, WhitespaceManager has started to copy more and more code
that is already present in FormatToken. Instead, I think it makes more
sense to actually store a reference to each FormatToken for each change.

This has as a consequence led to a change in the calculation of indent
levels. Now, we actually compute them for each Token ahead of time,
which should be more efficient as it removes an unsigned value for the
ParenState, which is used during the combinatorial exploration of the
solution space.

No functional changes intended.

Review: https://reviews.llvm.org/D29300

Modified:
cfe/trunk/lib/Format/BreakableToken.cpp
cfe/trunk/lib/Format/BreakableToken.h
cfe/trunk/lib/Format/ContinuationIndenter.cpp
cfe/trunk/lib/Format/ContinuationIndenter.h
cfe/trunk/lib/Format/FormatToken.h
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
cfe/trunk/lib/Format/UnwrappedLineFormatter.h
cfe/trunk/lib/Format/WhitespaceManager.cpp
cfe/trunk/lib/Format/WhitespaceManager.h

Modified: cfe/trunk/lib/Format/BreakableToken.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/BreakableToken.cpp?rev=293616=293615=293616=diff
==
--- cfe/trunk/lib/Format/BreakableToken.cpp (original)
+++ cfe/trunk/lib/Format/BreakableToken.cpp Tue Jan 31 05:25:01 2017
@@ -181,10 +181,10 @@ unsigned BreakableSingleLineToken::getLi
 }
 
 BreakableSingleLineToken::BreakableSingleLineToken(
-const FormatToken , unsigned IndentLevel, unsigned StartColumn,
-StringRef Prefix, StringRef Postfix, bool InPPDirective,
-encoding::Encoding Encoding, const FormatStyle )
-: BreakableToken(Tok, IndentLevel, InPPDirective, Encoding, Style),
+const FormatToken , unsigned StartColumn, StringRef Prefix,
+StringRef Postfix, bool InPPDirective, encoding::Encoding Encoding,
+const FormatStyle )
+: BreakableToken(Tok, InPPDirective, Encoding, Style),
   StartColumn(StartColumn), Prefix(Prefix), Postfix(Postfix) {
   assert(Tok.TokenText.endswith(Postfix));
   Line = Tok.TokenText.substr(
@@ -192,11 +192,11 @@ BreakableSingleLineToken::BreakableSingl
 }
 
 BreakableStringLiteral::BreakableStringLiteral(
-const FormatToken , unsigned IndentLevel, unsigned StartColumn,
-StringRef Prefix, StringRef Postfix, bool InPPDirective,
-encoding::Encoding Encoding, const FormatStyle )
-: BreakableSingleLineToken(Tok, IndentLevel, StartColumn, Prefix, Postfix,
-   InPPDirective, Encoding, Style) {}
+const FormatToken , unsigned StartColumn, StringRef Prefix,
+StringRef Postfix, bool InPPDirective, encoding::Encoding Encoding,
+const FormatStyle )
+: BreakableSingleLineToken(Tok, StartColumn, Prefix, Postfix, 
InPPDirective,
+   Encoding, Style) {}
 
 BreakableToken::Split
 BreakableStringLiteral::getSplit(unsigned LineIndex, unsigned TailOffset,
@@ -218,16 +218,16 @@ void BreakableStringLiteral::insertBreak
 --LeadingSpaces;
   Whitespaces.replaceWhitespaceInToken(
   Tok, Prefix.size() + TailOffset + Split.first, Split.second, Postfix,
-  Prefix, InPPDirective, 1, IndentLevel, LeadingSpaces);
+  Prefix, InPPDirective, 1, LeadingSpaces);
 }
 
 BreakableComment::BreakableComment(const FormatToken ,
-   unsigned IndentLevel, unsigned StartColumn,
+   unsigned StartColumn,
unsigned OriginalStartColumn,
bool FirstInLine, bool InPPDirective,
encoding::Encoding Encoding,
const FormatStyle )
-: BreakableToken(Token, IndentLevel, InPPDirective, Encoding, Style),
+: BreakableToken(Token, InPPDirective, Encoding, Style),
   StartColumn(StartColumn), OriginalStartColumn(OriginalStartColumn),
   FirstInLine(FirstInLine) {}
 
@@ -254,8 +254,7 @@ void BreakableComment::compressWhitespac
   unsigned CharsToRemove = Split.second;
   Whitespaces.replaceWhitespaceInToken(
   tokenAt(LineIndex), BreakOffsetInToken, CharsToRemove, "", "",
-  /*InPPDirective=*/false,
-  /*Newlines=*/0, /*IndentLevel=*/0, /*Spaces=*/1);
+  /*InPPDirective=*/false, /*Newlines=*/0, /*Spaces=*/1);
 }
 
 BreakableToken::Split
@@ -319,11 +318,11 @@ bool BreakableComment::mayReflow(unsigne
 }
 
 BreakableBlockComment::BreakableBlockComment(
-const FormatToken , unsigned IndentLevel, unsigned StartColumn,
+const FormatToken , unsigned StartColumn,
 unsigned 

[PATCH] D29300: [clang-format] Refactor WhitespaceManager and friends

[Daniel Jasper via Phabricator via cfe-commits]

djasper updated this revision to Diff 86404.
djasper added a comment.

Added assert. Removed unused InToken.


https://reviews.llvm.org/D29300

Files:
  lib/Format/BreakableToken.cpp
  lib/Format/BreakableToken.h
  lib/Format/ContinuationIndenter.cpp
  lib/Format/ContinuationIndenter.h
  lib/Format/FormatToken.h
  lib/Format/TokenAnnotator.cpp
  lib/Format/UnwrappedLineFormatter.cpp
  lib/Format/UnwrappedLineFormatter.h
  lib/Format/WhitespaceManager.cpp
  lib/Format/WhitespaceManager.h

Index: lib/Format/WhitespaceManager.h
===
--- lib/Format/WhitespaceManager.h
+++ lib/Format/WhitespaceManager.h
@@ -43,8 +43,7 @@
 
   /// \brief Replaces the whitespace in front of \p Tok. Only call once for
   /// each \c AnnotatedToken.
-  void replaceWhitespace(FormatToken , unsigned Newlines,
- unsigned IndentLevel, unsigned Spaces,
+  void replaceWhitespace(FormatToken , unsigned Newlines, unsigned Spaces,
  unsigned StartOfTokenColumn,
  bool InPPDirective = false);
 
@@ -72,8 +71,7 @@
 unsigned ReplaceChars,
 StringRef PreviousPostfix,
 StringRef CurrentPrefix, bool InPPDirective,
-unsigned Newlines, unsigned IndentLevel,
-int Spaces);
+unsigned Newlines, int Spaces);
 
   /// \brief Returns all the \c Replacements created during formatting.
   const tooling::Replacements ();
@@ -91,8 +89,6 @@
   const SourceManager 
 };
 
-Change() {}
-
 /// \brief Creates a \c Change.
 ///
 /// The generated \c Change will replace the characters at
@@ -102,12 +98,17 @@
 ///
 /// \p StartOfTokenColumn and \p InPPDirective will be used to lay out
 /// trailing comments and escaped newlines.
-Change(bool CreateReplacement, SourceRange OriginalWhitespaceRange,
-   unsigned IndentLevel, int Spaces, unsigned StartOfTokenColumn,
-   unsigned NewlinesBefore, StringRef PreviousLinePostfix,
-   StringRef CurrentLinePrefix, tok::TokenKind Kind,
-   bool ContinuesPPDirective, bool IsStartOfDeclName,
-   bool IsInsideToken);
+Change(const FormatToken , bool CreateReplacement,
+   SourceRange OriginalWhitespaceRange, int Spaces,
+   unsigned StartOfTokenColumn, unsigned NewlinesBefore,
+   StringRef PreviousLinePostfix, StringRef CurrentLinePrefix,
+   bool ContinuesPPDirective, bool IsInsideToken);
+
+// The kind of the token whose whitespace this change replaces, or in which
+// this change inserts whitespace.
+// FIXME: Currently this is not set correctly for breaks inside comments, as
+// the \c BreakableToken is still doing its own alignment.
+const FormatToken *Tok;
 
 bool CreateReplacement;
 // Changes might be in the middle of a token, so we cannot just keep the
@@ -117,18 +118,7 @@
 unsigned NewlinesBefore;
 std::string PreviousLinePostfix;
 std::string CurrentLinePrefix;
-// The kind of the token whose whitespace this change replaces, or in which
-// this change inserts whitespace.
-// FIXME: Currently this is not set correctly for breaks inside comments, as
-// the \c BreakableToken is still doing its own alignment.
-tok::TokenKind Kind;
 bool ContinuesPPDirective;
-bool IsStartOfDeclName;
-
-// The number of nested blocks the token is in. This is used to add tabs
-// only for the indentation, and not for alignment, when
-// UseTab = US_ForIndentation.
-unsigned IndentLevel;
 
 // The number of spaces in front of the token or broken part of the token.
 // This will be adapted when aligning tokens.
Index: lib/Format/WhitespaceManager.cpp
===
--- lib/Format/WhitespaceManager.cpp
+++ lib/Format/WhitespaceManager.cpp
@@ -25,64 +25,60 @@
   C2.OriginalWhitespaceRange.getBegin());
 }
 
-WhitespaceManager::Change::Change(
-bool CreateReplacement, SourceRange OriginalWhitespaceRange,
-unsigned IndentLevel, int Spaces, unsigned StartOfTokenColumn,
-unsigned NewlinesBefore, StringRef PreviousLinePostfix,
-StringRef CurrentLinePrefix, tok::TokenKind Kind, bool ContinuesPPDirective,
-bool IsStartOfDeclName, bool IsInsideToken)
-: CreateReplacement(CreateReplacement),
+WhitespaceManager::Change::Change(const FormatToken ,
+  bool CreateReplacement,
+  SourceRange OriginalWhitespaceRange,
+  int Spaces, unsigned StartOfTokenColumn,
+  unsigned NewlinesBefore,
+  StringRef PreviousLinePostfix,
+  StringRef CurrentLinePrefix,
+

[PATCH] D29322: [clang-format] Fix regression merging comments across newlines.

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added inline comments.



Comment at: lib/Format/BreakableToken.cpp:685
 LineTok = CurrentTok->Next;
+if (CurrentTok->Next && CurrentTok->Next->NewlinesBefore > 1) {
+  // A line comment section needs to broken by a line comment that is

klimek wrote:
> Could we put them as children into the higher level UnwrappedLine instead?
Putting them as children looks cleaner; will try that.


https://reviews.llvm.org/D29322



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

[PATCH] D29300: [clang-format] Refactor WhitespaceManager and friends

[Manuel Klimek via Phabricator via cfe-commits]

klimek added a comment.

Generally looks like the right direction, minus that I'm not sure yet what the 
plan for things broken in BreakableToken are.




Comment at: lib/Format/TokenAnnotator.cpp:1843
+Current->MatchingParen->opensBlockOrBlockTypeList(Style))
+  --IndentLevel;
+Current->IndentLevel = IndentLevel;

Perhaps add an assert that we don't underflow.



Comment at: lib/Format/UnwrappedLineFormatter.cpp:904-907
+void UnwrappedLineFormatter::formatFirstToken(const AnnotatedLine ,
   const AnnotatedLine 
*PreviousLine,
-  unsigned IndentLevel,
-  unsigned Indent,
-  bool InPPDirective) {
+  unsigned Indent) {
+  FormatToken& RootToken = *Line.First;

I'm not sure I understand the change in the function signature. Given that we 
really only need InPPDirective and FirstToken, it seems unnecessary to hand in 
the whole line? (in the spirit of minimal interfaces)



Comment at: lib/Format/WhitespaceManager.h:109-110
+// this change inserts whitespace.
+// FIXME: Currently this is not set correctly for breaks inside comments, 
as
+// the \c BreakableToken is still doing its own alignment.
+const FormatToken *Tok;

What's the proposed fix?


https://reviews.llvm.org/D29300



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

r293611 - Revert r293585 "Add better ODR checking for modules."

[Sam McCall via cfe-commits]

Author: sammccall
Date: Tue Jan 31 02:24:40 2017
New Revision: 293611

URL: http://llvm.org/viewvc/llvm-project?rev=293611=rev
Log:
Revert r293585 "Add better ODR checking for modules."

We're seeing what we believe are false positives. (It's hard to tell with the
available diagnostics, and I'm not sure how to reduce them yet).
I'll send Richard reproduction details offline.

djasper/chandlerc suggested this should be a warning for now, to make rolling it
out feasible.

Removed:
cfe/trunk/include/clang/AST/ODRHash.h
cfe/trunk/lib/AST/ODRHash.cpp
cfe/trunk/test/Modules/odr_hash.cpp
Modified:
cfe/trunk/include/clang/AST/DeclCXX.h
cfe/trunk/include/clang/AST/Stmt.h
cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td
cfe/trunk/lib/AST/CMakeLists.txt
cfe/trunk/lib/AST/DeclCXX.cpp
cfe/trunk/lib/AST/StmtProfile.cpp
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/lib/Serialization/ASTReader.cpp
cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
cfe/trunk/lib/Serialization/ASTWriter.cpp
cfe/trunk/test/Modules/merge-using-decls.cpp

Modified: cfe/trunk/include/clang/AST/DeclCXX.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=293611=293610=293611=diff
==
--- cfe/trunk/include/clang/AST/DeclCXX.h (original)
+++ cfe/trunk/include/clang/AST/DeclCXX.h Tue Jan 31 02:24:40 2017
@@ -458,9 +458,6 @@ class CXXRecordDecl : public RecordDecl
 /// \brief Whether we are currently parsing base specifiers.
 unsigned IsParsingBaseSpecifiers : 1;
 
-/// \brief A hash of parts of the class to help in ODR checking.
-unsigned ODRHash;
-
 /// \brief The number of base class specifiers in Bases.
 unsigned NumBases;
 
@@ -706,9 +703,6 @@ public:
 return data().IsParsingBaseSpecifiers;
   }
 
-  void computeODRHash();
-  unsigned getODRHash() const { return data().ODRHash; }
-
   /// \brief Sets the base classes of this struct or class.
   void setBases(CXXBaseSpecifier const * const *Bases, unsigned NumBases);
 

Removed: cfe/trunk/include/clang/AST/ODRHash.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ODRHash.h?rev=293610=auto
==
--- cfe/trunk/include/clang/AST/ODRHash.h (original)
+++ cfe/trunk/include/clang/AST/ODRHash.h (removed)
@@ -1,80 +0,0 @@
-//===-- ODRHash.h - Hashing to diagnose ODR failures *- C++ 
-*-===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===--===//
-///
-/// \file
-/// This file contains the declaration of the ODRHash class, which calculates
-/// a hash based on AST nodes, which is stable across different runs.
-///
-//===--===//
-
-#include "clang/AST/DeclarationName.h"
-#include "clang/AST/Type.h"
-#include "clang/AST/TemplateBase.h"
-#include "llvm/ADT/DenseMap.h"
-#include "llvm/ADT/FoldingSet.h"
-#include "llvm/ADT/PointerUnion.h"
-#include "llvm/ADT/SmallVector.h"
-
-namespace clang {
-
-class Decl;
-class IdentifierInfo;
-class NestedNameSpecifer;
-class Stmt;
-class TemplateParameterList;
-
-// ODRHash is used to calculate a hash based on AST node contents that
-// does not rely on pointer addresses.  This allows the hash to not vary
-// between runs and is usable to detect ODR problems in modules.  To use,
-// construct an ODRHash object, then call Add* methods over the nodes that
-// need to be hashed.  Then call CalculateHash to get the hash value.
-// Typically, only one Add* call is needed.  clear can be called to reuse the
-// object.
-class ODRHash {
-  // Use DenseMaps to convert between Decl and Type pointers and an index 
value.
-  llvm::DenseMap DeclMap;
-  llvm::DenseMap TypeMap;
-
-  // Save space by processing bools at the end.
-  llvm::SmallVector Bools;
-
-  llvm::FoldingSetNodeID ID;
-
-public:
-  ODRHash() {}
-
-  // Use this for ODR checking classes between modules.  This method compares
-  // more information than the AddDecl class.
-  void AddCXXRecordDecl(const CXXRecordDecl *Record);
-
-  // Add AST nodes that need to be processes.  Some nodes are processed
-  // immediately while others are queued and processed later.
-  void AddDecl(const Decl *D);
-  void AddType(const Type *T);
-  void AddQualType(QualType T);
-  void AddStmt(const Stmt *S);
-  void AddIdentifierInfo(const IdentifierInfo *II);
-  void AddNestedNameSpecifier(const NestedNameSpecifier *NNS);
-  void AddTemplateName(TemplateName Name);
-  void AddDeclarationName(DeclarationName Name);
-  void AddTemplateArgument(TemplateArgument TA);
-  void AddTemplateParameterList(const TemplateParameterList *TPL);
-
-  // Reset the object