[PATCH] D50736: [libc++] Use correct rand.eng.mers all-zeroes seed sequence fallback

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast created this revision.
hubert.reinterpretcast added reviewers: mclow.lists, EricWF, jasonliu.
Herald added subscribers: ldionne, christof.

When a seed sequence would lead to having no non-zero significant bits in the 
initial state of a `mersenne_twister_engine`, the fallback is to flip the most 
significant bit of the first value that appears in the textual representation 
of the initial state.

rand.eng.mers describes this as setting the value to be 2 to the power of one 
less than w; the previous value encoded in the implementation, namely one less 
than "2 to the power of w", is replaced by the correct value in this patch.


Repository:
  rCXX libc++

https://reviews.llvm.org/D50736

Files:
  include/random
  test/libcxx/numerics/rand/rand.eng.mers/
  test/libcxx/numerics/rand/rand.eng.mers/cnstr_sseq_all_zero.pass.cpp

Index: test/libcxx/numerics/rand/rand.eng.mers/cnstr_sseq_all_zero.pass.cpp
===
--- /dev/null
+++ test/libcxx/numerics/rand/rand.eng.mers/cnstr_sseq_all_zero.pass.cpp
@@ -0,0 +1,74 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// 
+
+// template 
+// class mersenne_twister_engine;
+
+// template  explicit mersenne_twister_engine(Sseq &q);
+
+#include 
+
+#include 
+#include 
+#include 
+#include 
+
+struct all_zero_seed_seq {
+  typedef unsigned int result_type;
+
+  all_zero_seed_seq() {}
+
+  template 
+  all_zero_seed_seq(InputIterator, InputIterator) {}
+#if TEST_STD_VER >= 11
+  all_zero_seed_seq(std::initializer_list) {}
+#endif
+
+  template 
+  void generate(RandomAccessIterator rb, RandomAccessIterator re) {
+std::fill(rb, re, 0u);
+  }
+
+  std::size_t size() const { return 0u; }
+  template  void param(OutputIterator) const {}
+};
+
+template  void test(void) {
+  const std::size_t state_size = 1u;
+  const std::size_t shift_size = 1u;
+  const std::size_t tempering_l = word_size;
+
+  all_zero_seed_seq q;
+  std::mersenne_twister_engine
+  e(q);
+
+  const result_type Xneg1 = result_type(1) << (word_size - 1);
+  const result_type Y = Xneg1;
+  const result_type X0 = Xneg1 ^ (Y >> 1);
+  assert(e() == X0);
+}
+
+int main() {
+  // Test for k == 1: word_size <= 32.
+  test();
+
+  // Test for k == 2: (32 < word_size <= 64).
+  test();
+}
Index: include/random
===
--- include/random
+++ include/random
@@ -2337,7 +2337,7 @@
 for (size_t __i = 1; __i < __n; ++__i)
 if (__x_[__i] != 0)
 return;
-__x_[0] = _Max;
+__x_[0] = result_type(1) << (__w - 1);
 }
 }
 
@@ -2363,7 +2363,7 @@
 for (size_t __i = 1; __i < __n; ++__i)
 if (__x_[__i] != 0)
 return;
-__x_[0] = _Max;
+__x_[0] = result_type(1) << (__w - 1);
 }
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50736: [libc++] Use correct rand.eng.mers all-zeroes seed sequence fallback

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

In https://reviews.llvm.org/D50736#1200761, @mclow.lists wrote:

> Is this test that's being added libc++ specific, or would it apply to other 
> implementations as well?


The test can apply to other implementations as well (although I am not sure how 
the `initializer_list` include behaves under C++03 on other implementations). 
Is there some other place to put such tests?




Comment at: 
test/libcxx/numerics/rand/rand.eng.mers/cnstr_sseq_all_zero.pass.cpp:47
+
+template  void test(void) {
+  const std::size_t state_size = 1u;

mclow.lists wrote:
> Need a line break before `void`
I have no objection to adding a line break, but I would like to clarify that 
this line came straight out of a recent build of `clang-format` with 
`--style=llvm`. I would like to know whether there is something else here that 
I should be doing with `clang-format`, including if it is deemed that 
`clang-format` does not do the right thing here.


Repository:
  rCXX libc++

https://reviews.llvm.org/D50736



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

[PATCH] D50736: [libc++] Use correct rand.eng.mers all-zeroes seed sequence fallback

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

In https://reviews.llvm.org/D50736#1200774, @hubert.reinterpretcast wrote:

> In https://reviews.llvm.org/D50736#1200761, @mclow.lists wrote:
>
> > Is this test that's being added libc++ specific, or would it apply to other 
> > implementations as well?
>
>
> The test can apply to other implementations as well (although I am not sure 
> how the `initializer_list` include behaves under C++03 on other 
> implementations). Is there some other place to put such tests?


It seems that `test/std/numerics/rand/rand.eng/rand.eng.mers/` may be the place 
to add the test. I am looking into it.




Comment at: 
test/libcxx/numerics/rand/rand.eng.mers/cnstr_sseq_all_zero.pass.cpp:18
+
+// template  explicit mersenne_twister_engine(Sseq &q);
+

mclow.lists wrote:
> Please add a comment here about what's being tested, because when I look at 
> this a year from now, I won't know what's going on.
> 
> Something like:
> `//  Finally, if the most significant w − r bits of X−n are zero, and if each 
> of the other resulting Xi is 0, changes X−n to 2w−1.`
> 
I will add a comment to this effect. In an effort to be clear in the 
formatting, I will use the LaTeX snippet for the text.


Repository:
  rCXX libc++

https://reviews.llvm.org/D50736



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

[PATCH] D50736: [libc++] Use correct rand.eng.mers all-zeroes seed sequence fallback

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast updated this revision to Diff 160923.
hubert.reinterpretcast added a comment.

Address review comments by Marshall

Add a line break after the template-head. Add a comment with the requested 
quote from the Standard. Move test from the `test/libcxx/` tree to the 
`test/std/` tree, and guard against inclusion of `` under 
C++03.


Repository:
  rCXX libc++

https://reviews.llvm.org/D50736

Files:
  include/random
  test/std/numerics/rand/rand.eng/rand.eng.mers/ctor_sseq_all_zero.pass.cpp

Index: test/std/numerics/rand/rand.eng/rand.eng.mers/ctor_sseq_all_zero.pass.cpp
===
--- /dev/null
+++ test/std/numerics/rand/rand.eng/rand.eng.mers/ctor_sseq_all_zero.pass.cpp
@@ -0,0 +1,81 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// 
+
+// template 
+// class mersenne_twister_engine;
+
+// template  explicit mersenne_twister_engine(Sseq &q);
+//
+// [ ... ] Finally, if the most significant $w-r$ bits of $X_{-n}$ are zero,
+// and if each of the other resulting $X_i$ is $0$, changes $X_{-n}$ to
+// $ 2^{w-1} $.
+
+#include 
+
+#include 
+#include 
+#include 
+#if TEST_STD_VER >= 11
+#include 
+#endif
+
+struct all_zero_seed_seq {
+  typedef unsigned int result_type;
+
+  all_zero_seed_seq() {}
+
+  template 
+  all_zero_seed_seq(InputIterator, InputIterator) {}
+#if TEST_STD_VER >= 11
+  all_zero_seed_seq(std::initializer_list) {}
+#endif
+
+  template 
+  void generate(RandomAccessIterator rb, RandomAccessIterator re) {
+std::fill(rb, re, 0u);
+  }
+
+  std::size_t size() const { return 0u; }
+  template  void param(OutputIterator) const {}
+};
+
+template 
+void test(void) {
+  const std::size_t state_size = 1u;
+  const std::size_t shift_size = 1u;
+  const std::size_t tempering_l = word_size;
+
+  all_zero_seed_seq q;
+  std::mersenne_twister_engine
+  e(q);
+
+  const result_type Xneg1 = result_type(1) << (word_size - 1);
+  const result_type Y = Xneg1;
+  const result_type X0 = Xneg1 ^ (Y >> 1);
+  assert(e() == X0);
+}
+
+int main() {
+  // Test for k == 1: word_size <= 32.
+  test();
+
+  // Test for k == 2: (32 < word_size <= 64).
+  test();
+}
Index: include/random
===
--- include/random
+++ include/random
@@ -2337,7 +2337,7 @@
 for (size_t __i = 1; __i < __n; ++__i)
 if (__x_[__i] != 0)
 return;
-__x_[0] = _Max;
+__x_[0] = result_type(1) << (__w - 1);
 }
 }
 
@@ -2363,7 +2363,7 @@
 for (size_t __i = 1; __i < __n; ++__i)
 if (__x_[__i] != 0)
 return;
-__x_[0] = _Max;
+__x_[0] = result_type(1) << (__w - 1);
 }
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50736: [libc++] Use correct rand.eng.mers all-zeroes seed sequence fallback

[Hubert Tong via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCXX339969: [libc++] Use correct rand.eng.mers all-zeroes seed 
sequence fallback (authored by hubert.reinterpretcast, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D50736?vs=160923&id=161145#toc

Repository:
  rCXX libc++

https://reviews.llvm.org/D50736

Files:
  include/random
  test/std/numerics/rand/rand.eng/rand.eng.mers/ctor_sseq_all_zero.pass.cpp

Index: include/random
===
--- include/random
+++ include/random
@@ -2337,7 +2337,7 @@
 for (size_t __i = 1; __i < __n; ++__i)
 if (__x_[__i] != 0)
 return;
-__x_[0] = _Max;
+__x_[0] = result_type(1) << (__w - 1);
 }
 }
 
@@ -2363,7 +2363,7 @@
 for (size_t __i = 1; __i < __n; ++__i)
 if (__x_[__i] != 0)
 return;
-__x_[0] = _Max;
+__x_[0] = result_type(1) << (__w - 1);
 }
 }
 
Index: test/std/numerics/rand/rand.eng/rand.eng.mers/ctor_sseq_all_zero.pass.cpp
===
--- test/std/numerics/rand/rand.eng/rand.eng.mers/ctor_sseq_all_zero.pass.cpp
+++ test/std/numerics/rand/rand.eng/rand.eng.mers/ctor_sseq_all_zero.pass.cpp
@@ -0,0 +1,81 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// 
+
+// template 
+// class mersenne_twister_engine;
+
+// template  explicit mersenne_twister_engine(Sseq &q);
+//
+// [ ... ] Finally, if the most significant $w-r$ bits of $X_{-n}$ are zero,
+// and if each of the other resulting $X_i$ is $0$, changes $X_{-n}$ to
+// $ 2^{w-1} $.
+
+#include 
+
+#include 
+#include 
+#include 
+#if TEST_STD_VER >= 11
+#include 
+#endif
+
+struct all_zero_seed_seq {
+  typedef unsigned int result_type;
+
+  all_zero_seed_seq() {}
+
+  template 
+  all_zero_seed_seq(InputIterator, InputIterator) {}
+#if TEST_STD_VER >= 11
+  all_zero_seed_seq(std::initializer_list) {}
+#endif
+
+  template 
+  void generate(RandomAccessIterator rb, RandomAccessIterator re) {
+std::fill(rb, re, 0u);
+  }
+
+  std::size_t size() const { return 0u; }
+  template  void param(OutputIterator) const {}
+};
+
+template 
+void test(void) {
+  const std::size_t state_size = 1u;
+  const std::size_t shift_size = 1u;
+  const std::size_t tempering_l = word_size;
+
+  all_zero_seed_seq q;
+  std::mersenne_twister_engine
+  e(q);
+
+  const result_type Xneg1 = result_type(1) << (word_size - 1);
+  const result_type Y = Xneg1;
+  const result_type X0 = Xneg1 ^ (Y >> 1);
+  assert(e() == X0);
+}
+
+int main() {
+  // Test for k == 1: word_size <= 32.
+  test();
+
+  // Test for k == 2: (32 < word_size <= 64).
+  test();
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50055: Update the coding standard about NFC changes and whitespace

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: docs/DeveloperPolicy.rst:395-408
+Commits with No Functional Change
+-
+
+It may be permissible to commit changes without prior review when the changes
+have no semantic impact on the code if the changes being made are obvious and
+not invasive. For instance, removing trailing whitespace from a line, fixing a
+line ending to be consistent with the rest of the file, fixing a typo, code

hfinkel wrote:
> aaron.ballman wrote:
> > chandlerc wrote:
> > > I think this is a much broader statement than is warranted to address the 
> > > specific problem. And I'm not completely comfortable with it.
> > > 
> > > I don't think guidance around "no functional change" is the right way to 
> > > give guidance about what is or isn't "obvious" and fine to commit with 
> > > post-commit review personally.
> > > 
> > > I'd really suggest just being direct about *formatting* and whitespace 
> > > changes, and specifically suggest that they not be made unless the file 
> > > or code region in question is an area that the author is planning 
> > > subsequent changes to.
> > We talk about formatting and whitespace in the CodingStandards document, 
> > but we talk about obviousness and post-commit review in DeveloperPolicy. 
> > Where would you like these new words to live? To me, they're more about the 
> > policy and less about the coding standard -- the coding standard says what 
> > the code should look like and the policy says what you should and should 
> > not do procedurally, but then I feel the need to tie it back to 
> > "obviousness". How about this in the developer policy:
> > ```
> > The Obviousness of Formatting Changes
> > -
> > 
> > While formatting and whitespace changes may be "obvious", they can also 
> > create
> > needless churn that causes difficulties for out-of-tree users carrying local
> > patches. Do not commit formatting or whitespace changes unless they are 
> > related
> > to a file or code region that you intend to make subsequent changes to. The
> > formatting and whitespace changes should be highly localized, committed 
> > before
> > you begin functionality-changing work on the code region, and the commit 
> > message
> > should clearly state that the commit is not intended to change 
> > functionality,
> > usually by stating it is :ref:`NFC `.
> > 
> > 
> > If you wish to make a change to formatting or whitespace that touches an 
> > entire
> > library or code base, please seek pre-commit approval first.
> > ```
> I agree with @chandlerc about the current proposed text, and I think that 
> this is better. I wonder if we want to insist on separating the commits, of 
> if, combined localized commits are okay?
> 
It depends on how much noise there is when combining the commits; and when 
evaluating for that, we have to remember that people use different diff tools.


https://reviews.llvm.org/D50055



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

[PATCH] D33833: Fix PR 33189: Clang assertion on template destructor declaration

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: lib/AST/DeclCXX.cpp:1420
   DeclContext::lookup_result R = lookup(Name);
-  if (R.empty())
+  if (R.empty() || !isa(R.front()))
 return nullptr;

As it is,
```
return R.empty() ? nullptr : dyn_cast(R.front());
```
would probably be much more succinct.



https://reviews.llvm.org/D33833



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

[PATCH] D33833: Fix PR 33189: Clang assertion on template destructor declaration

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

In https://reviews.llvm.org/D33833#789436, @kuang_he wrote:

> Can we get this patch reviewed by any chance?


@kuang_he; it is customary to "ping". In this case, "Ping 2".




Comment at: lib/AST/DeclCXX.cpp:1421
 
-  CXXDestructorDecl *Dtor = cast(R.front());
-  return Dtor;
+  return R.empty() ? nullptr : dyn_cast(R.front());
 }

I think what is here is probably what we want to do. My understanding is that 
the code is certainly invalid before we hit this, and the case is so odd that 
pursuing better recovery is not worthwhile.

Do we know if we can recover from getting a `FunctionTemplateDecl` by some 
other means? Perhaps by using the result of `getTemplatedDecl()`? I suspect 
there may be problems with cases where the //noexcept-specifier// is dependent.


https://reviews.llvm.org/D33833



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

[PATCH] D41544: Use backslash escape, replacing xargs -0 in test macro-multiline.c

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast created this revision.

xargs supports escaping of newline characters with backslash. xargs -0 is 
neither part of POSIX nor the LSB.

This patch removes the -0 option and adjusts the input to xargs accordingly; 
that is, the input is a text file not ending in an incomplete line, and the 
newline of interest is preceded by a backslash.

Note: The treatment of escaped newline characters is not as clearly specified 
by POSIX as for escaped blank characters; however, the same can be said for 
escaped backslashes. It is slightly more clear for the case where the -I option 
is used; however, -I is also of limited portability.


Repository:
  rC Clang

https://reviews.llvm.org/D41544

Files:
  test/Preprocessor/macro-multiline.c


Index: test/Preprocessor/macro-multiline.c
===
--- test/Preprocessor/macro-multiline.c
+++ test/Preprocessor/macro-multiline.c
@@ -1,4 +1,4 @@
-// RUN: printf -- "-DX=A\nTHIS_SHOULD_NOT_EXIST_IN_THE_OUTPUT" | xargs -0 
%clang -E %s | FileCheck -strict-whitespace %s
+// RUN: printf -- "-DX=A\nTHIS_SHOULD_NOT_EXIST_IN_THE_OUTPUT\n" | xargs 
%clang -E %s | FileCheck -strict-whitespace %s
 
 // Per GCC -D semantics, \n and anything that follows is ignored.
 


Index: test/Preprocessor/macro-multiline.c
===
--- test/Preprocessor/macro-multiline.c
+++ test/Preprocessor/macro-multiline.c
@@ -1,4 +1,4 @@
-// RUN: printf -- "-DX=A\nTHIS_SHOULD_NOT_EXIST_IN_THE_OUTPUT" | xargs -0 %clang -E %s | FileCheck -strict-whitespace %s
+// RUN: printf -- "-DX=A\nTHIS_SHOULD_NOT_EXIST_IN_THE_OUTPUT\n" | xargs %clang -E %s | FileCheck -strict-whitespace %s
 
 // Per GCC -D semantics, \n and anything that follows is ignored.
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41545: Replace cp -a in various Clang tests

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast created this revision.

`cp -a` is neither part of POSIX nor the LSB; this patch uses `cp -RPp`, the 
nearest equivalent under POSIX.

The tree being copied in each case currently contains only directories and 
regular files; so the `-P` is superfluous.

`test/Modules/crash-vfs-headermaps.m` is not updated since it requires 
`system-darwin` anyway.


Repository:
  rC Clang

https://reviews.llvm.org/D41545

Files:
  test/Modules/crash-vfs-path-emptydir-entries.m
  test/Modules/crash-vfs-path-symlink-component.m
  test/Modules/crash-vfs-path-symlink-topheader.m
  test/Modules/crash-vfs-umbrella-frameworks.m
  test/VFS/umbrella-framework-import-skipnonexist.m


Index: test/VFS/umbrella-framework-import-skipnonexist.m
===
--- test/VFS/umbrella-framework-import-skipnonexist.m
+++ test/VFS/umbrella-framework-import-skipnonexist.m
@@ -5,7 +5,7 @@
 
 // RUN: rm -rf %t
 // RUN: mkdir -p %t/vdir %t/outdir %t/cache
-// RUN: cp -a %S/Inputs/Bar.framework %t/outdir/
+// RUN: cp -RPp %S/Inputs/Bar.framework %t/outdir/
 //
 // RUN: sed -e "s:VDIR:%t/vdir:g" -e "s:OUT_DIR:%t/outdir:g" 
%S/Inputs/bar-headers.yaml > %t/vdir/bar-headers.yaml
 // RUN: rm -f %t/outdir/Bar.framework/Headers/B.h
Index: test/Modules/crash-vfs-umbrella-frameworks.m
===
--- test/Modules/crash-vfs-umbrella-frameworks.m
+++ test/Modules/crash-vfs-umbrella-frameworks.m
@@ -5,7 +5,7 @@
 
 // RUN: rm -rf %t
 // RUN: mkdir -p %t/i %t/m %t
-// RUN: cp -a %S/Inputs/crash-recovery/Frameworks %t/i/
+// RUN: cp -RPp %S/Inputs/crash-recovery/Frameworks %t/i/
 // RUN: mkdir -p %t/i/Frameworks/A.framework/Frameworks
 // RUN: ln -s ../../B.framework 
%t/i/Frameworks/A.framework/Frameworks/B.framework
 
Index: test/Modules/crash-vfs-path-symlink-topheader.m
===
--- test/Modules/crash-vfs-path-symlink-topheader.m
+++ test/Modules/crash-vfs-path-symlink-topheader.m
@@ -8,7 +8,7 @@
 
 // RUN: rm -rf %t
 // RUN: mkdir -p %t/i %t/m %t %t/sysroot
-// RUN: cp -a %S/Inputs/crash-recovery/usr %t/i/
+// RUN: cp -RPp %S/Inputs/crash-recovery/usr %t/i/
 // RUN: rm -f %t/i/usr/include/pthread_impl.h
 // RUN: ln -s pthread/pthread_impl.h %t/i/usr/include/pthread_impl.h
 
Index: test/Modules/crash-vfs-path-symlink-component.m
===
--- test/Modules/crash-vfs-path-symlink-component.m
+++ test/Modules/crash-vfs-path-symlink-component.m
@@ -8,7 +8,7 @@
 
 // RUN: rm -rf %t
 // RUN: mkdir -p %t/i %t/m %t %t/sysroot
-// RUN: cp -a %S/Inputs/crash-recovery/usr %t/i/
+// RUN: cp -RPp %S/Inputs/crash-recovery/usr %t/i/
 // RUN: ln -s include/tcl-private %t/i/usr/x
 
 // RUN: not env FORCE_CLANG_DIAGNOSTICS_CRASH= TMPDIR=%t TEMP=%t TMP=%t \
Index: test/Modules/crash-vfs-path-emptydir-entries.m
===
--- test/Modules/crash-vfs-path-emptydir-entries.m
+++ test/Modules/crash-vfs-path-emptydir-entries.m
@@ -8,7 +8,7 @@
 
 // RUN: rm -rf %t
 // RUN: mkdir -p %t/i %t/m %t %t/sysroot
-// RUN: cp -a %S/Inputs/crash-recovery/usr %t/i/
+// RUN: cp -RPp %S/Inputs/crash-recovery/usr %t/i/
 
 // RUN: not env FORCE_CLANG_DIAGNOSTICS_CRASH= TMPDIR=%t TEMP=%t TMP=%t \
 // RUN: %clang -fsyntax-only %s -I %/t/i -isysroot %/t/sysroot/ \


Index: test/VFS/umbrella-framework-import-skipnonexist.m
===
--- test/VFS/umbrella-framework-import-skipnonexist.m
+++ test/VFS/umbrella-framework-import-skipnonexist.m
@@ -5,7 +5,7 @@
 
 // RUN: rm -rf %t
 // RUN: mkdir -p %t/vdir %t/outdir %t/cache
-// RUN: cp -a %S/Inputs/Bar.framework %t/outdir/
+// RUN: cp -RPp %S/Inputs/Bar.framework %t/outdir/
 //
 // RUN: sed -e "s:VDIR:%t/vdir:g" -e "s:OUT_DIR:%t/outdir:g" %S/Inputs/bar-headers.yaml > %t/vdir/bar-headers.yaml
 // RUN: rm -f %t/outdir/Bar.framework/Headers/B.h
Index: test/Modules/crash-vfs-umbrella-frameworks.m
===
--- test/Modules/crash-vfs-umbrella-frameworks.m
+++ test/Modules/crash-vfs-umbrella-frameworks.m
@@ -5,7 +5,7 @@
 
 // RUN: rm -rf %t
 // RUN: mkdir -p %t/i %t/m %t
-// RUN: cp -a %S/Inputs/crash-recovery/Frameworks %t/i/
+// RUN: cp -RPp %S/Inputs/crash-recovery/Frameworks %t/i/
 // RUN: mkdir -p %t/i/Frameworks/A.framework/Frameworks
 // RUN: ln -s ../../B.framework %t/i/Frameworks/A.framework/Frameworks/B.framework
 
Index: test/Modules/crash-vfs-path-symlink-topheader.m
===
--- test/Modules/crash-vfs-path-symlink-topheader.m
+++ test/Modules/crash-vfs-path-symlink-topheader.m
@@ -8,7 +8,7 @@
 
 // RUN: rm -rf %t
 // RUN: mkdir -p %t/i %t/m %t %t/sysroot
-// RUN: cp -a %S/Inputs/crash-recovery/usr %t/i/
+// RUN: cp -RPp %S/Inputs/crash-recovery/usr %t/i/
 // RUN: rm -f %t/i/usr/

[PATCH] D41544: Use backslash escape, replacing xargs -0 in test macro-multiline.c

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

Ping


Repository:
  rC Clang

https://reviews.llvm.org/D41544



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

[PATCH] D41545: Replace cp -a in various Clang tests

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

Maybe `cp -R` is sufficient? `cp -RPp` was the just-to-be-safe "minimal change".


Repository:
  rC Clang

https://reviews.llvm.org/D41545



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

[PATCH] D41545: Replace cp -a in various Clang tests

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

I'll commit with `cp -R` tomorrow then; thanks.


Repository:
  rC Clang

https://reviews.llvm.org/D41545



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

[PATCH] D41545: Replace cp -a in various Clang tests

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast updated this revision to Diff 128584.
hubert.reinterpretcast added a comment.

Use cp -R, which is sufficient instead of cp -RPp


Repository:
  rC Clang

https://reviews.llvm.org/D41545

Files:
  test/Modules/crash-vfs-path-emptydir-entries.m
  test/Modules/crash-vfs-path-symlink-component.m
  test/Modules/crash-vfs-path-symlink-topheader.m
  test/Modules/crash-vfs-umbrella-frameworks.m
  test/VFS/umbrella-framework-import-skipnonexist.m


Index: test/VFS/umbrella-framework-import-skipnonexist.m
===
--- test/VFS/umbrella-framework-import-skipnonexist.m
+++ test/VFS/umbrella-framework-import-skipnonexist.m
@@ -5,7 +5,7 @@
 
 // RUN: rm -rf %t
 // RUN: mkdir -p %t/vdir %t/outdir %t/cache
-// RUN: cp -a %S/Inputs/Bar.framework %t/outdir/
+// RUN: cp -R %S/Inputs/Bar.framework %t/outdir/
 //
 // RUN: sed -e "s:VDIR:%t/vdir:g" -e "s:OUT_DIR:%t/outdir:g" 
%S/Inputs/bar-headers.yaml > %t/vdir/bar-headers.yaml
 // RUN: rm -f %t/outdir/Bar.framework/Headers/B.h
Index: test/Modules/crash-vfs-umbrella-frameworks.m
===
--- test/Modules/crash-vfs-umbrella-frameworks.m
+++ test/Modules/crash-vfs-umbrella-frameworks.m
@@ -5,7 +5,7 @@
 
 // RUN: rm -rf %t
 // RUN: mkdir -p %t/i %t/m %t
-// RUN: cp -a %S/Inputs/crash-recovery/Frameworks %t/i/
+// RUN: cp -R %S/Inputs/crash-recovery/Frameworks %t/i/
 // RUN: mkdir -p %t/i/Frameworks/A.framework/Frameworks
 // RUN: ln -s ../../B.framework 
%t/i/Frameworks/A.framework/Frameworks/B.framework
 
Index: test/Modules/crash-vfs-path-symlink-topheader.m
===
--- test/Modules/crash-vfs-path-symlink-topheader.m
+++ test/Modules/crash-vfs-path-symlink-topheader.m
@@ -8,7 +8,7 @@
 
 // RUN: rm -rf %t
 // RUN: mkdir -p %t/i %t/m %t %t/sysroot
-// RUN: cp -a %S/Inputs/crash-recovery/usr %t/i/
+// RUN: cp -R %S/Inputs/crash-recovery/usr %t/i/
 // RUN: rm -f %t/i/usr/include/pthread_impl.h
 // RUN: ln -s pthread/pthread_impl.h %t/i/usr/include/pthread_impl.h
 
Index: test/Modules/crash-vfs-path-symlink-component.m
===
--- test/Modules/crash-vfs-path-symlink-component.m
+++ test/Modules/crash-vfs-path-symlink-component.m
@@ -8,7 +8,7 @@
 
 // RUN: rm -rf %t
 // RUN: mkdir -p %t/i %t/m %t %t/sysroot
-// RUN: cp -a %S/Inputs/crash-recovery/usr %t/i/
+// RUN: cp -R %S/Inputs/crash-recovery/usr %t/i/
 // RUN: ln -s include/tcl-private %t/i/usr/x
 
 // RUN: not env FORCE_CLANG_DIAGNOSTICS_CRASH= TMPDIR=%t TEMP=%t TMP=%t \
Index: test/Modules/crash-vfs-path-emptydir-entries.m
===
--- test/Modules/crash-vfs-path-emptydir-entries.m
+++ test/Modules/crash-vfs-path-emptydir-entries.m
@@ -8,7 +8,7 @@
 
 // RUN: rm -rf %t
 // RUN: mkdir -p %t/i %t/m %t %t/sysroot
-// RUN: cp -a %S/Inputs/crash-recovery/usr %t/i/
+// RUN: cp -R %S/Inputs/crash-recovery/usr %t/i/
 
 // RUN: not env FORCE_CLANG_DIAGNOSTICS_CRASH= TMPDIR=%t TEMP=%t TMP=%t \
 // RUN: %clang -fsyntax-only %s -I %/t/i -isysroot %/t/sysroot/ \


Index: test/VFS/umbrella-framework-import-skipnonexist.m
===
--- test/VFS/umbrella-framework-import-skipnonexist.m
+++ test/VFS/umbrella-framework-import-skipnonexist.m
@@ -5,7 +5,7 @@
 
 // RUN: rm -rf %t
 // RUN: mkdir -p %t/vdir %t/outdir %t/cache
-// RUN: cp -a %S/Inputs/Bar.framework %t/outdir/
+// RUN: cp -R %S/Inputs/Bar.framework %t/outdir/
 //
 // RUN: sed -e "s:VDIR:%t/vdir:g" -e "s:OUT_DIR:%t/outdir:g" %S/Inputs/bar-headers.yaml > %t/vdir/bar-headers.yaml
 // RUN: rm -f %t/outdir/Bar.framework/Headers/B.h
Index: test/Modules/crash-vfs-umbrella-frameworks.m
===
--- test/Modules/crash-vfs-umbrella-frameworks.m
+++ test/Modules/crash-vfs-umbrella-frameworks.m
@@ -5,7 +5,7 @@
 
 // RUN: rm -rf %t
 // RUN: mkdir -p %t/i %t/m %t
-// RUN: cp -a %S/Inputs/crash-recovery/Frameworks %t/i/
+// RUN: cp -R %S/Inputs/crash-recovery/Frameworks %t/i/
 // RUN: mkdir -p %t/i/Frameworks/A.framework/Frameworks
 // RUN: ln -s ../../B.framework %t/i/Frameworks/A.framework/Frameworks/B.framework
 
Index: test/Modules/crash-vfs-path-symlink-topheader.m
===
--- test/Modules/crash-vfs-path-symlink-topheader.m
+++ test/Modules/crash-vfs-path-symlink-topheader.m
@@ -8,7 +8,7 @@
 
 // RUN: rm -rf %t
 // RUN: mkdir -p %t/i %t/m %t %t/sysroot
-// RUN: cp -a %S/Inputs/crash-recovery/usr %t/i/
+// RUN: cp -R %S/Inputs/crash-recovery/usr %t/i/
 // RUN: rm -f %t/i/usr/include/pthread_impl.h
 // RUN: ln -s pthread/pthread_impl.h %t/i/usr/include/pthread_impl.h
 
Index: test/Modules/crash-vfs-path-symlink-component.m
===
--- test/Modules/cra

[PATCH] D41545: Replace cp -a in various Clang tests

[Hubert Tong via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC321778: Replace cp -a in various Clang tests (authored by 
hubert.reinterpretcast, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D41545?vs=128584&id=128586#toc

Repository:
  rC Clang

https://reviews.llvm.org/D41545

Files:
  test/Modules/crash-vfs-path-emptydir-entries.m
  test/Modules/crash-vfs-path-symlink-component.m
  test/Modules/crash-vfs-path-symlink-topheader.m
  test/Modules/crash-vfs-umbrella-frameworks.m
  test/VFS/umbrella-framework-import-skipnonexist.m


Index: test/VFS/umbrella-framework-import-skipnonexist.m
===
--- test/VFS/umbrella-framework-import-skipnonexist.m
+++ test/VFS/umbrella-framework-import-skipnonexist.m
@@ -5,7 +5,7 @@
 
 // RUN: rm -rf %t
 // RUN: mkdir -p %t/vdir %t/outdir %t/cache
-// RUN: cp -a %S/Inputs/Bar.framework %t/outdir/
+// RUN: cp -R %S/Inputs/Bar.framework %t/outdir/
 //
 // RUN: sed -e "s:VDIR:%t/vdir:g" -e "s:OUT_DIR:%t/outdir:g" 
%S/Inputs/bar-headers.yaml > %t/vdir/bar-headers.yaml
 // RUN: rm -f %t/outdir/Bar.framework/Headers/B.h
Index: test/Modules/crash-vfs-path-symlink-component.m
===
--- test/Modules/crash-vfs-path-symlink-component.m
+++ test/Modules/crash-vfs-path-symlink-component.m
@@ -8,7 +8,7 @@
 
 // RUN: rm -rf %t
 // RUN: mkdir -p %t/i %t/m %t %t/sysroot
-// RUN: cp -a %S/Inputs/crash-recovery/usr %t/i/
+// RUN: cp -R %S/Inputs/crash-recovery/usr %t/i/
 // RUN: ln -s include/tcl-private %t/i/usr/x
 
 // RUN: not env FORCE_CLANG_DIAGNOSTICS_CRASH= TMPDIR=%t TEMP=%t TMP=%t \
Index: test/Modules/crash-vfs-umbrella-frameworks.m
===
--- test/Modules/crash-vfs-umbrella-frameworks.m
+++ test/Modules/crash-vfs-umbrella-frameworks.m
@@ -5,7 +5,7 @@
 
 // RUN: rm -rf %t
 // RUN: mkdir -p %t/i %t/m %t
-// RUN: cp -a %S/Inputs/crash-recovery/Frameworks %t/i/
+// RUN: cp -R %S/Inputs/crash-recovery/Frameworks %t/i/
 // RUN: mkdir -p %t/i/Frameworks/A.framework/Frameworks
 // RUN: ln -s ../../B.framework 
%t/i/Frameworks/A.framework/Frameworks/B.framework
 
Index: test/Modules/crash-vfs-path-symlink-topheader.m
===
--- test/Modules/crash-vfs-path-symlink-topheader.m
+++ test/Modules/crash-vfs-path-symlink-topheader.m
@@ -8,7 +8,7 @@
 
 // RUN: rm -rf %t
 // RUN: mkdir -p %t/i %t/m %t %t/sysroot
-// RUN: cp -a %S/Inputs/crash-recovery/usr %t/i/
+// RUN: cp -R %S/Inputs/crash-recovery/usr %t/i/
 // RUN: rm -f %t/i/usr/include/pthread_impl.h
 // RUN: ln -s pthread/pthread_impl.h %t/i/usr/include/pthread_impl.h
 
Index: test/Modules/crash-vfs-path-emptydir-entries.m
===
--- test/Modules/crash-vfs-path-emptydir-entries.m
+++ test/Modules/crash-vfs-path-emptydir-entries.m
@@ -8,7 +8,7 @@
 
 // RUN: rm -rf %t
 // RUN: mkdir -p %t/i %t/m %t %t/sysroot
-// RUN: cp -a %S/Inputs/crash-recovery/usr %t/i/
+// RUN: cp -R %S/Inputs/crash-recovery/usr %t/i/
 
 // RUN: not env FORCE_CLANG_DIAGNOSTICS_CRASH= TMPDIR=%t TEMP=%t TMP=%t \
 // RUN: %clang -fsyntax-only %s -I %/t/i -isysroot %/t/sysroot/ \


Index: test/VFS/umbrella-framework-import-skipnonexist.m
===
--- test/VFS/umbrella-framework-import-skipnonexist.m
+++ test/VFS/umbrella-framework-import-skipnonexist.m
@@ -5,7 +5,7 @@
 
 // RUN: rm -rf %t
 // RUN: mkdir -p %t/vdir %t/outdir %t/cache
-// RUN: cp -a %S/Inputs/Bar.framework %t/outdir/
+// RUN: cp -R %S/Inputs/Bar.framework %t/outdir/
 //
 // RUN: sed -e "s:VDIR:%t/vdir:g" -e "s:OUT_DIR:%t/outdir:g" %S/Inputs/bar-headers.yaml > %t/vdir/bar-headers.yaml
 // RUN: rm -f %t/outdir/Bar.framework/Headers/B.h
Index: test/Modules/crash-vfs-path-symlink-component.m
===
--- test/Modules/crash-vfs-path-symlink-component.m
+++ test/Modules/crash-vfs-path-symlink-component.m
@@ -8,7 +8,7 @@
 
 // RUN: rm -rf %t
 // RUN: mkdir -p %t/i %t/m %t %t/sysroot
-// RUN: cp -a %S/Inputs/crash-recovery/usr %t/i/
+// RUN: cp -R %S/Inputs/crash-recovery/usr %t/i/
 // RUN: ln -s include/tcl-private %t/i/usr/x
 
 // RUN: not env FORCE_CLANG_DIAGNOSTICS_CRASH= TMPDIR=%t TEMP=%t TMP=%t \
Index: test/Modules/crash-vfs-umbrella-frameworks.m
===
--- test/Modules/crash-vfs-umbrella-frameworks.m
+++ test/Modules/crash-vfs-umbrella-frameworks.m
@@ -5,7 +5,7 @@
 
 // RUN: rm -rf %t
 // RUN: mkdir -p %t/i %t/m %t
-// RUN: cp -a %S/Inputs/crash-recovery/Frameworks %t/i/
+// RUN: cp -R %S/Inputs/crash-recovery/Frameworks %t/i/
 // RUN: mkdir -p %t/i/Frameworks/A.framework/Frameworks
 // RUN: ln -s ../../B.framework %t/i/Frameworks/A.framework/Frameworks/B.framework
 
Index: test/Modu

[PATCH] D41544: Use backslash escape, replacing xargs -0 in test macro-multiline.c

[Hubert Tong via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC321828: Use backslash escape, replacing xargs -0 in test 
macro-multiline.c (authored by hubert.reinterpretcast, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D41544?vs=128034&id=128658#toc

Repository:
  rC Clang

https://reviews.llvm.org/D41544

Files:
  test/Preprocessor/macro-multiline.c


Index: test/Preprocessor/macro-multiline.c
===
--- test/Preprocessor/macro-multiline.c
+++ test/Preprocessor/macro-multiline.c
@@ -1,4 +1,4 @@
-// RUN: printf -- "-DX=A\nTHIS_SHOULD_NOT_EXIST_IN_THE_OUTPUT" | xargs -0 
%clang -E %s | FileCheck -strict-whitespace %s
+// RUN: printf -- "-DX=A\nTHIS_SHOULD_NOT_EXIST_IN_THE_OUTPUT\n" | xargs 
%clang -E %s | FileCheck -strict-whitespace %s
 
 // Per GCC -D semantics, \n and anything that follows is ignored.
 


Index: test/Preprocessor/macro-multiline.c
===
--- test/Preprocessor/macro-multiline.c
+++ test/Preprocessor/macro-multiline.c
@@ -1,4 +1,4 @@
-// RUN: printf -- "-DX=A\nTHIS_SHOULD_NOT_EXIST_IN_THE_OUTPUT" | xargs -0 %clang -E %s | FileCheck -strict-whitespace %s
+// RUN: printf -- "-DX=A\nTHIS_SHOULD_NOT_EXIST_IN_THE_OUTPUT\n" | xargs %clang -E %s | FileCheck -strict-whitespace %s
 
 // Per GCC -D semantics, \n and anything that follows is ignored.
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41217: [Concepts] Concept Specialization Expressions

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: lib/AST/ExprCXX.cpp:1478
+  {
+// We do not want error diagnostics escaping here.
+Sema::SFINAETrap Trap(S);

saar.raz wrote:
> faisalv wrote:
> > Hubert: This needs a TODO: the idea is not to drop SFINAE errors, but to 
> > avoid instantiation that may trigger errors not in the immediate context of 
> > instantiation. The substitution needs to happen piecewise.
> Could you elaborate/give an example where this handling is inappropriate?
Determining satisfaction requires normalization (lazy normalization should be 
considered).
The determination of satisfaction then proceeds by handling the left-hand side 
of conjunction and disjunction constraints before possibly substituting into 
the right-hand side; i.e., there is short-circuiting behaviour.

Example:
```
template 
concept C = true;

template 
struct Q { static constexpr T value = nullptr; };

template 
requires C || T::value
struct A { };

template 
requires C || Q::value
struct B { };

A a; // okay
B b; // okay
```



Repository:
  rC Clang

https://reviews.llvm.org/D41217



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

[PATCH] D53417: [Clang][PowerPC] Choose a better candidate in overload function call if there is a compatible vector conversion instead of ambiguous call error

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Sema/SemaOverload.cpp:3908
+   QualType ToType) {
+assert(FromType->isVectorType() && "FromType should be a vector type");
+assert(ToType->isVectorType() && "ToType should be a vector type");

These `assert`s would be redundant if `castAs` is used instead of `getAs`.



Comment at: clang/lib/Sema/SemaOverload.cpp:3911
+
+if (FromType->getAs()->getVectorKind() ==
+VectorType::AltiVecVector ||

Use `castAs` here and below.



Comment at: clang/lib/Sema/SemaOverload.cpp:3930
+  // }
+  // Here, we'd like to choose f(vector float) but not
+  // report an ambiguous call error

s/but/and/;


Repository:
  rC Clang

https://reviews.llvm.org/D53417



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

[PATCH] D53417: [Clang][PowerPC] Choose a better candidate in overload function call if there is a compatible vector conversion instead of ambiguous call error

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/test/Sema/altivec-generic-overload.c:3
+
+typedef signed char __v4sc __attribute__((__vector_size__(16)));
+typedef unsigned char __v4uc __attribute__((__vector_size__(16)));

`__v4sc` is suspicious. The most consistent naming I have seen is `v16i8`, 
`v16u8`, `v8i16`, ...
Do we need the double underscores?



Repository:
  rC Clang

https://reviews.llvm.org/D53417



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

[PATCH] D53417: [Clang][PowerPC] Choose a better candidate in overload function call if there is a compatible vector conversion instead of ambiguous call error

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast requested changes to this revision.
hubert.reinterpretcast added inline comments.
This revision now requires changes to proceed.



Comment at: clang/lib/Sema/SemaOverload.cpp:3920
+
+  // Prefer a compatible vector conversion to lax vector conversion
+  // For example:

s/to/over a/;



Comment at: clang/lib/Sema/SemaOverload.cpp:3941
+  return ImplicitConversionSequence::Better;
+  }
+

This seems to duplicate the bug described here in 
https://bugs.llvm.org/show_bug.cgi?id=39361.
```
typedef unsigned int GccType __attribute__((__vector_size__(16)));
typedef __vector unsigned int AltiVecType;

typedef float GccOtherType __attribute__((__vector_size__(16)));

char *f(GccOtherType, int);
template  int f(AltiVecType, T);
template  int g(AltiVecType, T);
char *g(GccOtherType, int);

bool zip(GccType v) { return f(v, 0) == g(v, 0); }
```


Repository:
  rC Clang

https://reviews.llvm.org/D53417



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

[PATCH] D53417: [Clang][PowerPC] Choose a better candidate in overload function call if there is a compatible vector conversion instead of ambiguous call error

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/test/Sema/altivec-generic-overload.c:3
+
+typedef signed char __v4sc __attribute__((__vector_size__(16)));
+typedef unsigned char __v4uc __attribute__((__vector_size__(16)));

wuzish wrote:
> hubert.reinterpretcast wrote:
> > `__v4sc` is suspicious. The most consistent naming I have seen is `v16i8`, 
> > `v16u8`, `v8i16`, ...
> > Do we need the double underscores?
> > 
> Because it's generic gcc vector type instead of altivec type so I choose the 
> naming style.
The naming style from `clang/lib/Headers/avxintrin.h` would still say that the 
`__v4sc` here should be `__v16qi`.


Repository:
  rC Clang

https://reviews.llvm.org/D53417



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

[PATCH] D53417: [Clang][Sema][PowerPC] Choose a better candidate in overload function call if there is a compatible vector conversion instead of ambiguous call error

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Sema/SemaOverload.cpp:3913
+for (auto Type : Types) {
+  if (S.Context.getCanonicalType(Type)->getTypeClass() != Type::Vector)
+return false;

Considering that this is a local lambda called in one place, are we expecting 
cases where the canonical type is not something on which we can call 
getVectorKind()? If not, then we do not need this `if`.


https://reviews.llvm.org/D53417



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

[PATCH] D53417: [Clang][Sema][PowerPC] Choose a better candidate in overload function call if there is a compatible vector conversion instead of ambiguous call error

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Sema/SemaOverload.cpp:3913
+for (auto Type : Types) {
+  if (S.Context.getCanonicalType(Type)->getTypeClass() != Type::Vector)
+return false;

wuzish wrote:
> hubert.reinterpretcast wrote:
> > Considering that this is a local lambda called in one place, are we 
> > expecting cases where the canonical type is not something on which we can 
> > call getVectorKind()? If not, then we do not need this `if`.
> Well, that's for ExtVectorType. I encounter some testcase failure because of 
> that. So I just narrow the condition to only handle Type::Vector.
> 
> As the following comment said, so I treat it separately.
> 
> /// ExtVectorType - Extended vector type. This type is created using
> /// __attribute__((ext_vector_type(n)), where "n" is the number of elements.
> /// Unlike vector_size, ext_vector_type is only allowed on typedef's. This
> /// class enables syntactic extensions, like Vector Components for accessing
> /// points (as .xyzw), colors (as .rgba), and textures (modeled after OpenGL
> /// Shading Language).
An ExtVectorType is a VectorType, so what sort of failures are you hitting?


https://reviews.llvm.org/D53417



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

[PATCH] D53417: [Clang][Sema][PowerPC] Choose a better candidate in overload function call if there is a compatible vector conversion instead of ambiguous call error

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Sema/SemaOverload.cpp:3913
+for (auto Type : Types) {
+  if (S.Context.getCanonicalType(Type)->getTypeClass() != Type::Vector)
+return false;

wuzish wrote:
> hubert.reinterpretcast wrote:
> > wuzish wrote:
> > > hubert.reinterpretcast wrote:
> > > > Considering that this is a local lambda called in one place, are we 
> > > > expecting cases where the canonical type is not something on which we 
> > > > can call getVectorKind()? If not, then we do not need this `if`.
> > > Well, that's for ExtVectorType. I encounter some testcase failure because 
> > > of that. So I just narrow the condition to only handle Type::Vector.
> > > 
> > > As the following comment said, so I treat it separately.
> > > 
> > > /// ExtVectorType - Extended vector type. This type is created using
> > > /// __attribute__((ext_vector_type(n)), where "n" is the number of 
> > > elements.
> > > /// Unlike vector_size, ext_vector_type is only allowed on typedef's. This
> > > /// class enables syntactic extensions, like Vector Components for 
> > > accessing
> > > /// points (as .xyzw), colors (as .rgba), and textures (modeled after 
> > > OpenGL
> > > /// Shading Language).
> > An ExtVectorType is a VectorType, so what sort of failures are you hitting?
> Yes. But the TypeClass of ExtVectorType is ExtVector.
> 
> some test points check the error report for ambiguous call because of too 
> many implicit cast choices from ext_vector_type to vector type. Such as blow.
> 
> 
> ```
> typedef char char16 __attribute__ ((__vector_size__ (16)));
> typedef long long longlong16 __attribute__ ((__vector_size__ (16)));
> typedef char char16_e __attribute__ ((__ext_vector_type__ (16)));
> typedef long long longlong16_e __attribute__ ((__ext_vector_type__ (2)));
> 
> 
> void f1_test(char16 c16, longlong16 ll16, char16_e c16e, longlong16_e ll16e) {
>   int &ir1 = f1(c16);
>   float &fr1 = f1(ll16);
>   f1(c16e); // expected-error{{call to 'f1' is ambiguous}}
>   f1(ll16e); // expected-error{{call to 'f1' is ambiguous}}
> }
> ```
> 
> If we are gonna widen the condition, we can make a follow-up patch. Or we 
> need include this condition and do this together in this patch?
The widening that has occurred is in allowing the scope of the change to 
encompass cases where AltiVec vectors are not sufficiently involved. The change 
in behaviour makes sense, and perhaps the community may want to pursue it; 
however, the mandate to make that level of change has not been given.

I do not believe that requiring that the TypeClass be VectorType is the correct 
narrowing of the scope. Instead, we may want to consider requiring that for 
each `SCS` in { `SCS1`, `SCS2` }, there is one AltiVec type and one generic 
vector type in { `SCS.getFromType()`, `SCS.getToType(2)` }.



https://reviews.llvm.org/D53417



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

[PATCH] D53417: [Clang][Sema][PowerPC] Choose a better candidate in overload function call if there is a compatible vector conversion instead of ambiguous call error

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Sema/SemaOverload.cpp:3913
+for (auto Type : Types) {
+  if (S.Context.getCanonicalType(Type)->getTypeClass() != Type::Vector)
+return false;

wuzish wrote:
> hubert.reinterpretcast wrote:
> > wuzish wrote:
> > > hubert.reinterpretcast wrote:
> > > > wuzish wrote:
> > > > > hubert.reinterpretcast wrote:
> > > > > > Considering that this is a local lambda called in one place, are we 
> > > > > > expecting cases where the canonical type is not something on which 
> > > > > > we can call getVectorKind()? If not, then we do not need this `if`.
> > > > > Well, that's for ExtVectorType. I encounter some testcase failure 
> > > > > because of that. So I just narrow the condition to only handle 
> > > > > Type::Vector.
> > > > > 
> > > > > As the following comment said, so I treat it separately.
> > > > > 
> > > > > /// ExtVectorType - Extended vector type. This type is created using
> > > > > /// __attribute__((ext_vector_type(n)), where "n" is the number of 
> > > > > elements.
> > > > > /// Unlike vector_size, ext_vector_type is only allowed on typedef's. 
> > > > > This
> > > > > /// class enables syntactic extensions, like Vector Components for 
> > > > > accessing
> > > > > /// points (as .xyzw), colors (as .rgba), and textures (modeled after 
> > > > > OpenGL
> > > > > /// Shading Language).
> > > > An ExtVectorType is a VectorType, so what sort of failures are you 
> > > > hitting?
> > > Yes. But the TypeClass of ExtVectorType is ExtVector.
> > > 
> > > some test points check the error report for ambiguous call because of too 
> > > many implicit cast choices from ext_vector_type to vector type. Such as 
> > > blow.
> > > 
> > > 
> > > ```
> > > typedef char char16 __attribute__ ((__vector_size__ (16)));
> > > typedef long long longlong16 __attribute__ ((__vector_size__ (16)));
> > > typedef char char16_e __attribute__ ((__ext_vector_type__ (16)));
> > > typedef long long longlong16_e __attribute__ ((__ext_vector_type__ (2)));
> > > 
> > > 
> > > void f1_test(char16 c16, longlong16 ll16, char16_e c16e, longlong16_e 
> > > ll16e) {
> > >   int &ir1 = f1(c16);
> > >   float &fr1 = f1(ll16);
> > >   f1(c16e); // expected-error{{call to 'f1' is ambiguous}}
> > >   f1(ll16e); // expected-error{{call to 'f1' is ambiguous}}
> > > }
> > > ```
> > > 
> > > If we are gonna widen the condition, we can make a follow-up patch. Or we 
> > > need include this condition and do this together in this patch?
> > The widening that has occurred is in allowing the scope of the change to 
> > encompass cases where AltiVec vectors are not sufficiently involved. The 
> > change in behaviour makes sense, and perhaps the community may want to 
> > pursue it; however, the mandate to make that level of change has not been 
> > given.
> > 
> > I do not believe that requiring that the TypeClass be VectorType is the 
> > correct narrowing of the scope. Instead, we may want to consider requiring 
> > that for each `SCS` in { `SCS1`, `SCS2` }, there is one AltiVec type and 
> > one generic vector type in { `SCS.getFromType()`, `SCS.getToType(2)` }.
> > 
> The key point is that ExtVector is a kind of typeclass, **and** its vector 
> kind is generic vector type.
> 
> So you think we should encompass ext_vector cases as a part of the scope of 
> this patch? And modify the above cases' expected behavior (remove the 
> **expected-error**)?
I gave a concrete suggested narrowing of the scope that does not exclude 
ExtVectorType or other derived types of VectorType. It also does not change the 
behaviour of the `f1_test` case above. We could pursue additional discussion 
over that case (separable from the current patch) on the mailing list.

I believe my suggestion does do something about this case:
```
typedef unsigned int GccType __attribute__((__ext_vector_type__(16)));
typedef __vector unsigned int AltiVecType;

typedef float GccOtherType __attribute__((__vector_size__(16)));

void f(GccType);
void f(GccOtherType);

void g(AltiVecType v) { f(v); }
```

I think that addressing the latter case is within the realm of things that we 
should consider for this patch. Either way, we should ensure that tests for 
AltiVec/__ext_vector_type__ conversions are available.


https://reviews.llvm.org/D53417



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

[PATCH] D53417: [Clang][Sema][PowerPC] Choose a better candidate in overload function call if there is a compatible vector conversion instead of ambiguous call error

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Sema/SemaOverload.cpp:3913
+for (auto Type : Types) {
+  if (S.Context.getCanonicalType(Type)->getTypeClass() != Type::Vector)
+return false;

hubert.reinterpretcast wrote:
> wuzish wrote:
> > hubert.reinterpretcast wrote:
> > > wuzish wrote:
> > > > hubert.reinterpretcast wrote:
> > > > > wuzish wrote:
> > > > > > hubert.reinterpretcast wrote:
> > > > > > > Considering that this is a local lambda called in one place, are 
> > > > > > > we expecting cases where the canonical type is not something on 
> > > > > > > which we can call getVectorKind()? If not, then we do not need 
> > > > > > > this `if`.
> > > > > > Well, that's for ExtVectorType. I encounter some testcase failure 
> > > > > > because of that. So I just narrow the condition to only handle 
> > > > > > Type::Vector.
> > > > > > 
> > > > > > As the following comment said, so I treat it separately.
> > > > > > 
> > > > > > /// ExtVectorType - Extended vector type. This type is created using
> > > > > > /// __attribute__((ext_vector_type(n)), where "n" is the number of 
> > > > > > elements.
> > > > > > /// Unlike vector_size, ext_vector_type is only allowed on 
> > > > > > typedef's. This
> > > > > > /// class enables syntactic extensions, like Vector Components for 
> > > > > > accessing
> > > > > > /// points (as .xyzw), colors (as .rgba), and textures (modeled 
> > > > > > after OpenGL
> > > > > > /// Shading Language).
> > > > > An ExtVectorType is a VectorType, so what sort of failures are you 
> > > > > hitting?
> > > > Yes. But the TypeClass of ExtVectorType is ExtVector.
> > > > 
> > > > some test points check the error report for ambiguous call because of 
> > > > too many implicit cast choices from ext_vector_type to vector type. 
> > > > Such as blow.
> > > > 
> > > > 
> > > > ```
> > > > typedef char char16 __attribute__ ((__vector_size__ (16)));
> > > > typedef long long longlong16 __attribute__ ((__vector_size__ (16)));
> > > > typedef char char16_e __attribute__ ((__ext_vector_type__ (16)));
> > > > typedef long long longlong16_e __attribute__ ((__ext_vector_type__ 
> > > > (2)));
> > > > 
> > > > 
> > > > void f1_test(char16 c16, longlong16 ll16, char16_e c16e, longlong16_e 
> > > > ll16e) {
> > > >   int &ir1 = f1(c16);
> > > >   float &fr1 = f1(ll16);
> > > >   f1(c16e); // expected-error{{call to 'f1' is ambiguous}}
> > > >   f1(ll16e); // expected-error{{call to 'f1' is ambiguous}}
> > > > }
> > > > ```
> > > > 
> > > > If we are gonna widen the condition, we can make a follow-up patch. Or 
> > > > we need include this condition and do this together in this patch?
> > > The widening that has occurred is in allowing the scope of the change to 
> > > encompass cases where AltiVec vectors are not sufficiently involved. The 
> > > change in behaviour makes sense, and perhaps the community may want to 
> > > pursue it; however, the mandate to make that level of change has not been 
> > > given.
> > > 
> > > I do not believe that requiring that the TypeClass be VectorType is the 
> > > correct narrowing of the scope. Instead, we may want to consider 
> > > requiring that for each `SCS` in { `SCS1`, `SCS2` }, there is one AltiVec 
> > > type and one generic vector type in { `SCS.getFromType()`, 
> > > `SCS.getToType(2)` }.
> > > 
> > The key point is that ExtVector is a kind of typeclass, **and** its vector 
> > kind is generic vector type.
> > 
> > So you think we should encompass ext_vector cases as a part of the scope of 
> > this patch? And modify the above cases' expected behavior (remove the 
> > **expected-error**)?
> I gave a concrete suggested narrowing of the scope that does not exclude 
> ExtVectorType or other derived types of VectorType. It also does not change 
> the behaviour of the `f1_test` case above. We could pursue additional 
> discussion over that case (separable from the current patch) on the mailing 
> list.
> 
> I believe my suggestion does do something about this case:
> ```
> typedef unsigned int GccType __attribute__((__ext_vector_type__(16)));
> typedef __vector unsigned int AltiVecType;
> 
> typedef float GccOtherType __attribute__((__vector_size__(16)));
> 
> void f(GccType);
> void f(GccOtherType);
> 
> void g(AltiVecType v) { f(v); }
> ```
> 
> I think that addressing the latter case is within the realm of things that we 
> should consider for this patch. Either way, we should ensure that tests for 
> AltiVec/__ext_vector_type__ conversions are available.
Sorry, typo in the above case:
```
typedef unsigned int GccType __attribute__((__ext_vector_type__(4)));
```



https://reviews.llvm.org/D53417



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

[PATCH] D53417: [Clang][Sema][PowerPC] Choose a better candidate in overload function call if there is a compatible vector conversion instead of ambiguous call error

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Sema/SemaOverload.cpp:3913
+for (auto Type : Types) {
+  if (S.Context.getCanonicalType(Type)->getTypeClass() != Type::Vector)
+return false;

wuzish wrote:
> hubert.reinterpretcast wrote:
> > hubert.reinterpretcast wrote:
> > > wuzish wrote:
> > > > hubert.reinterpretcast wrote:
> > > > > wuzish wrote:
> > > > > > hubert.reinterpretcast wrote:
> > > > > > > wuzish wrote:
> > > > > > > > hubert.reinterpretcast wrote:
> > > > > > > > > Considering that this is a local lambda called in one place, 
> > > > > > > > > are we expecting cases where the canonical type is not 
> > > > > > > > > something on which we can call getVectorKind()? If not, then 
> > > > > > > > > we do not need this `if`.
> > > > > > > > Well, that's for ExtVectorType. I encounter some testcase 
> > > > > > > > failure because of that. So I just narrow the condition to only 
> > > > > > > > handle Type::Vector.
> > > > > > > > 
> > > > > > > > As the following comment said, so I treat it separately.
> > > > > > > > 
> > > > > > > > /// ExtVectorType - Extended vector type. This type is created 
> > > > > > > > using
> > > > > > > > /// __attribute__((ext_vector_type(n)), where "n" is the number 
> > > > > > > > of elements.
> > > > > > > > /// Unlike vector_size, ext_vector_type is only allowed on 
> > > > > > > > typedef's. This
> > > > > > > > /// class enables syntactic extensions, like Vector Components 
> > > > > > > > for accessing
> > > > > > > > /// points (as .xyzw), colors (as .rgba), and textures (modeled 
> > > > > > > > after OpenGL
> > > > > > > > /// Shading Language).
> > > > > > > An ExtVectorType is a VectorType, so what sort of failures are 
> > > > > > > you hitting?
> > > > > > Yes. But the TypeClass of ExtVectorType is ExtVector.
> > > > > > 
> > > > > > some test points check the error report for ambiguous call because 
> > > > > > of too many implicit cast choices from ext_vector_type to vector 
> > > > > > type. Such as blow.
> > > > > > 
> > > > > > 
> > > > > > ```
> > > > > > typedef char char16 __attribute__ ((__vector_size__ (16)));
> > > > > > typedef long long longlong16 __attribute__ ((__vector_size__ (16)));
> > > > > > typedef char char16_e __attribute__ ((__ext_vector_type__ (16)));
> > > > > > typedef long long longlong16_e __attribute__ ((__ext_vector_type__ 
> > > > > > (2)));
> > > > > > 
> > > > > > 
> > > > > > void f1_test(char16 c16, longlong16 ll16, char16_e c16e, 
> > > > > > longlong16_e ll16e) {
> > > > > >   int &ir1 = f1(c16);
> > > > > >   float &fr1 = f1(ll16);
> > > > > >   f1(c16e); // expected-error{{call to 'f1' is ambiguous}}
> > > > > >   f1(ll16e); // expected-error{{call to 'f1' is ambiguous}}
> > > > > > }
> > > > > > ```
> > > > > > 
> > > > > > If we are gonna widen the condition, we can make a follow-up patch. 
> > > > > > Or we need include this condition and do this together in this 
> > > > > > patch?
> > > > > The widening that has occurred is in allowing the scope of the change 
> > > > > to encompass cases where AltiVec vectors are not sufficiently 
> > > > > involved. The change in behaviour makes sense, and perhaps the 
> > > > > community may want to pursue it; however, the mandate to make that 
> > > > > level of change has not been given.
> > > > > 
> > > > > I do not believe that requiring that the TypeClass be VectorType is 
> > > > > the correct narrowing of the scope. Instead, we may want to consider 
> > > > > requiring that for each `SCS` in { `SCS1`, `SCS2` }, there is one 
> > > > > AltiVec type and one generic vector type in { `SCS.getFromType()`, 
> > > > > `SCS.getToType(2)` }.
> > > > > 
> > > > The key point is that ExtVector is a kind of typeclass, **and** its 
> > > > vector kind is generic vector type.
> > > > 
> > > > So you think we should encompass ext_vector cases as a part of the 
> > > > scope of this patch? And modify the above cases' expected behavior 
> > > > (remove the **expected-error**)?
> > > I gave a concrete suggested narrowing of the scope that does not exclude 
> > > ExtVectorType or other derived types of VectorType. It also does not 
> > > change the behaviour of the `f1_test` case above. We could pursue 
> > > additional discussion over that case (separable from the current patch) 
> > > on the mailing list.
> > > 
> > > I believe my suggestion does do something about this case:
> > > ```
> > > typedef unsigned int GccType __attribute__((__ext_vector_type__(16)));
> > > typedef __vector unsigned int AltiVecType;
> > > 
> > > typedef float GccOtherType __attribute__((__vector_size__(16)));
> > > 
> > > void f(GccType);
> > > void f(GccOtherType);
> > > 
> > > void g(AltiVecType v) { f(v); }
> > > ```
> > > 
> > > I think that addressing the latter case is within the realm of things 
> > > that we should consider for this patch. Either way, we should ensure that 
> > > tests for AltiVec/__ext_vector_ty

[PATCH] D53417: [Clang][Sema][PowerPC] Choose a better candidate in overload function call if there is a compatible vector conversion instead of ambiguous call error

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Sema/SemaOverload.cpp:3913
+for (auto Type : Types) {
+  if (S.Context.getCanonicalType(Type)->getTypeClass() != Type::Vector)
+return false;

wuzish wrote:
> hubert.reinterpretcast wrote:
> > wuzish wrote:
> > > hubert.reinterpretcast wrote:
> > > > hubert.reinterpretcast wrote:
> > > > > wuzish wrote:
> > > > > > hubert.reinterpretcast wrote:
> > > > > > > wuzish wrote:
> > > > > > > > hubert.reinterpretcast wrote:
> > > > > > > > > wuzish wrote:
> > > > > > > > > > hubert.reinterpretcast wrote:
> > > > > > > > > > > Considering that this is a local lambda called in one 
> > > > > > > > > > > place, are we expecting cases where the canonical type is 
> > > > > > > > > > > not something on which we can call getVectorKind()? If 
> > > > > > > > > > > not, then we do not need this `if`.
> > > > > > > > > > Well, that's for ExtVectorType. I encounter some testcase 
> > > > > > > > > > failure because of that. So I just narrow the condition to 
> > > > > > > > > > only handle Type::Vector.
> > > > > > > > > > 
> > > > > > > > > > As the following comment said, so I treat it separately.
> > > > > > > > > > 
> > > > > > > > > > /// ExtVectorType - Extended vector type. This type is 
> > > > > > > > > > created using
> > > > > > > > > > /// __attribute__((ext_vector_type(n)), where "n" is the 
> > > > > > > > > > number of elements.
> > > > > > > > > > /// Unlike vector_size, ext_vector_type is only allowed on 
> > > > > > > > > > typedef's. This
> > > > > > > > > > /// class enables syntactic extensions, like Vector 
> > > > > > > > > > Components for accessing
> > > > > > > > > > /// points (as .xyzw), colors (as .rgba), and textures 
> > > > > > > > > > (modeled after OpenGL
> > > > > > > > > > /// Shading Language).
> > > > > > > > > An ExtVectorType is a VectorType, so what sort of failures 
> > > > > > > > > are you hitting?
> > > > > > > > Yes. But the TypeClass of ExtVectorType is ExtVector.
> > > > > > > > 
> > > > > > > > some test points check the error report for ambiguous call 
> > > > > > > > because of too many implicit cast choices from ext_vector_type 
> > > > > > > > to vector type. Such as blow.
> > > > > > > > 
> > > > > > > > 
> > > > > > > > ```
> > > > > > > > typedef char char16 __attribute__ ((__vector_size__ (16)));
> > > > > > > > typedef long long longlong16 __attribute__ ((__vector_size__ 
> > > > > > > > (16)));
> > > > > > > > typedef char char16_e __attribute__ ((__ext_vector_type__ 
> > > > > > > > (16)));
> > > > > > > > typedef long long longlong16_e __attribute__ 
> > > > > > > > ((__ext_vector_type__ (2)));
> > > > > > > > 
> > > > > > > > 
> > > > > > > > void f1_test(char16 c16, longlong16 ll16, char16_e c16e, 
> > > > > > > > longlong16_e ll16e) {
> > > > > > > >   int &ir1 = f1(c16);
> > > > > > > >   float &fr1 = f1(ll16);
> > > > > > > >   f1(c16e); // expected-error{{call to 'f1' is ambiguous}}
> > > > > > > >   f1(ll16e); // expected-error{{call to 'f1' is ambiguous}}
> > > > > > > > }
> > > > > > > > ```
> > > > > > > > 
> > > > > > > > If we are gonna widen the condition, we can make a follow-up 
> > > > > > > > patch. Or we need include this condition and do this together 
> > > > > > > > in this patch?
> > > > > > > The widening that has occurred is in allowing the scope of the 
> > > > > > > change to encompass cases where AltiVec vectors are not 
> > > > > > > sufficiently involved. The change in behaviour makes sense, and 
> > > > > > > perhaps the community may want to pursue it; however, the mandate 
> > > > > > > to make that level of change has not been given.
> > > > > > > 
> > > > > > > I do not believe that requiring that the TypeClass be VectorType 
> > > > > > > is the correct narrowing of the scope. Instead, we may want to 
> > > > > > > consider requiring that for each `SCS` in { `SCS1`, `SCS2` }, 
> > > > > > > there is one AltiVec type and one generic vector type in { 
> > > > > > > `SCS.getFromType()`, `SCS.getToType(2)` }.
> > > > > > > 
> > > > > > The key point is that ExtVector is a kind of typeclass, **and** its 
> > > > > > vector kind is generic vector type.
> > > > > > 
> > > > > > So you think we should encompass ext_vector cases as a part of the 
> > > > > > scope of this patch? And modify the above cases' expected behavior 
> > > > > > (remove the **expected-error**)?
> > > > > I gave a concrete suggested narrowing of the scope that does not 
> > > > > exclude ExtVectorType or other derived types of VectorType. It also 
> > > > > does not change the behaviour of the `f1_test` case above. We could 
> > > > > pursue additional discussion over that case (separable from the 
> > > > > current patch) on the mailing list.
> > > > > 
> > > > > I believe my suggestion does do something about this case:
> > > > > ```
> > > > > typedef unsigned int GccType __attribute__((__ext_vector_type__(16)));
> > > > > typedef __

[PATCH] D53417: [Clang][Sema][PowerPC] Choose a better candidate in overload function call if there is a compatible vector conversion instead of ambiguous call error

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Sema/SemaOverload.cpp:3913
+for (auto Type : Types) {
+  if (S.Context.getCanonicalType(Type)->getTypeClass() != Type::Vector)
+return false;

wuzish wrote:
> hubert.reinterpretcast wrote:
> > wuzish wrote:
> > > hubert.reinterpretcast wrote:
> > > > wuzish wrote:
> > > > > hubert.reinterpretcast wrote:
> > > > > > hubert.reinterpretcast wrote:
> > > > > > > wuzish wrote:
> > > > > > > > hubert.reinterpretcast wrote:
> > > > > > > > > wuzish wrote:
> > > > > > > > > > hubert.reinterpretcast wrote:
> > > > > > > > > > > wuzish wrote:
> > > > > > > > > > > > hubert.reinterpretcast wrote:
> > > > > > > > > > > > > Considering that this is a local lambda called in one 
> > > > > > > > > > > > > place, are we expecting cases where the canonical 
> > > > > > > > > > > > > type is not something on which we can call 
> > > > > > > > > > > > > getVectorKind()? If not, then we do not need this 
> > > > > > > > > > > > > `if`.
> > > > > > > > > > > > Well, that's for ExtVectorType. I encounter some 
> > > > > > > > > > > > testcase failure because of that. So I just narrow the 
> > > > > > > > > > > > condition to only handle Type::Vector.
> > > > > > > > > > > > 
> > > > > > > > > > > > As the following comment said, so I treat it separately.
> > > > > > > > > > > > 
> > > > > > > > > > > > /// ExtVectorType - Extended vector type. This type is 
> > > > > > > > > > > > created using
> > > > > > > > > > > > /// __attribute__((ext_vector_type(n)), where "n" is 
> > > > > > > > > > > > the number of elements.
> > > > > > > > > > > > /// Unlike vector_size, ext_vector_type is only allowed 
> > > > > > > > > > > > on typedef's. This
> > > > > > > > > > > > /// class enables syntactic extensions, like Vector 
> > > > > > > > > > > > Components for accessing
> > > > > > > > > > > > /// points (as .xyzw), colors (as .rgba), and textures 
> > > > > > > > > > > > (modeled after OpenGL
> > > > > > > > > > > > /// Shading Language).
> > > > > > > > > > > An ExtVectorType is a VectorType, so what sort of 
> > > > > > > > > > > failures are you hitting?
> > > > > > > > > > Yes. But the TypeClass of ExtVectorType is ExtVector.
> > > > > > > > > > 
> > > > > > > > > > some test points check the error report for ambiguous call 
> > > > > > > > > > because of too many implicit cast choices from 
> > > > > > > > > > ext_vector_type to vector type. Such as blow.
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > ```
> > > > > > > > > > typedef char char16 __attribute__ ((__vector_size__ (16)));
> > > > > > > > > > typedef long long longlong16 __attribute__ 
> > > > > > > > > > ((__vector_size__ (16)));
> > > > > > > > > > typedef char char16_e __attribute__ ((__ext_vector_type__ 
> > > > > > > > > > (16)));
> > > > > > > > > > typedef long long longlong16_e __attribute__ 
> > > > > > > > > > ((__ext_vector_type__ (2)));
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > void f1_test(char16 c16, longlong16 ll16, char16_e c16e, 
> > > > > > > > > > longlong16_e ll16e) {
> > > > > > > > > >   int &ir1 = f1(c16);
> > > > > > > > > >   float &fr1 = f1(ll16);
> > > > > > > > > >   f1(c16e); // expected-error{{call to 'f1' is ambiguous}}
> > > > > > > > > >   f1(ll16e); // expected-error{{call to 'f1' is ambiguous}}
> > > > > > > > > > }
> > > > > > > > > > ```
> > > > > > > > > > 
> > > > > > > > > > If we are gonna widen the condition, we can make a 
> > > > > > > > > > follow-up patch. Or we need include this condition and do 
> > > > > > > > > > this together in this patch?
> > > > > > > > > The widening that has occurred is in allowing the scope of 
> > > > > > > > > the change to encompass cases where AltiVec vectors are not 
> > > > > > > > > sufficiently involved. The change in behaviour makes sense, 
> > > > > > > > > and perhaps the community may want to pursue it; however, the 
> > > > > > > > > mandate to make that level of change has not been given.
> > > > > > > > > 
> > > > > > > > > I do not believe that requiring that the TypeClass be 
> > > > > > > > > VectorType is the correct narrowing of the scope. Instead, we 
> > > > > > > > > may want to consider requiring that for each `SCS` in { 
> > > > > > > > > `SCS1`, `SCS2` }, there is one AltiVec type and one generic 
> > > > > > > > > vector type in { `SCS.getFromType()`, `SCS.getToType(2)` }.
> > > > > > > > > 
> > > > > > > > The key point is that ExtVector is a kind of typeclass, **and** 
> > > > > > > > its vector kind is generic vector type.
> > > > > > > > 
> > > > > > > > So you think we should encompass ext_vector cases as a part of 
> > > > > > > > the scope of this patch? And modify the above cases' expected 
> > > > > > > > behavior (remove the **expected-error**)?
> > > > > > > I gave a concrete suggested narrowing of the scope that does not 
> > > > > > > exclude ExtVectorType or other derived types of VectorType.

[PATCH] D53417: [Clang][Sema][PowerPC] Choose a better candidate in overload function call if there is a compatible vector conversion instead of ambiguous call error

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

> some test points check the error report for ambiguous call because of too 
> many implicit cast choices from ext_vector_type to vector type.

It appears the answer is to update these tests and remove the restriction on 
the type class.


https://reviews.llvm.org/D53417



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

[PATCH] D53417: [Clang][Sema][PowerPC] Choose a better candidate in overload function call if there is a compatible vector conversion instead of ambiguous call error

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/test/CodeGen/altivec-generic-overload.c:74
+  convert1(gv1);
+  // CHECK: call void @_Z8convert1Dv16_a(<16 x i8> %{{[0-9]+}})
+  convert1(gv2);

Checking that the call is to the expected target in terms of overload 
resolution can be achieved by having a distinct return types for each member of 
the overload set.


https://reviews.llvm.org/D53417



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

[PATCH] D53417: [Clang][Sema][PowerPC] Choose a better candidate in overload function call if there is a compatible vector conversion instead of ambiguous call error

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/test/CodeGen/altivec-generic-overload.c:74
+  convert1(gv1);
+  // CHECK: call void @_Z8convert1Dv16_a(<16 x i8> %{{[0-9]+}})
+  convert1(gv2);

wuzish wrote:
> hubert.reinterpretcast wrote:
> > Checking that the call is to the expected target in terms of overload 
> > resolution can be achieved by having a distinct return types for each 
> > member of the overload set.
> What's meaning of `having a distinct return types for each member of the 
> overload set`?
> 
> Could you please give a example to show your expect?
This can be done with `-fsyntax-only`:
```
typedef signed char __v16sc __attribute__((__vector_size__(16)));
typedef unsigned char __v16uc __attribute__((__vector_size__(16)));

__v16sc *__attribute__((__overloadable__)) convert1(vector signed char);
__v16uc *__attribute__((__overloadable__)) convert1(vector unsigned char);

void test()
{
  __v16sc gv1;
  __v16uc gv2;
  _Generic(convert1(gv1), __v16sc *: (void)0);
  _Generic(convert1(gv2), __v16uc *: (void)0);
}
```



Comment at: clang/test/SemaCXX/vector.cpp:26
   float &fr1 = f1(ll16);
-  f1(c16e); // expected-error{{call to 'f1' is ambiguous}}
-  f1(ll16e); // expected-error{{call to 'f1' is ambiguous}}
+  f1(c16e);
+  f1(ll16e);

Check the return types here like with the two lines above.


https://reviews.llvm.org/D53417



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

[PATCH] D53417: [Clang][Sema][PowerPC] Choose a better candidate in overload function call if there is a compatible vector conversion instead of ambiguous call error

[Hubert Tong via Phabricator via cfe-commits]

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

LGTM.




Comment at: clang/test/Sema/altivec-generic-overload.c:73
+
+  __v16sc *gv1_p = convert1(gv1);
+  __v16uc *gv2_p = convert1(gv2);

Using assignment instead of `_Generic` produces only a warning when the types 
mismatch (at least in certain cases), but that should be okay because we are 
using `-verify`.


https://reviews.llvm.org/D53417



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

[PATCH] D55510: [ExprConstant] Improve memchr/memcmp for type mismatch and multibyte element types

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast created this revision.
hubert.reinterpretcast added reviewers: rsmith, aaron.ballman, hfinkel.

`memchr` and `memcmp` operate upon the character units of the object 
representation; that is, the `size_t` parameter expresses the number of 
character units. The constant folding implementation is updated in this patch 
to account for multibyte element types in the arrays passed to 
`memchr`/`memcmp` and, in the case of `memcmp`, to account for the possibility 
that the arrays may have differing element types (even when they are 
byte-sized).

Actual inspection of the object representation is not implemented. Comparisons 
are done only between elements with the same object size; that is, `memchr` 
will fail when inspecting at least one character unit of a multibyte element. 
The integer types are assumed to have two's complement representation with 0 
for `false`, 1 for `true`, and no padding bits.

`memcmp` on multibyte elements will only be able to fold in cases where enough 
elements are equal for the answer to be 0.

CodeGen tests are added for cases that miscompile on some system or other prior 
to this patch.


Repository:
  rC Clang

https://reviews.llvm.org/D55510

Files:
  include/clang/Basic/DiagnosticASTKinds.td
  lib/AST/ExprConstant.cpp
  test/CodeGenCXX/builtins.cpp
  test/SemaCXX/constexpr-string.cpp

Index: test/SemaCXX/constexpr-string.cpp
===
--- test/SemaCXX/constexpr-string.cpp
+++ test/SemaCXX/constexpr-string.cpp
@@ -95,6 +95,51 @@
   static_assert(__builtin_memcmp("abab\0banana", "abab\0canada", 6) == -1);
   static_assert(__builtin_memcmp("abab\0banana", "abab\0canada", 5) == 0);
 
+  extern struct Incomplete incomplete;
+  static_assert(__builtin_memcmp(&incomplete, "", 0u) == 0);
+  static_assert(__builtin_memcmp("", &incomplete, 0u) == 0);
+  static_assert(__builtin_memcmp(&incomplete, "", 1u) == 42); // expected-error {{not an integral constant}} expected-note {{read of incomplete type 'struct Incomplete'}}
+  static_assert(__builtin_memcmp("", &incomplete, 1u) == 42); // expected-error {{not an integral constant}} expected-note {{read of incomplete type 'struct Incomplete'}}
+
+  constexpr unsigned char ku00fe00[] = {0x00, 0xfe, 0x00};
+  constexpr unsigned char ku00feff[] = {0x00, 0xfe, 0xff};
+  constexpr signed char ks00fe00[] = {0, -2, 0};
+  constexpr signed char ks00feff[] = {0, -2, -1};
+  static_assert(__builtin_memcmp(ku00feff, ks00fe00, 2) == 0);
+  static_assert(__builtin_memcmp(ku00feff, ks00fe00, 99) == 1);
+  static_assert(__builtin_memcmp(ku00fe00, ks00feff, 99) == -1);
+  static_assert(__builtin_memcmp(ks00feff, ku00fe00, 2) == 0);
+  static_assert(__builtin_memcmp(ks00feff, ku00fe00, 99) == 1);
+  static_assert(__builtin_memcmp(ks00fe00, ku00feff, 99) == -1);
+  static_assert(__builtin_memcmp(ks00fe00, ks00feff, 2) == 0);
+  static_assert(__builtin_memcmp(ks00feff, ks00fe00, 99) == 1);
+  static_assert(__builtin_memcmp(ks00fe00, ks00feff, 99) == -1);
+
+  constexpr bool kb000100[] = {false, true, false};
+  static_assert(sizeof(bool) != 1u || __builtin_memcmp(ks00fe00, kb000100, 1) == 0);
+  static_assert(sizeof(bool) != 1u || __builtin_memcmp(ks00fe00, kb000100, 2) == 1);
+
+  constexpr long ksl[] = {0, -1};
+  constexpr unsigned int kui[] = {0, 0u - 1};
+  constexpr unsigned long long kull[] = {0, 0ull - 1};
+  constexpr const auto *kuSizeofLong(void) {
+if constexpr(sizeof(long) == sizeof(int)) {
+  return kui;
+} else {
+  static_assert(sizeof(long) == sizeof(long long));
+  return kull;
+}
+  }
+  static_assert(__builtin_memcmp(ksl, kuSizeofLong(), sizeof(long) - 1) == 0);
+  static_assert(__builtin_memcmp(ksl, kuSizeofLong(), sizeof(long) + 0) == 0);
+  static_assert(__builtin_memcmp(ksl, kuSizeofLong(), sizeof(long) + 1) == 0);
+  static_assert(__builtin_memcmp(ksl, kuSizeofLong(), 2*sizeof(long) - 1) == 0);
+  static_assert(__builtin_memcmp(ksl, kuSizeofLong(), 2*sizeof(long) + 0) == 0);
+  static_assert(__builtin_memcmp(ksl, kuSizeofLong(), 2*sizeof(long) + 1) == 42); // expected-error {{not an integral constant}} expected-note {{dereferenced one-past-the-end}}
+  static_assert(__builtin_memcmp(ksl + 1, kuSizeofLong() + 1, sizeof(long) - 1) == 0);
+  static_assert(__builtin_memcmp(ksl + 1, kuSizeofLong() + 1, sizeof(long) + 0) == 0);
+  static_assert(__builtin_memcmp(ksl + 1, kuSizeofLong() + 1, sizeof(long) + 1) == 42); // expected-error {{not an integral constant}} expected-note {{dereferenced one-past-the-end}}
+
   constexpr int a = strcmp("hello", "world"); // expected-error {{constant expression}} expected-note {{non-constexpr function 'strcmp' cannot be used in a constant expression}}
   constexpr int b = strncmp("hello", "world", 3); // expected-error {{constant expression}} expected-note {{non-constexpr function 'strncmp' cannot be used in a constant expression}}
   constexpr int c = memcmp("hello", "world", 3); // expected-error {{consta

[PATCH] D55510: [ExprConstant] Improve memchr/memcmp for type mismatch and multibyte element types

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: lib/AST/ExprConstant.cpp:6159-6160
+// Give up on byte-oriented matching against multibyte elements.
+if (IsRawByte && Info.Ctx.getTypeSize(CharTy) > Info.Ctx.getCharWidth())
+  return false;
 // Figure out what value we're actually looking for (after converting to

rsmith wrote:
> Please add an assert here that the types do match in the non-raw-byte case. 
> (They must, or the subobject designator would be invalid; the only special 
> case is that a `void*` can point to anything, and only the `memchr` variants 
> here take a `void*`.)
Will do.



Comment at: lib/AST/ExprConstant.cpp:8471-8476
+// We have compatible in-memory widths, but a possible type and 
internal
+// representation mismatch. Assuming two's complement representation,
+// including 0 for `false` and 1 for `true`, we can check an 
appropriate
+// number of elements for equality even if they are not byte-sized.
+const APSInt Char1InMem = Char1.getInt().extOrTrunc(CharTy1Width);
+const APSInt Char2InMem = Char2.getInt().extOrTrunc(CharTy1Width);

rsmith wrote:
> The *only* possible problem here is for `bool`, right? This comment would be 
> clearer if it said that.
We do assume two's complement representation (which C89 does not require to be 
true); so, `bool` is more problematic than other things, but the comment is not 
just about `bool`.



Comment at: lib/AST/ExprConstant.cpp:8488-8489
+  return false;
+if (MaxLength <= LengthPerElement)
+  break;
+MaxLength -= LengthPerElement;

rsmith wrote:
> This looks wrong. If `0 < MaxLength && MaxLength < LengthPerElement`, we're 
> supposed to compare *part of* the next element; this current approach doesn't 
> do that. (If you did one more full-element compare, that'd be conservatively 
> correct because we're only checking for equality, not ordering, but perhaps 
> we should only permit cases where `MaxLength` is a multiple of 
> `LengthPerElement`?
We //are// doing one more full-element compare to be conservatively correct. 
There is a test for that.



Comment at: test/CodeGenCXX/builtins.cpp:41
+matchedFirstByteIn04030201();
+// CHECK: call void 
@_ZN27MemchrMultibyteElementTests26matchedFirstByteIn04030201Ev()
+  }

rsmith wrote:
> Please test this in a way that doesn't rely on IR generation 
> constant-evaluating `if` conditions. Moreover, there's nothing about IR 
> generation that you're actually testing here, so please phrase this as a 
> `Sema` test instead (eg, check that a VLA bound gets folded to a constant). 
> Likewise for the other tests below.
Not all of these cases can fold successfully with the current implementation. 
The check would need to be that we either fold (and get the right value), or 
don't fold at all.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55510/new/

https://reviews.llvm.org/D55510



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

[PATCH] D55510: [ExprConstant] Improve memchr/memcmp for type mismatch and multibyte element types

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast updated this revision to Diff 177597.
hubert.reinterpretcast marked 11 inline comments as done.
hubert.reinterpretcast added a comment.

Make an initial pass at addressing the review comments

Address comments on style and code comments


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55510/new/

https://reviews.llvm.org/D55510

Files:
  include/clang/Basic/DiagnosticASTKinds.td
  lib/AST/ExprConstant.cpp
  test/CodeGenCXX/builtins.cpp
  test/SemaCXX/constexpr-string.cpp

Index: test/SemaCXX/constexpr-string.cpp
===
--- test/SemaCXX/constexpr-string.cpp
+++ test/SemaCXX/constexpr-string.cpp
@@ -95,6 +95,51 @@
   static_assert(__builtin_memcmp("abab\0banana", "abab\0canada", 6) == -1);
   static_assert(__builtin_memcmp("abab\0banana", "abab\0canada", 5) == 0);
 
+  extern struct Incomplete incomplete;
+  static_assert(__builtin_memcmp(&incomplete, "", 0u) == 0);
+  static_assert(__builtin_memcmp("", &incomplete, 0u) == 0);
+  static_assert(__builtin_memcmp(&incomplete, "", 1u) == 42); // expected-error {{not an integral constant}} expected-note {{read of incomplete type 'struct Incomplete'}}
+  static_assert(__builtin_memcmp("", &incomplete, 1u) == 42); // expected-error {{not an integral constant}} expected-note {{read of incomplete type 'struct Incomplete'}}
+
+  constexpr unsigned char ku00fe00[] = {0x00, 0xfe, 0x00};
+  constexpr unsigned char ku00feff[] = {0x00, 0xfe, 0xff};
+  constexpr signed char ks00fe00[] = {0, -2, 0};
+  constexpr signed char ks00feff[] = {0, -2, -1};
+  static_assert(__builtin_memcmp(ku00feff, ks00fe00, 2) == 0);
+  static_assert(__builtin_memcmp(ku00feff, ks00fe00, 99) == 1);
+  static_assert(__builtin_memcmp(ku00fe00, ks00feff, 99) == -1);
+  static_assert(__builtin_memcmp(ks00feff, ku00fe00, 2) == 0);
+  static_assert(__builtin_memcmp(ks00feff, ku00fe00, 99) == 1);
+  static_assert(__builtin_memcmp(ks00fe00, ku00feff, 99) == -1);
+  static_assert(__builtin_memcmp(ks00fe00, ks00feff, 2) == 0);
+  static_assert(__builtin_memcmp(ks00feff, ks00fe00, 99) == 1);
+  static_assert(__builtin_memcmp(ks00fe00, ks00feff, 99) == -1);
+
+  constexpr bool kb000100[] = {false, true, false};
+  static_assert(sizeof(bool) != 1u || __builtin_memcmp(ks00fe00, kb000100, 1) == 0);
+  static_assert(sizeof(bool) != 1u || __builtin_memcmp(ks00fe00, kb000100, 2) == 1);
+
+  constexpr long ksl[] = {0, -1};
+  constexpr unsigned int kui[] = {0, 0u - 1};
+  constexpr unsigned long long kull[] = {0, 0ull - 1};
+  constexpr const auto *kuSizeofLong(void) {
+if constexpr(sizeof(long) == sizeof(int)) {
+  return kui;
+} else {
+  static_assert(sizeof(long) == sizeof(long long));
+  return kull;
+}
+  }
+  static_assert(__builtin_memcmp(ksl, kuSizeofLong(), sizeof(long) - 1) == 0);
+  static_assert(__builtin_memcmp(ksl, kuSizeofLong(), sizeof(long) + 0) == 0);
+  static_assert(__builtin_memcmp(ksl, kuSizeofLong(), sizeof(long) + 1) == 0);
+  static_assert(__builtin_memcmp(ksl, kuSizeofLong(), 2*sizeof(long) - 1) == 0);
+  static_assert(__builtin_memcmp(ksl, kuSizeofLong(), 2*sizeof(long) + 0) == 0);
+  static_assert(__builtin_memcmp(ksl, kuSizeofLong(), 2*sizeof(long) + 1) == 42); // expected-error {{not an integral constant}} expected-note {{dereferenced one-past-the-end}}
+  static_assert(__builtin_memcmp(ksl + 1, kuSizeofLong() + 1, sizeof(long) - 1) == 0);
+  static_assert(__builtin_memcmp(ksl + 1, kuSizeofLong() + 1, sizeof(long) + 0) == 0);
+  static_assert(__builtin_memcmp(ksl + 1, kuSizeofLong() + 1, sizeof(long) + 1) == 42); // expected-error {{not an integral constant}} expected-note {{dereferenced one-past-the-end}}
+
   constexpr int a = strcmp("hello", "world"); // expected-error {{constant expression}} expected-note {{non-constexpr function 'strcmp' cannot be used in a constant expression}}
   constexpr int b = strncmp("hello", "world", 3); // expected-error {{constant expression}} expected-note {{non-constexpr function 'strncmp' cannot be used in a constant expression}}
   constexpr int c = memcmp("hello", "world", 3); // expected-error {{constant expression}} expected-note {{non-constexpr function 'memcmp' cannot be used in a constant expression}}
@@ -187,6 +232,26 @@
   static_assert(__builtin_memchr(nullptr, 'x', 3) == nullptr); // expected-error {{not an integral constant}} expected-note {{dereferenced null}}
   static_assert(__builtin_memchr(nullptr, 'x', 0) == nullptr); // FIXME: Should we reject this?
 
+  extern struct Incomplete incomplete;
+  static_assert(__builtin_memchr(&incomplete, 0, 0u) == nullptr);
+  static_assert(__builtin_memchr(&incomplete, 0, 1u) == nullptr); // expected-error {{not an integral constant}} expected-note {{read of incomplete type 'struct Incomplete'}}
+
+  const unsigned char &u1 = 0xf0;
+  auto &&i1 = (const signed char []){-128}; // expected-warning {{compound literals are a C99-specific feature}}
+  static_assert(__

[PATCH] D55510: [ExprConstant] Improve memchr/memcmp for type mismatch and multibyte element types

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast updated this revision to Diff 177618.
hubert.reinterpretcast marked 6 inline comments as done.
hubert.reinterpretcast added a comment.

Use lvalue designator, add assertions for type matching


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55510/new/

https://reviews.llvm.org/D55510

Files:
  include/clang/Basic/DiagnosticASTKinds.td
  lib/AST/ExprConstant.cpp
  test/CodeGenCXX/builtins.cpp
  test/SemaCXX/constexpr-string.cpp

Index: test/SemaCXX/constexpr-string.cpp
===
--- test/SemaCXX/constexpr-string.cpp
+++ test/SemaCXX/constexpr-string.cpp
@@ -95,6 +95,52 @@
   static_assert(__builtin_memcmp("abab\0banana", "abab\0canada", 6) == -1);
   static_assert(__builtin_memcmp("abab\0banana", "abab\0canada", 5) == 0);
 
+  extern struct Incomplete incomplete;
+  static_assert(__builtin_memcmp(&incomplete, "", 0u) == 0);
+  static_assert(__builtin_memcmp("", &incomplete, 0u) == 0);
+  static_assert(__builtin_memcmp(&incomplete, "", 1u) == 42); // expected-error {{not an integral constant}} expected-note {{read of incomplete type 'struct Incomplete'}}
+  static_assert(__builtin_memcmp("", &incomplete, 1u) == 42); // expected-error {{not an integral constant}} expected-note {{read of incomplete type 'struct Incomplete'}}
+
+  constexpr unsigned char ku00fe00[] = {0x00, 0xfe, 0x00};
+  constexpr unsigned char ku00feff[] = {0x00, 0xfe, 0xff};
+  constexpr signed char ks00fe00[] = {0, -2, 0};
+  constexpr signed char ks00feff[] = {0, -2, -1};
+  static_assert(__builtin_memcmp(ku00feff, ks00fe00, 2) == 0);
+  static_assert(__builtin_memcmp(ku00feff, ks00fe00, 99) == 1);
+  static_assert(__builtin_memcmp(ku00fe00, ks00feff, 99) == -1);
+  static_assert(__builtin_memcmp(ks00feff, ku00fe00, 2) == 0);
+  static_assert(__builtin_memcmp(ks00feff, ku00fe00, 99) == 1);
+  static_assert(__builtin_memcmp(ks00fe00, ku00feff, 99) == -1);
+  static_assert(__builtin_memcmp(ks00fe00, ks00feff, 2) == 0);
+  static_assert(__builtin_memcmp(ks00feff, ks00fe00, 99) == 1);
+  static_assert(__builtin_memcmp(ks00fe00, ks00feff, 99) == -1);
+
+  struct Bool3Tuple { bool bb[3]; };
+  constexpr Bool3Tuple kb000100 = {{false, true, false}};
+  static_assert(sizeof(bool) != 1u || __builtin_memcmp(ks00fe00, kb000100.bb, 1) == 0);
+  static_assert(sizeof(bool) != 1u || __builtin_memcmp(ks00fe00, kb000100.bb, 2) == 1);
+
+  constexpr long ksl[] = {0, -1};
+  constexpr unsigned int kui[] = {0, 0u - 1};
+  constexpr unsigned long long kull[] = {0, 0ull - 1};
+  constexpr const auto *kuSizeofLong(void) {
+if constexpr(sizeof(long) == sizeof(int)) {
+  return kui;
+} else {
+  static_assert(sizeof(long) == sizeof(long long));
+  return kull;
+}
+  }
+  static_assert(__builtin_memcmp(ksl, kuSizeofLong(), sizeof(long) - 1) == 0);
+  static_assert(__builtin_memcmp(ksl, kuSizeofLong(), sizeof(long) + 0) == 0);
+  static_assert(__builtin_memcmp(ksl, kuSizeofLong(), sizeof(long) + 1) == 0);
+  static_assert(__builtin_memcmp(ksl, kuSizeofLong(), 2*sizeof(long) - 1) == 0);
+  static_assert(__builtin_memcmp(ksl, kuSizeofLong(), 2*sizeof(long) + 0) == 0);
+  static_assert(__builtin_memcmp(ksl, kuSizeofLong(), 2*sizeof(long) + 1) == 42); // expected-error {{not an integral constant}} expected-note {{dereferenced one-past-the-end}}
+  static_assert(__builtin_memcmp(ksl + 1, kuSizeofLong() + 1, sizeof(long) - 1) == 0);
+  static_assert(__builtin_memcmp(ksl + 1, kuSizeofLong() + 1, sizeof(long) + 0) == 0);
+  static_assert(__builtin_memcmp(ksl + 1, kuSizeofLong() + 1, sizeof(long) + 1) == 42); // expected-error {{not an integral constant}} expected-note {{dereferenced one-past-the-end}}
+
   constexpr int a = strcmp("hello", "world"); // expected-error {{constant expression}} expected-note {{non-constexpr function 'strcmp' cannot be used in a constant expression}}
   constexpr int b = strncmp("hello", "world", 3); // expected-error {{constant expression}} expected-note {{non-constexpr function 'strncmp' cannot be used in a constant expression}}
   constexpr int c = memcmp("hello", "world", 3); // expected-error {{constant expression}} expected-note {{non-constexpr function 'memcmp' cannot be used in a constant expression}}
@@ -187,6 +233,27 @@
   static_assert(__builtin_memchr(nullptr, 'x', 3) == nullptr); // expected-error {{not an integral constant}} expected-note {{dereferenced null}}
   static_assert(__builtin_memchr(nullptr, 'x', 0) == nullptr); // FIXME: Should we reject this?
 
+  extern struct Incomplete incomplete;
+  static_assert(__builtin_memchr(&incomplete, 0, 0u) == nullptr);
+  static_assert(__builtin_memchr(&incomplete, 0, 1u) == nullptr); // expected-error {{not an integral constant}} expected-note {{read of incomplete type 'struct Incomplete'}}
+
+  const unsigned char &u1 = 0xf0;
+  auto &&i1 = (const signed char []){-128}; // expected-warning {{compound literals are a C99-specific feature}}
+  static_asse

[PATCH] D55510: [ExprConstant] Improve memchr/memcmp for type mismatch and multibyte element types

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast marked an inline comment as done.
hubert.reinterpretcast added inline comments.



Comment at: lib/AST/ExprConstant.cpp:6147-6148
+  return ZeroInitialization(E);
+if (!Result.checkNullPointerForFoldAccess(Info, E, AK_Read))
+  return false;
+QualType CharTy =

rsmith wrote:
> Why do we need to do this explicitly, rather than allowing to simply happen 
> as part of the first `handleLValueToRValueConversion` below?
We want the error message to be produced even if we bail early on not having a 
valid designator.



Comment at: lib/AST/ExprConstant.cpp:6150
+QualType CharTy =
+Info.Ctx.getBaseElementType(getType(Result.getLValueBase()));
+const bool IsRawByte = BuiltinOp == Builtin::BImemchr ||

rsmith wrote:
> Considering the LValue base here is incorrect. The object or array we're 
> copying could be a subobject, and the complete object type (the type of the 
> lvalue base) is irrelevant for such copies. Use 
> `Result.Designator.getType(Info.Ctx)` to find the type that the lvalue 
> designates. (You can bail out here if the designator is invalid; 
> `handleLValueToRValueConversion` will always fail on such cases anyway.
Got it; I also updated the tests to be sensitive to this issue.



Comment at: test/CodeGenCXX/builtins.cpp:41
+matchedFirstByteIn04030201();
+// CHECK: call void 
@_ZN27MemchrMultibyteElementTests26matchedFirstByteIn04030201Ev()
+  }

rsmith wrote:
> hubert.reinterpretcast wrote:
> > rsmith wrote:
> > > Please test this in a way that doesn't rely on IR generation 
> > > constant-evaluating `if` conditions. Moreover, there's nothing about IR 
> > > generation that you're actually testing here, so please phrase this as a 
> > > `Sema` test instead (eg, check that a VLA bound gets folded to a 
> > > constant). Likewise for the other tests below.
> > Not all of these cases can fold successfully with the current 
> > implementation. The check would need to be that we either fold (and get the 
> > right value), or don't fold at all.
> Seems fine to have a test that we don't fold certain cases at all, with a 
> comment saying that it's OK if we start folding them to a constant and what 
> that constant should be.
> 
> Generally the principle is that we want our unit tests to test as few layers 
> of the stack as possible; the chosen test directory should ideally correspond 
> to the layer under test. If you'd really like this to be more of an 
> integration test, that's fine too (eg, if you have code like this in a 
> platform header that really must be handled a certain way), but then it would 
> belong in `test/Integration`. If you do put it in `test/Integration`, it'd 
> then be reasonable to include -O in the test flags too, if that makes sense 
> for what you want to test.
I think a `SemaCXX` test based on the following pattern would work:
```
constexpr decltype(sizeof 0) Good = 42, Bad = 43;
template  struct A;
struct NotBad {};
template <> struct A : NotBad {};

template  A *evalFor(T (&)[N]);
NotBad *evalFor(...);
void chk(NotBad *);

void f() {
  int x[__builtin_memcmp("", "", 1) == 0 ? Good : Bad];
  chk(evalFor(x));
}
```


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55510/new/

https://reviews.llvm.org/D55510



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

[PATCH] D55510: [ExprConstant] Improve memchr/memcmp for type mismatch and multibyte element types

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast updated this revision to Diff 177656.
hubert.reinterpretcast marked 3 inline comments as done.
hubert.reinterpretcast added a comment.

Address remaining ExprConstant.cpp review comments

Use BytesRemaining/BytesPerElement to improve readability;
tweak FIXME comment to refer to the _remaining_ bytes.
Use CharUnits-to-CharUnits comparisons where straightforward.
Tweak comment about internal representation mismatch.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55510/new/

https://reviews.llvm.org/D55510

Files:
  include/clang/Basic/DiagnosticASTKinds.td
  lib/AST/ExprConstant.cpp
  test/CodeGenCXX/builtins.cpp
  test/SemaCXX/constexpr-string.cpp

Index: test/SemaCXX/constexpr-string.cpp
===
--- test/SemaCXX/constexpr-string.cpp
+++ test/SemaCXX/constexpr-string.cpp
@@ -95,6 +95,52 @@
   static_assert(__builtin_memcmp("abab\0banana", "abab\0canada", 6) == -1);
   static_assert(__builtin_memcmp("abab\0banana", "abab\0canada", 5) == 0);
 
+  extern struct Incomplete incomplete;
+  static_assert(__builtin_memcmp(&incomplete, "", 0u) == 0);
+  static_assert(__builtin_memcmp("", &incomplete, 0u) == 0);
+  static_assert(__builtin_memcmp(&incomplete, "", 1u) == 42); // expected-error {{not an integral constant}} expected-note {{read of incomplete type 'struct Incomplete'}}
+  static_assert(__builtin_memcmp("", &incomplete, 1u) == 42); // expected-error {{not an integral constant}} expected-note {{read of incomplete type 'struct Incomplete'}}
+
+  constexpr unsigned char ku00fe00[] = {0x00, 0xfe, 0x00};
+  constexpr unsigned char ku00feff[] = {0x00, 0xfe, 0xff};
+  constexpr signed char ks00fe00[] = {0, -2, 0};
+  constexpr signed char ks00feff[] = {0, -2, -1};
+  static_assert(__builtin_memcmp(ku00feff, ks00fe00, 2) == 0);
+  static_assert(__builtin_memcmp(ku00feff, ks00fe00, 99) == 1);
+  static_assert(__builtin_memcmp(ku00fe00, ks00feff, 99) == -1);
+  static_assert(__builtin_memcmp(ks00feff, ku00fe00, 2) == 0);
+  static_assert(__builtin_memcmp(ks00feff, ku00fe00, 99) == 1);
+  static_assert(__builtin_memcmp(ks00fe00, ku00feff, 99) == -1);
+  static_assert(__builtin_memcmp(ks00fe00, ks00feff, 2) == 0);
+  static_assert(__builtin_memcmp(ks00feff, ks00fe00, 99) == 1);
+  static_assert(__builtin_memcmp(ks00fe00, ks00feff, 99) == -1);
+
+  struct Bool3Tuple { bool bb[3]; };
+  constexpr Bool3Tuple kb000100 = {{false, true, false}};
+  static_assert(sizeof(bool) != 1u || __builtin_memcmp(ks00fe00, kb000100.bb, 1) == 0);
+  static_assert(sizeof(bool) != 1u || __builtin_memcmp(ks00fe00, kb000100.bb, 2) == 1);
+
+  constexpr long ksl[] = {0, -1};
+  constexpr unsigned int kui[] = {0, 0u - 1};
+  constexpr unsigned long long kull[] = {0, 0ull - 1};
+  constexpr const auto *kuSizeofLong(void) {
+if constexpr(sizeof(long) == sizeof(int)) {
+  return kui;
+} else {
+  static_assert(sizeof(long) == sizeof(long long));
+  return kull;
+}
+  }
+  static_assert(__builtin_memcmp(ksl, kuSizeofLong(), sizeof(long) - 1) == 0);
+  static_assert(__builtin_memcmp(ksl, kuSizeofLong(), sizeof(long) + 0) == 0);
+  static_assert(__builtin_memcmp(ksl, kuSizeofLong(), sizeof(long) + 1) == 0);
+  static_assert(__builtin_memcmp(ksl, kuSizeofLong(), 2*sizeof(long) - 1) == 0);
+  static_assert(__builtin_memcmp(ksl, kuSizeofLong(), 2*sizeof(long) + 0) == 0);
+  static_assert(__builtin_memcmp(ksl, kuSizeofLong(), 2*sizeof(long) + 1) == 42); // expected-error {{not an integral constant}} expected-note {{dereferenced one-past-the-end}}
+  static_assert(__builtin_memcmp(ksl + 1, kuSizeofLong() + 1, sizeof(long) - 1) == 0);
+  static_assert(__builtin_memcmp(ksl + 1, kuSizeofLong() + 1, sizeof(long) + 0) == 0);
+  static_assert(__builtin_memcmp(ksl + 1, kuSizeofLong() + 1, sizeof(long) + 1) == 42); // expected-error {{not an integral constant}} expected-note {{dereferenced one-past-the-end}}
+
   constexpr int a = strcmp("hello", "world"); // expected-error {{constant expression}} expected-note {{non-constexpr function 'strcmp' cannot be used in a constant expression}}
   constexpr int b = strncmp("hello", "world", 3); // expected-error {{constant expression}} expected-note {{non-constexpr function 'strncmp' cannot be used in a constant expression}}
   constexpr int c = memcmp("hello", "world", 3); // expected-error {{constant expression}} expected-note {{non-constexpr function 'memcmp' cannot be used in a constant expression}}
@@ -187,6 +233,27 @@
   static_assert(__builtin_memchr(nullptr, 'x', 3) == nullptr); // expected-error {{not an integral constant}} expected-note {{dereferenced null}}
   static_assert(__builtin_memchr(nullptr, 'x', 0) == nullptr); // FIXME: Should we reject this?
 
+  extern struct Incomplete incomplete;
+  static_assert(__builtin_memchr(&incomplete, 0, 0u) == nullptr);
+  static_assert(__builtin_memchr(&incomplete, 0, 1u) == nullptr); // expected-error {{not an integral constant}} e

[PATCH] D55510: [ExprConstant] Improve memchr/memcmp for type mismatch and multibyte element types

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

I'll update the tests to be in terms of constant/variable array length 
tomorrow. I think I've gotten through the rest of the comments.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55510/new/

https://reviews.llvm.org/D55510



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

[PATCH] D58930: Add XCOFF triple object format type for AIX

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: llvm/lib/Support/Triple.cpp:537
   return StringSwitch(EnvironmentName)
+// FIXME: We have to put XCOFF before COFF;
+// perhaps an order-independent pattern matching is desired?

If the conclusion is that checking XCOFF before COFF is fine, then we should 
remove the FIXME and just leave a normal comment.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58930/new/

https://reviews.llvm.org/D58930



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

[PATCH] D58930: Add XCOFF triple object format type for AIX

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:2079
+  if (log)
+log->Printf("sorry: unimplemented for XCOFF");
+  return false;

JDevlieghere wrote:
> jasonliu wrote:
> > JDevlieghere wrote:
> > > jasonliu wrote:
> > > > apaprocki wrote:
> > > > > No need to be `sorry:` :) This should probably just say `error: XCOFF 
> > > > > is unimplemented` to be more direct in case anything is expecting 
> > > > > "error:" in the output.
> > > > Sure. Will address in next revision.
> > > Just bundle this with the WASM case, the error message is correct for 
> > > both.
> > I think they are different. 
> > The error message for WASM seems to suggest that it will never ever get 
> > supported on WASM. 
> > But it is not the case for XCOFF, we want to indicate that it is not 
> > implemented yet.  
> I don't think the error message suggests that at all, and it's definitely not 
> true. At this point neither XCOFF nor WASM is supported, and that's exactly 
> what the log message says.
> 
I agree that the error message for WASM does not indicate that the lack of 
support is inherent or intended to be permanent; however, it is not indicative 
either of an intent to implement the support. I am not sure what the intent is 
for WASM, but I do know that the intent for XCOFF is to eventually implement 
the support. I do not see how using an ambiguous message in this commit (when 
we know what the intent is) is superior to the alternative of having an 
unambiguous message.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58930/new/

https://reviews.llvm.org/D58930



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

[PATCH] D58930: Add XCOFF triple object format type for AIX

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:2079
+  if (log)
+log->Printf("sorry: unimplemented for XCOFF");
+  return false;

davide wrote:
> hubert.reinterpretcast wrote:
> > JDevlieghere wrote:
> > > jasonliu wrote:
> > > > JDevlieghere wrote:
> > > > > jasonliu wrote:
> > > > > > apaprocki wrote:
> > > > > > > No need to be `sorry:` :) This should probably just say `error: 
> > > > > > > XCOFF is unimplemented` to be more direct in case anything is 
> > > > > > > expecting "error:" in the output.
> > > > > > Sure. Will address in next revision.
> > > > > Just bundle this with the WASM case, the error message is correct for 
> > > > > both.
> > > > I think they are different. 
> > > > The error message for WASM seems to suggest that it will never ever get 
> > > > supported on WASM. 
> > > > But it is not the case for XCOFF, we want to indicate that it is not 
> > > > implemented yet.  
> > > I don't think the error message suggests that at all, and it's definitely 
> > > not true. At this point neither XCOFF nor WASM is supported, and that's 
> > > exactly what the log message says.
> > > 
> > I agree that the error message for WASM does not indicate that the lack of 
> > support is inherent or intended to be permanent; however, it is not 
> > indicative either of an intent to implement the support. I am not sure what 
> > the intent is for WASM, but I do know that the intent for XCOFF is to 
> > eventually implement the support. I do not see how using an ambiguous 
> > message in this commit (when we know what the intent is) is superior to the 
> > alternative of having an unambiguous message.
> I think we should keep this consistent with the other target so my vote is 
> for grouping XCOFF with WASM. After all, if it's going to be implemented 
> soon, the message will go away :)
Well, I don't know about "soon"...
Using the WASM message for XCOFF is not actually wrong; so, I can be okay with 
it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58930/new/

https://reviews.llvm.org/D58930



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

[PATCH] D59048: Add AIX Target Info

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a subscriber: rsmith.
hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Basic/Targets/OSTargets.h:626
+
+// FIXME: Define AIX OS-Version Macros
+Builder.defineMacro("_AIX");

Comments should be full sentences (with periods). Please update throughout this 
patch.



Comment at: clang/lib/Basic/Targets/OSTargets.h:629
+   
+// FIXME: Determine the proper way for users to select the no-long-long 
+// version of the standard library

@rsmith suggested that having a `-fno-long-long` option to disable support for 
long long would be preferable to having an `-maix-no-long-long` that just 
controls the AIX binary-compatibility aspects. Perhaps the FIXME should be 
updated to mention checking `-fno-long-long`.



Comment at: clang/lib/Basic/Targets/OSTargets.h:637
+
+// Define _WCHAR_T only for C++
+if (Opts.CPlusPlus && Opts.WChar) {

Suggestion for the comment text:
Define `_WCHAR_T` when it is a fundamental type (i.e., for C++ without 
`-fno-wchar`).



Comment at: clang/lib/Basic/Targets/OSTargets.h:641
+}
+  }
+

D18360 sets `_THREAD_SAFE` to `1` when `-pthread` is specified. I believe that 
to be correct. I believe whether or not `-pthread` is taken to be the default 
on the platform is a separate consideration.



Comment at: clang/lib/Basic/Targets/OSTargets.h:651
+}
+this->UseZeroLengthBitfieldAlignment = true;
+  }

Can we have a test for this?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59048/new/

https://reviews.llvm.org/D59048



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

[PATCH] D18360: Add AIX Target/ToolChain to Clang Driver

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

D59048  has been posted with some initial 
`OSTargetInfo` changes. Noticeable differences include not defining 
`_ALL_SOURCE` and focus on 64-bit long double. This is consistent with the 
invocation provided with the recent v16.1 release of the IBM XL C/C++ compiler 
for AIX that incorporates Clang-based technology.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D18360/new/

https://reviews.llvm.org/D18360



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

[PATCH] D59048: Add AIX Target Info

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Basic/Targets/OSTargets.h:626
+
+// FIXME: Define AIX OS-Version Macros
+Builder.defineMacro("_AIX");

apaprocki wrote:
> hubert.reinterpretcast wrote:
> > Comments should be full sentences (with periods). Please update throughout 
> > this patch.
> I do not think it is realistic to support an AIX target prior to 7.1, so 
> should this unconditionally define up to and including `_AIX71` similar to 
> D18360 did (it was written earlier and defined up to `_AIX61` 
> unconditionally) and leave a `FIXME` to determine when to emit `_AIX72` or 
> any other later versions?
I agree with your assessment re: older versions of AIX in terms of having a 
full solution for modern C++. Nevertheless, we can leave room open for hobby 
builds targeting older versions where the immediate cost is not high. Cross 
compiling to an older AIX target will probably be entirely possible for C.



Comment at: clang/test/Preprocessor/init.c:6420
+// PPC64-AIX:#define _LONG_LONG 1
+// PPC64-AIX:#define _POWER 1
+// PPC64-AIX:#define __64BIT__ 1

apaprocki wrote:
> XL on AIX emits `#define _LP64 1` in 64-bit mode and `#define _ILP32 1` in 
> 32-bit mode in the pre-defined macros. Is that important to capture?
I think so. The v16.1 XL compiler's `xlclang` also produces these.

```
#define __LP64__ 1
#define _LP64 1
```

```
#define __ILP32__ 1
#define _ILP32 1
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59048/new/

https://reviews.llvm.org/D59048



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

[PATCH] D59048: Add AIX Target Info

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/test/Preprocessor/init.c:6420
+// PPC64-AIX:#define _LONG_LONG 1
+// PPC64-AIX:#define _POWER 1
+// PPC64-AIX:#define __64BIT__ 1

hubert.reinterpretcast wrote:
> apaprocki wrote:
> > XL on AIX emits `#define _LP64 1` in 64-bit mode and `#define _ILP32 1` in 
> > 32-bit mode in the pre-defined macros. Is that important to capture?
> I think so. The v16.1 XL compiler's `xlclang` also produces these.
> 
> ```
> #define __LP64__ 1
> #define _LP64 1
> ```
> 
> ```
> #define __ILP32__ 1
> #define _ILP32 1
> ```
It seems GCC on AIX only defines the macros for 64-bit, and not the 32-bit 
versions. The system headers do not appear to depend on the 32-bit versions. It 
makes sense to start with the common intersection between the GCC and XL 
predefined macros first. We can add `__LP64__` and `_LP64` with this patch. I 
think we can leave more macros for later.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59048/new/

https://reviews.llvm.org/D59048



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

[PATCH] D58930: Add XCOFF triple object format type for AIX

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

@jasonliu, you have had a number of patches committed into the project already 
(D22698 , D22702 
, D34649 ). 
Please go ahead with requesting commit access, and commit this patch with the 
additional fix when you are ready.




Comment at: llvm/lib/MC/MCObjectFileInfo.cpp:806
+Env = IsXCOFF;
+// TODO: Initialize MCObjectFileInfo for XCOFF format when MCSectionXCOFF 
is ready.
+break;

This line is over 80 characters.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58930/new/

https://reviews.llvm.org/D58930



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

[PATCH] D59048: Add AIX Target Info

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Basic/Targets/OSTargets.h:640
+
+// Define _WCHAR_T when it is a fundamental type (i.e., for C++ without 
-fno-wchar).
+if (Opts.CPlusPlus && Opts.WChar) {

Line is longer than 80 characters. Please split it.



Comment at: clang/test/CodeGen/arm-aapcs-zerolength-bitfield.c:1
 // REQUIRES: arm-registered-target
 // RUN: %clang_cc1 -target-abi aapcs -triple armv7-apple-darwin10 %s -verify

Given the requirement for `arm-registered-target`, is the file actually run 
whenever we intend it to be? Also, a note re: the existing test: This does not 
seem to be a CodeGen test; indeed, it is effective even with `-fsyntax-only`. 
It seems this should be moved to `clang/test/Sema`.

However, not all of the cases are common anyway. We will later need to post 
changes that implement AIX's 4-byte storage units for bit-fields. I think we 
should leave this file alone for the purposes of this patch.



Comment at: clang/test/Preprocessor/init.c:7027
+// PPC-AIX:#define _IBMR2 1
+// PPC-AIX:#define _LONG_LONG 1
+// PPC-AIX:#define _POWER 1

Add a check that `_LP64` is not defined under the 32-bit mode. Similarly for 
`__LP64__` and `__64BIT__`. Other targets also check for the presence or 
absence of `_ILP32` and `__ILP32__`, so we probably should do the same.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59048/new/

https://reviews.llvm.org/D59048



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

[PATCH] D59048: Add AIX Target Info

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast accepted this revision.
hubert.reinterpretcast added a subscriber: WuZhao.
hubert.reinterpretcast added a comment.
This revision is now accepted and ready to land.

I think all comments have been addressed or otherwise responded to.
@jasonliu: Since this patch is dependent on D58930 
, I would suggest that you commit it after 
D58930 . A friendly reminder to include the 
patch attribution for @andusy. @apaprocki, and @WuZhao. Thanks.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59048/new/

https://reviews.llvm.org/D59048



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

[PATCH] D59233: libclang/CIndexer.cpp: Use loadquery() on AIX for path to library

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast created this revision.
hubert.reinterpretcast added reviewers: sfertile, xingxue, jasonliu.
Herald added a subscriber: arphaman.
Herald added a project: clang.

`dladdr` is not available on AIX. Similar functionality is presented through 
`loadquery`. This patch replaces a use of `dladdr` with a version based on 
`loadquery`.


Repository:
  rC Clang

https://reviews.llvm.org/D59233

Files:
  tools/libclang/CIndexer.cpp


Index: tools/libclang/CIndexer.cpp
===
--- tools/libclang/CIndexer.cpp
+++ tools/libclang/CIndexer.cpp
@@ -32,12 +32,68 @@
 
 #ifdef _WIN32
 #include 
+#elif defined(_AIX)
+#include 
+#include 
 #else
 #include 
 #endif
 
 using namespace clang;
 
+#ifdef _AIX
+namespace clang {
+namespace {
+
+template 
+void getClangResourcesPathImplAIX(LibClangPathType &LibClangPath) {
+  int PrevErrno = errno;
+
+  size_t BufSize = 2048u;
+  std::unique_ptr Buf;
+  while (true) {
+Buf = llvm::make_unique(BufSize);
+errno = 0;
+int Ret = loadquery(L_GETXINFO, Buf.get(), (unsigned int)BufSize);
+if (Ret != -1)
+  break; // loadquery() was successful.
+if (errno != ENOMEM)
+  llvm_unreachable("Encountered an unexpected loadquery() failure");
+
+// errno == ENOMEM; try to allocate more memory.
+if ((BufSize & ~((-1u) >> 1u)) != 0u)
+  llvm_unreachable("BufSize needed for loadquery() too large");
+
+Buf.release();
+BufSize <<= 1u;
+  }
+
+  // Extract the function entry point from the function descriptor.
+  uint64_t EntryAddr =
+  reinterpret_cast(clang_createTranslationUnit);
+
+  // Loop to locate the function entry point in the loadquery() results.
+  ld_xinfo *CurInfo = reinterpret_cast(Buf.get());
+  while (true) {
+uint64_t CurTextStart = (uint64_t)CurInfo->ldinfo_textorg;
+uint64_t CurTextEnd = CurTextStart + CurInfo->ldinfo_textsize;
+if (CurTextStart <= EntryAddr && EntryAddr < CurTextEnd)
+  break; // Successfully located.
+
+if (CurInfo->ldinfo_next == 0u)
+  llvm_unreachable("Cannot locate entry point in the loadquery() results");
+CurInfo = reinterpret_cast(reinterpret_cast(CurInfo) +
+   CurInfo->ldinfo_next);
+  }
+
+  LibClangPath += reinterpret_cast(CurInfo) + CurInfo->ldinfo_filename;
+  errno = PrevErrno;
+}
+
+} // end anonymous namespace
+} // end namespace clang
+#endif
+
 const std::string &CIndexer::getClangResourcesPath() {
   // Did we already compute the path?
   if (!ResourcesPath.empty())
@@ -64,6 +120,8 @@
 #endif
 
   LibClangPath += path;
+#elif defined(_AIX)
+  getClangResourcesPathImplAIX(LibClangPath);
 #else
   // This silly cast below avoids a C++ warning.
   Dl_info info;


Index: tools/libclang/CIndexer.cpp
===
--- tools/libclang/CIndexer.cpp
+++ tools/libclang/CIndexer.cpp
@@ -32,12 +32,68 @@
 
 #ifdef _WIN32
 #include 
+#elif defined(_AIX)
+#include 
+#include 
 #else
 #include 
 #endif
 
 using namespace clang;
 
+#ifdef _AIX
+namespace clang {
+namespace {
+
+template 
+void getClangResourcesPathImplAIX(LibClangPathType &LibClangPath) {
+  int PrevErrno = errno;
+
+  size_t BufSize = 2048u;
+  std::unique_ptr Buf;
+  while (true) {
+Buf = llvm::make_unique(BufSize);
+errno = 0;
+int Ret = loadquery(L_GETXINFO, Buf.get(), (unsigned int)BufSize);
+if (Ret != -1)
+  break; // loadquery() was successful.
+if (errno != ENOMEM)
+  llvm_unreachable("Encountered an unexpected loadquery() failure");
+
+// errno == ENOMEM; try to allocate more memory.
+if ((BufSize & ~((-1u) >> 1u)) != 0u)
+  llvm_unreachable("BufSize needed for loadquery() too large");
+
+Buf.release();
+BufSize <<= 1u;
+  }
+
+  // Extract the function entry point from the function descriptor.
+  uint64_t EntryAddr =
+  reinterpret_cast(clang_createTranslationUnit);
+
+  // Loop to locate the function entry point in the loadquery() results.
+  ld_xinfo *CurInfo = reinterpret_cast(Buf.get());
+  while (true) {
+uint64_t CurTextStart = (uint64_t)CurInfo->ldinfo_textorg;
+uint64_t CurTextEnd = CurTextStart + CurInfo->ldinfo_textsize;
+if (CurTextStart <= EntryAddr && EntryAddr < CurTextEnd)
+  break; // Successfully located.
+
+if (CurInfo->ldinfo_next == 0u)
+  llvm_unreachable("Cannot locate entry point in the loadquery() results");
+CurInfo = reinterpret_cast(reinterpret_cast(CurInfo) +
+   CurInfo->ldinfo_next);
+  }
+
+  LibClangPath += reinterpret_cast(CurInfo) + CurInfo->ldinfo_filename;
+  errno = PrevErrno;
+}
+
+} // end anonymous namespace
+} // end namespace clang
+#endif
+
 const std::string &CIndexer::getClangResourcesPath() {
   // Did we already compute the path?
   if (!ResourcesPath.empty())
@@ -64,6 +120,8 @@
 #endif
 
   LibClangPath += path;
+#elif defined(_AIX)
+  getClangResourcesPathImplAIX(LibClangP

[PATCH] D59233: libclang/CIndexer.cpp: Use loadquery() on AIX for path to library

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

I believe that the conditions being checked for with `llvm_unreachable` in this 
patch are of the debug-only variety; however, some input would be appreciated 
regarding the choice of using `llvm_unreachable` instead of 
`report_fatal_error` or `assert`.




Comment at: tools/libclang/CIndexer.cpp:61
+if (errno != ENOMEM)
+  llvm_unreachable("Encountered an unexpected loadquery() failure");
+

Based on available documentation, this situation should not occur (and thus I 
believe `llvm_unreachable` is appropriate).



Comment at: tools/libclang/CIndexer.cpp:65
+if ((BufSize & ~((-1u) >> 1u)) != 0u)
+  llvm_unreachable("BufSize needed for loadquery() too large");
+

This situation is not impossible, but highly improbable. This is a 
non-programmatic error, and the Programmer's Manual appears to recommend the 
use of `report_fatal_error`. Some additional guidance would be appreciated.



Comment at: tools/libclang/CIndexer.cpp:84
+if (CurInfo->ldinfo_next == 0u)
+  llvm_unreachable("Cannot locate entry point in the loadquery() results");
+CurInfo = reinterpret_cast(reinterpret_cast(CurInfo) +

This is also supposed to not happen.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59233/new/

https://reviews.llvm.org/D59233



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

[PATCH] D59048: Add AIX Target Info

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: test/Headers/max_align.c:1
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// expected-no-diagnostics

We may need to explicitly specify C11. It also seems that we should XFAIL 
Windows targets.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59048/new/

https://reviews.llvm.org/D59048



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

[PATCH] D59048: Add AIX Target Info

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: test/Headers/max_align.c:1
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// expected-no-diagnostics

lebedev.ri wrote:
> hubert.reinterpretcast wrote:
> > We may need to explicitly specify C11. It also seems that we should XFAIL 
> > Windows targets.
> Does this need an explicit `-triple=???` ?
I don't think this test is target-specific, I read the specifications (as 
opposed to the implementations) of `max_align_t` and `__BIGGEST_ALIGNMENT__` as 
saying that `_Alignof(max_align_t)` should be the same as 
`__BIGGEST_ALIGNMENT__`.

Granted, it seems `max_align_t` on Windows does not match 
`__BIGGEST_ALIGNMENT__`. So, a custom `malloc` implementation querying on one 
might not align sufficiently based on the other.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59048/new/

https://reviews.llvm.org/D59048



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

[PATCH] D59233: libclang/CIndexer.cpp: Use loadquery() on AIX for path to library

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added subscribers: asb, majnemer, lhames.
hubert.reinterpretcast added a comment.

@asb @lhames @majnemer, based on D36826 , I 
believe that your guidance would be helpful here regarding the use of 
`llvm_unreachable`, `report_fatal_error`, and `assert`.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59233/new/

https://reviews.llvm.org/D59233



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

[PATCH] D59233: libclang/CIndexer.cpp: Use loadquery() on AIX for path to library

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast marked 6 inline comments as done.
hubert.reinterpretcast added a comment.
Herald added a subscriber: jsji.

Thanks @xingxue for the review. I will update for the error handling before 
committing.




Comment at: tools/libclang/CIndexer.cpp:61
+if (errno != ENOMEM)
+  llvm_unreachable("Encountered an unexpected loadquery() failure");
+

hubert.reinterpretcast wrote:
> Based on available documentation, this situation should not occur (and thus I 
> believe `llvm_unreachable` is appropriate).
I'll leave this one as `llvm_unreachable`, either the failure is resolved on a 
later call or the loop will terminate when the amount of memory to request 
(checked below) is too large.



Comment at: tools/libclang/CIndexer.cpp:65
+if ((BufSize & ~((-1u) >> 1u)) != 0u)
+  llvm_unreachable("BufSize needed for loadquery() too large");
+

hubert.reinterpretcast wrote:
> This situation is not impossible, but highly improbable. This is a 
> non-programmatic error, and the Programmer's Manual appears to recommend the 
> use of `report_fatal_error`. Some additional guidance would be appreciated.
I will change this to `report_fatal_error`.



Comment at: tools/libclang/CIndexer.cpp:84
+if (CurInfo->ldinfo_next == 0u)
+  llvm_unreachable("Cannot locate entry point in the loadquery() results");
+CurInfo = reinterpret_cast(reinterpret_cast(CurInfo) +

hubert.reinterpretcast wrote:
> This is also supposed to not happen.
I will change this to `report_fatal_error`. Infinite loops are unfriendly.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59233/new/

https://reviews.llvm.org/D59233



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

[PATCH] D59233: libclang/CIndexer.cpp: Use loadquery() on AIX for path to library

[Hubert Tong via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
hubert.reinterpretcast marked 3 inline comments as done.
Closed by commit rC356843: libclang/CIndexer.cpp: Use loadquery() on AIX for 
path to library (authored by hubert.reinterpretcast, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D59233?vs=190170&id=192005#toc

Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59233/new/

https://reviews.llvm.org/D59233

Files:
  tools/libclang/CIndexer.cpp


Index: tools/libclang/CIndexer.cpp
===
--- tools/libclang/CIndexer.cpp
+++ tools/libclang/CIndexer.cpp
@@ -32,12 +32,69 @@
 
 #ifdef _WIN32
 #include 
+#elif defined(_AIX)
+#include 
+#include 
 #else
 #include 
 #endif
 
 using namespace clang;
 
+#ifdef _AIX
+namespace clang {
+namespace {
+
+template 
+void getClangResourcesPathImplAIX(LibClangPathType &LibClangPath) {
+  int PrevErrno = errno;
+
+  size_t BufSize = 2048u;
+  std::unique_ptr Buf;
+  while (true) {
+Buf = llvm::make_unique(BufSize);
+errno = 0;
+int Ret = loadquery(L_GETXINFO, Buf.get(), (unsigned int)BufSize);
+if (Ret != -1)
+  break; // loadquery() was successful.
+if (errno != ENOMEM)
+  llvm_unreachable("Encountered an unexpected loadquery() failure");
+
+// errno == ENOMEM; try to allocate more memory.
+if ((BufSize & ~((-1u) >> 1u)) != 0u)
+  llvm::report_fatal_error("BufSize needed for loadquery() too large");
+
+Buf.release();
+BufSize <<= 1u;
+  }
+
+  // Extract the function entry point from the function descriptor.
+  uint64_t EntryAddr =
+  reinterpret_cast(clang_createTranslationUnit);
+
+  // Loop to locate the function entry point in the loadquery() results.
+  ld_xinfo *CurInfo = reinterpret_cast(Buf.get());
+  while (true) {
+uint64_t CurTextStart = (uint64_t)CurInfo->ldinfo_textorg;
+uint64_t CurTextEnd = CurTextStart + CurInfo->ldinfo_textsize;
+if (CurTextStart <= EntryAddr && EntryAddr < CurTextEnd)
+  break; // Successfully located.
+
+if (CurInfo->ldinfo_next == 0u)
+  llvm::report_fatal_error("Cannot locate entry point in "
+   "the loadquery() results");
+CurInfo = reinterpret_cast(reinterpret_cast(CurInfo) +
+   CurInfo->ldinfo_next);
+  }
+
+  LibClangPath += reinterpret_cast(CurInfo) + CurInfo->ldinfo_filename;
+  errno = PrevErrno;
+}
+
+} // end anonymous namespace
+} // end namespace clang
+#endif
+
 const std::string &CIndexer::getClangResourcesPath() {
   // Did we already compute the path?
   if (!ResourcesPath.empty())
@@ -64,6 +121,8 @@
 #endif
 
   LibClangPath += path;
+#elif defined(_AIX)
+  getClangResourcesPathImplAIX(LibClangPath);
 #else
   // This silly cast below avoids a C++ warning.
   Dl_info info;


Index: tools/libclang/CIndexer.cpp
===
--- tools/libclang/CIndexer.cpp
+++ tools/libclang/CIndexer.cpp
@@ -32,12 +32,69 @@
 
 #ifdef _WIN32
 #include 
+#elif defined(_AIX)
+#include 
+#include 
 #else
 #include 
 #endif
 
 using namespace clang;
 
+#ifdef _AIX
+namespace clang {
+namespace {
+
+template 
+void getClangResourcesPathImplAIX(LibClangPathType &LibClangPath) {
+  int PrevErrno = errno;
+
+  size_t BufSize = 2048u;
+  std::unique_ptr Buf;
+  while (true) {
+Buf = llvm::make_unique(BufSize);
+errno = 0;
+int Ret = loadquery(L_GETXINFO, Buf.get(), (unsigned int)BufSize);
+if (Ret != -1)
+  break; // loadquery() was successful.
+if (errno != ENOMEM)
+  llvm_unreachable("Encountered an unexpected loadquery() failure");
+
+// errno == ENOMEM; try to allocate more memory.
+if ((BufSize & ~((-1u) >> 1u)) != 0u)
+  llvm::report_fatal_error("BufSize needed for loadquery() too large");
+
+Buf.release();
+BufSize <<= 1u;
+  }
+
+  // Extract the function entry point from the function descriptor.
+  uint64_t EntryAddr =
+  reinterpret_cast(clang_createTranslationUnit);
+
+  // Loop to locate the function entry point in the loadquery() results.
+  ld_xinfo *CurInfo = reinterpret_cast(Buf.get());
+  while (true) {
+uint64_t CurTextStart = (uint64_t)CurInfo->ldinfo_textorg;
+uint64_t CurTextEnd = CurTextStart + CurInfo->ldinfo_textsize;
+if (CurTextStart <= EntryAddr && EntryAddr < CurTextEnd)
+  break; // Successfully located.
+
+if (CurInfo->ldinfo_next == 0u)
+  llvm::report_fatal_error("Cannot locate entry point in "
+   "the loadquery() results");
+CurInfo = reinterpret_cast(reinterpret_cast(CurInfo) +
+   CurInfo->ldinfo_next);
+  }
+
+  LibClangPath += reinterpret_cast(CurInfo) + CurInfo->ldinfo_filename;
+  errno = PrevErrno;
+}
+
+} // end anonymous namespace
+} // end namespace clang
+#endif
+
 const std::string &CIndexer::getClangResourcesPath() {
   // Di

[PATCH] D59741: [lit] Set shlibpath_var on AIX

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast created this revision.
hubert.reinterpretcast added reviewers: xingxue, jasonliu, sfertile.
Herald added a reviewer: serge-sans-paille.
Herald added subscribers: cfe-commits, jsji.
Herald added a project: clang.

When building the `check-all` target on AIX, lit produces

  warning: unable to inject shared library path on 'AIX'

This patch addresses this. `LIBPATH` is the environment variable of interest on 
AIX. Newer versions of AIX may consider `LD_LIBRARY_PATH`, but only when 
`LIBPATH` is unset.


Repository:
  rC Clang

https://reviews.llvm.org/D59741

Files:
  test/Unit/lit.cfg.py


Index: test/Unit/lit.cfg.py
===
--- test/Unit/lit.cfg.py
+++ test/Unit/lit.cfg.py
@@ -42,6 +42,8 @@
 yield 'DYLD_LIBRARY_PATH'
 elif platform.system() == 'Windows':
 yield 'PATH'
+elif platform.system() == 'AIX':
+yield 'LIBPATH'
 
 for shlibpath_var in find_shlibpath_var():
 # in stand-alone builds, shlibdir is clang's build tree


Index: test/Unit/lit.cfg.py
===
--- test/Unit/lit.cfg.py
+++ test/Unit/lit.cfg.py
@@ -42,6 +42,8 @@
 yield 'DYLD_LIBRARY_PATH'
 elif platform.system() == 'Windows':
 yield 'PATH'
+elif platform.system() == 'AIX':
+yield 'LIBPATH'
 
 for shlibpath_var in find_shlibpath_var():
 # in stand-alone builds, shlibdir is clang's build tree
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59741: [lit] Set shlibpath_var on AIX

[Hubert Tong via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC357334: [lit] Set shlibpath_var on AIX (authored by 
hubert.reinterpretcast, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D59741?vs=192014&id=192942#toc

Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59741/new/

https://reviews.llvm.org/D59741

Files:
  test/Unit/lit.cfg.py


Index: test/Unit/lit.cfg.py
===
--- test/Unit/lit.cfg.py
+++ test/Unit/lit.cfg.py
@@ -42,6 +42,8 @@
 yield 'DYLD_LIBRARY_PATH'
 elif platform.system() == 'Windows':
 yield 'PATH'
+elif platform.system() == 'AIX':
+yield 'LIBPATH'
 
 for shlibpath_var in find_shlibpath_var():
 # in stand-alone builds, shlibdir is clang's build tree


Index: test/Unit/lit.cfg.py
===
--- test/Unit/lit.cfg.py
+++ test/Unit/lit.cfg.py
@@ -42,6 +42,8 @@
 yield 'DYLD_LIBRARY_PATH'
 elif platform.system() == 'Windows':
 yield 'PATH'
+elif platform.system() == 'AIX':
+yield 'LIBPATH'
 
 for shlibpath_var in find_shlibpath_var():
 # in stand-alone builds, shlibdir is clang's build tree
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57896: Variable names rule

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

I am generally in favour of this direction.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57896/new/

https://reviews.llvm.org/D57896



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

[PATCH] D57896: Variable names rule

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

In D57896#1391611 , @zturner wrote:

> Is this actually any better?  Whereas before we can’t differentiate type 
> names and variable names, under this proposal we can’t differentiate type 
> names and function names.  So it seems a bit of “6 of 1, half dozen of 
> another”


Perhaps you mistyped? The proposal does not change the status quo of either 
type names nor function names. If you mean that we can't differentiate variable 
names and function names, then it seems worthwhile to point out that the actual 
letters (not just the case of said letters) matter too. Whereas the guidelines 
state that types and variables should have names that are nouns, the guidelines 
state that functions should have names that are verb phrases.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57896/new/

https://reviews.llvm.org/D57896



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

[PATCH] D58128: [PowerPC] Stop defining _ARCH_PWR6X on POWER7 and up

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast created this revision.
hubert.reinterpretcast added reviewers: echristo, hfinkel, kbarton, nemanjai, 
wschmidt.
Herald added a subscriber: jsji.
Herald added a project: clang.

The predefined macro `_ARCH_PWR6X` is associated with GCC's `-mcpu=power6x` 
option, which enables generation of P6  "raw mode" 
instructions such as `mftgpr`.

Later POWER processors build upon the "architected mode", not the raw one. 
`_ARCH_PWR6X` should not be defined for these later processors.

Fixes PR#40236.


Repository:
  rC Clang

https://reviews.llvm.org/D58128

Files:
  lib/Basic/Targets/PPC.h
  test/Preprocessor/init.c


Index: test/Preprocessor/init.c
===
--- test/Preprocessor/init.c
+++ test/Preprocessor/init.c
@@ -5991,7 +5991,7 @@
 // PPC64LE:#define _ARCH_PWR5 1
 // PPC64LE:#define _ARCH_PWR5X 1
 // PPC64LE:#define _ARCH_PWR6 1
-// PPC64LE:#define _ARCH_PWR6X 1
+// PPC64LE-NOT:#define _ARCH_PWR6X 1
 // PPC64LE:#define _ARCH_PWR7 1
 // PPC64LE:#define _CALL_ELF 2
 // PPC64LE:#define _LITTLE_ENDIAN 1
@@ -6331,7 +6331,7 @@
 // PPCPWR7:#define _ARCH_PWR5 1
 // PPCPWR7:#define _ARCH_PWR5X 1
 // PPCPWR7:#define _ARCH_PWR6 1
-// PPCPWR7:#define _ARCH_PWR6X 1
+// PPCPWR7-NOT:#define _ARCH_PWR6X 1
 // PPCPWR7:#define _ARCH_PWR7 1
 //
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc64-none-none 
-target-cpu power7 -fno-signed-char < /dev/null | FileCheck -match-full-lines 
-check-prefix PPCPOWER7 %s
@@ -6344,7 +6344,7 @@
 // PPCPOWER7:#define _ARCH_PWR5 1
 // PPCPOWER7:#define _ARCH_PWR5X 1
 // PPCPOWER7:#define _ARCH_PWR6 1
-// PPCPOWER7:#define _ARCH_PWR6X 1
+// PPCPOWER7-NOT:#define _ARCH_PWR6X 1
 // PPCPOWER7:#define _ARCH_PWR7 1
 //
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc64-none-none 
-target-cpu pwr8 -fno-signed-char < /dev/null | FileCheck -match-full-lines 
-check-prefix PPCPWR8 %s
@@ -6357,7 +6357,7 @@
 // PPCPWR8:#define _ARCH_PWR5 1
 // PPCPWR8:#define _ARCH_PWR5X 1
 // PPCPWR8:#define _ARCH_PWR6 1
-// PPCPWR8:#define _ARCH_PWR6X 1
+// PPCPWR8-NOT:#define _ARCH_PWR6X 1
 // PPCPWR8:#define _ARCH_PWR7 1
 // PPCPWR8:#define _ARCH_PWR8 1
 //
@@ -6374,7 +6374,7 @@
 // PPCPOWER8:#define _ARCH_PWR5 1
 // PPCPOWER8:#define _ARCH_PWR5X 1
 // PPCPOWER8:#define _ARCH_PWR6 1
-// PPCPOWER8:#define _ARCH_PWR6X 1
+// PPCPOWER8-NOT:#define _ARCH_PWR6X 1
 // PPCPOWER8:#define _ARCH_PWR7 1
 // PPCPOWER8:#define _ARCH_PWR8 1
 //
@@ -6388,7 +6388,7 @@
 // PPCPWR9:#define _ARCH_PWR5 1
 // PPCPWR9:#define _ARCH_PWR5X 1
 // PPCPWR9:#define _ARCH_PWR6 1
-// PPCPWR9:#define _ARCH_PWR6X 1
+// PPCPWR9-NOT:#define _ARCH_PWR6X 1
 // PPCPWR9:#define _ARCH_PWR7 1
 // PPCPWR9:#define _ARCH_PWR9 1
 //
@@ -6402,7 +6402,7 @@
 // PPCPOWER9:#define _ARCH_PWR5 1
 // PPCPOWER9:#define _ARCH_PWR5X 1
 // PPCPOWER9:#define _ARCH_PWR6 1
-// PPCPOWER9:#define _ARCH_PWR6X 1
+// PPCPOWER9-NOT:#define _ARCH_PWR6X 1
 // PPCPOWER9:#define _ARCH_PWR7 1
 // PPCPOWER9:#define _ARCH_PWR9 1
 //
Index: lib/Basic/Targets/PPC.h
===
--- lib/Basic/Targets/PPC.h
+++ lib/Basic/Targets/PPC.h
@@ -131,19 +131,18 @@
 ArchDefinePwr5 | ArchDefinePwr4 | ArchDefinePpcgr |
 ArchDefinePpcsq)
   .Cases("power7", "pwr7",
-ArchDefinePwr7 | ArchDefinePwr6x | ArchDefinePwr6 |
-ArchDefinePwr5x | ArchDefinePwr5 | ArchDefinePwr4 |
-ArchDefinePpcgr | ArchDefinePpcsq)
+ ArchDefinePwr7 | ArchDefinePwr6 | ArchDefinePwr5x |
+ ArchDefinePwr5 | ArchDefinePwr4 | ArchDefinePpcgr |
+ ArchDefinePpcsq)
   // powerpc64le automatically defaults to at least power8.
   .Cases("power8", "pwr8", "ppc64le",
-ArchDefinePwr8 | ArchDefinePwr7 | ArchDefinePwr6x |
-ArchDefinePwr6 | ArchDefinePwr5x | ArchDefinePwr5 |
-ArchDefinePwr4 | ArchDefinePpcgr | ArchDefinePpcsq)
+ ArchDefinePwr8 | ArchDefinePwr7 | ArchDefinePwr6 |
+ ArchDefinePwr5x | ArchDefinePwr5 | ArchDefinePwr4 |
+ ArchDefinePpcgr | ArchDefinePpcsq)
   .Cases("power9", "pwr9",
-ArchDefinePwr9 | ArchDefinePwr8 | ArchDefinePwr7 |
-ArchDefinePwr6x | ArchDefinePwr6 | ArchDefinePwr5x |
-ArchDefinePwr5 | ArchDefinePwr4 | ArchDefinePpcgr |
-ArchDefinePpcsq)
+ ArchDefinePwr9 | ArchDefinePwr8 | ArchDefinePwr7 |
+ ArchDefinePwr6 | ArchDefinePwr5x | ArchDefinePwr5 |
+ ArchDefinePwr4 | ArchDefinePpcgr | ArchDefinePpcsq)
   .Default(ArchDefineNone);
 }
 return CPUKnown;


Index: test/Preprocessor/init.c
===

[PATCH] D58128: [PowerPC] Stop defining _ARCH_PWR6X on POWER7 and up

[Hubert Tong via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC353975: [PowerPC] Stop defining _ARCH_PWR6X on POWER7 and up 
(authored by hubert.reinterpretcast, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D58128?vs=186480&id=186715#toc

Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58128/new/

https://reviews.llvm.org/D58128

Files:
  lib/Basic/Targets/PPC.h
  test/Preprocessor/init.c


Index: test/Preprocessor/init.c
===
--- test/Preprocessor/init.c
+++ test/Preprocessor/init.c
@@ -5991,7 +5991,7 @@
 // PPC64LE:#define _ARCH_PWR5 1
 // PPC64LE:#define _ARCH_PWR5X 1
 // PPC64LE:#define _ARCH_PWR6 1
-// PPC64LE:#define _ARCH_PWR6X 1
+// PPC64LE-NOT:#define _ARCH_PWR6X 1
 // PPC64LE:#define _ARCH_PWR7 1
 // PPC64LE:#define _CALL_ELF 2
 // PPC64LE:#define _LITTLE_ENDIAN 1
@@ -6331,7 +6331,7 @@
 // PPCPWR7:#define _ARCH_PWR5 1
 // PPCPWR7:#define _ARCH_PWR5X 1
 // PPCPWR7:#define _ARCH_PWR6 1
-// PPCPWR7:#define _ARCH_PWR6X 1
+// PPCPWR7-NOT:#define _ARCH_PWR6X 1
 // PPCPWR7:#define _ARCH_PWR7 1
 //
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc64-none-none 
-target-cpu power7 -fno-signed-char < /dev/null | FileCheck -match-full-lines 
-check-prefix PPCPOWER7 %s
@@ -6344,7 +6344,7 @@
 // PPCPOWER7:#define _ARCH_PWR5 1
 // PPCPOWER7:#define _ARCH_PWR5X 1
 // PPCPOWER7:#define _ARCH_PWR6 1
-// PPCPOWER7:#define _ARCH_PWR6X 1
+// PPCPOWER7-NOT:#define _ARCH_PWR6X 1
 // PPCPOWER7:#define _ARCH_PWR7 1
 //
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc64-none-none 
-target-cpu pwr8 -fno-signed-char < /dev/null | FileCheck -match-full-lines 
-check-prefix PPCPWR8 %s
@@ -6357,7 +6357,7 @@
 // PPCPWR8:#define _ARCH_PWR5 1
 // PPCPWR8:#define _ARCH_PWR5X 1
 // PPCPWR8:#define _ARCH_PWR6 1
-// PPCPWR8:#define _ARCH_PWR6X 1
+// PPCPWR8-NOT:#define _ARCH_PWR6X 1
 // PPCPWR8:#define _ARCH_PWR7 1
 // PPCPWR8:#define _ARCH_PWR8 1
 //
@@ -6374,7 +6374,7 @@
 // PPCPOWER8:#define _ARCH_PWR5 1
 // PPCPOWER8:#define _ARCH_PWR5X 1
 // PPCPOWER8:#define _ARCH_PWR6 1
-// PPCPOWER8:#define _ARCH_PWR6X 1
+// PPCPOWER8-NOT:#define _ARCH_PWR6X 1
 // PPCPOWER8:#define _ARCH_PWR7 1
 // PPCPOWER8:#define _ARCH_PWR8 1
 //
@@ -6388,7 +6388,7 @@
 // PPCPWR9:#define _ARCH_PWR5 1
 // PPCPWR9:#define _ARCH_PWR5X 1
 // PPCPWR9:#define _ARCH_PWR6 1
-// PPCPWR9:#define _ARCH_PWR6X 1
+// PPCPWR9-NOT:#define _ARCH_PWR6X 1
 // PPCPWR9:#define _ARCH_PWR7 1
 // PPCPWR9:#define _ARCH_PWR9 1
 //
@@ -6402,7 +6402,7 @@
 // PPCPOWER9:#define _ARCH_PWR5 1
 // PPCPOWER9:#define _ARCH_PWR5X 1
 // PPCPOWER9:#define _ARCH_PWR6 1
-// PPCPOWER9:#define _ARCH_PWR6X 1
+// PPCPOWER9-NOT:#define _ARCH_PWR6X 1
 // PPCPOWER9:#define _ARCH_PWR7 1
 // PPCPOWER9:#define _ARCH_PWR9 1
 //
Index: lib/Basic/Targets/PPC.h
===
--- lib/Basic/Targets/PPC.h
+++ lib/Basic/Targets/PPC.h
@@ -131,19 +131,18 @@
 ArchDefinePwr5 | ArchDefinePwr4 | ArchDefinePpcgr |
 ArchDefinePpcsq)
   .Cases("power7", "pwr7",
-ArchDefinePwr7 | ArchDefinePwr6x | ArchDefinePwr6 |
-ArchDefinePwr5x | ArchDefinePwr5 | ArchDefinePwr4 |
-ArchDefinePpcgr | ArchDefinePpcsq)
+ ArchDefinePwr7 | ArchDefinePwr6 | ArchDefinePwr5x |
+ ArchDefinePwr5 | ArchDefinePwr4 | ArchDefinePpcgr |
+ ArchDefinePpcsq)
   // powerpc64le automatically defaults to at least power8.
   .Cases("power8", "pwr8", "ppc64le",
-ArchDefinePwr8 | ArchDefinePwr7 | ArchDefinePwr6x |
-ArchDefinePwr6 | ArchDefinePwr5x | ArchDefinePwr5 |
-ArchDefinePwr4 | ArchDefinePpcgr | ArchDefinePpcsq)
+ ArchDefinePwr8 | ArchDefinePwr7 | ArchDefinePwr6 |
+ ArchDefinePwr5x | ArchDefinePwr5 | ArchDefinePwr4 |
+ ArchDefinePpcgr | ArchDefinePpcsq)
   .Cases("power9", "pwr9",
-ArchDefinePwr9 | ArchDefinePwr8 | ArchDefinePwr7 |
-ArchDefinePwr6x | ArchDefinePwr6 | ArchDefinePwr5x |
-ArchDefinePwr5 | ArchDefinePwr4 | ArchDefinePpcgr |
-ArchDefinePpcsq)
+ ArchDefinePwr9 | ArchDefinePwr8 | ArchDefinePwr7 |
+ ArchDefinePwr6 | ArchDefinePwr5x | ArchDefinePwr5 |
+ ArchDefinePwr4 | ArchDefinePpcgr | ArchDefinePpcsq)
   .Default(ArchDefineNone);
 }
 return CPUKnown;


Index: test/Preprocessor/init.c
===
--- test/Preprocessor/init.c
+++ test/Preprocessor/init.c
@@ -5991,7 +5991,7 @@
 // PPC64LE:#define _ARCH_PWR5 1
 // PPC64LE:#define _ARCH_PWR5X 

[PATCH] D18360: Add AIX Target/ToolChain to Clang Driver

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.
Herald added subscribers: jdoerfert, arphaman.

@apaprocki, as mentioned in our recent RFC ( 
http://lists.llvm.org/pipermail/llvm-dev/2019-February/130175.html ), IBM is 
working on AIX support for Clang and LLVM. We would like to continue the work 
on this patch; however, as you may be aware, LLVM is undergoing relicensing ( 
https://llvm.org/docs/DeveloperPolicy.html#relicensing ). As such, we would 
like to know whether this patch is available under the terms of the Apache 2.0 
License with LLVM exceptions. Thanks.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D18360/new/

https://reviews.llvm.org/D18360



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

[PATCH] D18360: Add AIX Target/ToolChain to Clang Driver

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

In D18360#1406455 , @apaprocki wrote:

> @hubert.reinterpretcast Yes, this patch is available under the new license.


Thank you, @apaprocki. We will be splitting this into updated and more granular 
patches.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D18360/new/

https://reviews.llvm.org/D18360



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

[PATCH] D59253: [AIX][libcxx] AIX system headers need stdint.h and inttypes.h to be re-enterable when macro _STD_TYPES_T is defined

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast accepted this revision.
hubert.reinterpretcast added a comment.

LGTM with minor comment.




Comment at: libcxx/test/std/depr/depr.c.headers/stdint_h.sh.cpp:21
+// Test that limits macros are available when  is included with
+// or without macro _XOPEN_SOUECE=700.
+

Typo:
s/_XOPEN_SOUECE/_XOPEN_SOURCE/;


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59253/new/

https://reviews.llvm.org/D59253



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

[PATCH] D62533: Build with _XOPEN_SOURCE defined on AIX

[Hubert Tong via Phabricator via cfe-commits]

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

@daltenty, I believe that you have had a good number of patches committed into 
the project already (rL360898 , rL361410 
, rL362454 
). Please go ahead with requesting commit 
access, and commit this patch when you are ready.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62533/new/

https://reviews.llvm.org/D62533



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

[Diffusion] rL358949: [PowerPC] [Clang] Port MMX intrinsics and basic test cases to Power

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a subscriber: cfe-commits.

https://reviews.llvm.org/rL358949



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

[Diffusion] rL358949: [PowerPC] [Clang] Port MMX intrinsics and basic test cases to Power

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.

/cfe/trunk/lib/Driver/ToolChains/PPCLinux.cpp:26 I'm not particularly concerned 
about the scoping of the work; however, the `ppc_wrappers` directory is not at 
all indicative of that scoping. It neither indicates the scope in terms of 
purpose (providing x86 vector intrinsics) nor does it indicate the scope in 
terms of applicability (64-bit Linux). I would suggest that the header belongs 
as part of a deeper structure that can be used for other purposes as well 
`ppc_wrappers/ppc64common/linux/`. If the header becomes applicable to more 
platforms, we can move it with the possibility of additionally placing headers 
that use `#include_next` as appropriate.

https://reviews.llvm.org/rL358949



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

[Diffusion] rL358949: [PowerPC] [Clang] Port MMX intrinsics and basic test cases to Power

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

> I think the name `ppc_wrappers` is accurate to describe its meaning that 
> wrapping the headers under standard library path or standard header search 
> path.

Thanks for clarifying this for me. In the case where all of the headers are 
expected to be present somewhere in the usual search path, I agree that the 
current directory structure would be okay given proper guards within the 
headers themselves.

> [ ... ] which I also prefer to use because it's easier to maintain headers 
> instead of modifying cpp code about toolchain class such as PPCLinuxToolChain 
> .



> We will use better way described above to guard those headers only in 64-bit 
> mode soon after other headers have been upstreamed.

That (controlling within the headers) sounds good to me. When that lands, then 
my understanding is that the `PPCLinuxToolChain` code would be adjusted to 
include `ppc_wrappers` in the search path in the general case. Please let me 
know if that's not the case. Thanks.


https://reviews.llvm.org/rL358949



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

[PATCH] D62949: [analyzer][tests] Add normalize_plist to replace diff_plist

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast created this revision.
hubert.reinterpretcast added reviewers: NoQ, sfertile, xingxue, jasonliu, 
daltenty.
Herald added subscribers: jsji, Charusso, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun.
Herald added a project: clang.

The `%diff_plist` lit substitution invokes `diff` with a non-portable `-I` 
option. The intended effect can be achieved by normalizing the inputs to `diff` 
beforehand. Such normalization can be done with `grep -Ev`, which is also used 
by other tests.

This patch applies the change described in 
http://lists.llvm.org/pipermail/cfe-dev/2019-April/061904.html to the specific 
case shown in the list message. Mechanical changes to the other affected files 
will follow in later patches.

Note that `grep` expects text files (ending with a newline) as input. The 
`echo` command is used to generate a newline for the test output files, which 
do not have such newlines.


Repository:
  rC Clang

https://reviews.llvm.org/D62949

Files:
  test/Analysis/lit.local.cfg
  test/Analysis/unix-fns.c


Index: test/Analysis/unix-fns.c
===
--- test/Analysis/unix-fns.c
+++ test/Analysis/unix-fns.c
@@ -1,5 +1,5 @@
 // RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 
-analyzer-checker=core,unix.API,osx.API,optin.portability %s 
-analyzer-store=region -analyzer-output=plist -analyzer-config faux-bodies=true 
 -fblocks -verify -o %t.plist
-// RUN: cat %t.plist | %diff_plist %S/Inputs/expected-plists/unix-fns.c.plist -
+// RUN: %normalize_plist <%S/Inputs/expected-plists/unix-fns.c.plist 
>%t.expected.sed.plist && echo >>%t.plist && %normalize_plist <%t.plist | diff 
-u %t.expected.sed.plist -
 // RUN: mkdir -p %t.dir
 // RUN: %clang_analyze_cc1 
-analyzer-checker=core,unix.API,osx.API,optin.portability -analyzer-output=html 
-analyzer-config faux-bodies=true -fblocks -o %t.dir %s
 // RUN: rm -fR %t.dir
Index: test/Analysis/lit.local.cfg
===
--- test/Analysis/lit.local.cfg
+++ test/Analysis/lit.local.cfg
@@ -14,6 +14,14 @@
 config.substitutions.append(('%diff_plist',
 'diff -u -w -I "/" -I ".:" -I "version"'))
 
+# Filtering command used by Clang Analyzer tests (when comparing .plist files
+# with reference output)
+config.substitutions.append(('%normalize_plist',
+"grep -Ev '%s|%s|%s'" %
+('^[[:space:]]*.* version .*$',
+ '^[[:space:]]*/.*$',
+ '^[[:space:]]*.:.*$')))
+
 # Diff command for testing SARIF output to reference output.
 config.substitutions.append(('%diff_sarif',
 '''diff -U1 -w -I ".*file:.*%basename_t" -I '"version":' -I 
"2\.0\.0\-csd\.[0-9]*\.beta\."'''))


Index: test/Analysis/unix-fns.c
===
--- test/Analysis/unix-fns.c
+++ test/Analysis/unix-fns.c
@@ -1,5 +1,5 @@
 // RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core,unix.API,osx.API,optin.portability %s -analyzer-store=region -analyzer-output=plist -analyzer-config faux-bodies=true  -fblocks -verify -o %t.plist
-// RUN: cat %t.plist | %diff_plist %S/Inputs/expected-plists/unix-fns.c.plist -
+// RUN: %normalize_plist <%S/Inputs/expected-plists/unix-fns.c.plist >%t.expected.sed.plist && echo >>%t.plist && %normalize_plist <%t.plist | diff -u %t.expected.sed.plist -
 // RUN: mkdir -p %t.dir
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.API,osx.API,optin.portability -analyzer-output=html -analyzer-config faux-bodies=true -fblocks -o %t.dir %s
 // RUN: rm -fR %t.dir
Index: test/Analysis/lit.local.cfg
===
--- test/Analysis/lit.local.cfg
+++ test/Analysis/lit.local.cfg
@@ -14,6 +14,14 @@
 config.substitutions.append(('%diff_plist',
 'diff -u -w -I "/" -I ".:" -I "version"'))
 
+# Filtering command used by Clang Analyzer tests (when comparing .plist files
+# with reference output)
+config.substitutions.append(('%normalize_plist',
+"grep -Ev '%s|%s|%s'" %
+('^[[:space:]]*.* version .*$',
+ '^[[:space:]]*/.*$',
+ '^[[:space:]]*.:.*$')))
+
 # Diff command for testing SARIF output to reference output.
 config.substitutions.append(('%diff_sarif',
 '''diff -U1 -w -I ".*file:.*%basename_t" -I '"version":' -I "2\.0\.0\-csd\.[0-9]*\.beta\."'''))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62950: [analyzer][tests] Use normalize_plist in place of diff_plist (`cat` cases)

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast created this revision.
hubert.reinterpretcast added reviewers: NoQ, sfertile, xingxue, jasonliu, 
daltenty.
Herald added subscribers: jsji, Charusso, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun.
Herald added a project: clang.
hubert.reinterpretcast added a parent revision: D62949: [analyzer][tests] Add 
normalize_plist to replace diff_plist.

The `%diff_plist` lit substitution invokes `diff` with a non-portable `-I` 
option. The intended effect can be achieved by normalizing the inputs to `diff` 
beforehand. Such normalization can be done with `grep -Ev`, which is also used 
by other tests.

This patch applies the change described in 
http://lists.llvm.org/pipermail/cfe-dev/2019-April/061904.html mechanically to 
the cases where the output file is piped to `%diff_plist` via `cat`.

The changes were applied via a script, except that 
`clang/test/Analysis/NewDelete-path-notes.cpp` and 
`clang/test/Analysis/plist-macros-with-expansion.cpp` were each adjusted for 
the line-continuation on the relevant `RUN` step.

Note that `grep` expects text files (ending with a newline) as input. The 
`echo` command is used to generate a newline for the test output files, which 
do not have such newlines.


Repository:
  rC Clang

https://reviews.llvm.org/D62950

Files:
  test/Analysis/NewDelete-path-notes.cpp
  test/Analysis/conditional-path-notes.c
  test/Analysis/copypaste/plist-diagnostics-notes-as-events.cpp
  test/Analysis/copypaste/plist-diagnostics.cpp
  test/Analysis/cxx-for-range.cpp
  test/Analysis/diagnostics/deref-track-symbolic-region.c
  test/Analysis/diagnostics/report-issues-within-main-file.cpp
  test/Analysis/diagnostics/undef-value-caller.c
  test/Analysis/diagnostics/undef-value-param.c
  test/Analysis/diagnostics/undef-value-param.m
  test/Analysis/edges-new.mm
  test/Analysis/generics.m
  test/Analysis/inline-plist.c
  test/Analysis/inline-unique-reports.c
  test/Analysis/inlining/eager-reclamation-path-notes.c
  test/Analysis/inlining/eager-reclamation-path-notes.cpp
  test/Analysis/inlining/path-notes.c
  test/Analysis/inlining/path-notes.cpp
  test/Analysis/inlining/path-notes.m
  test/Analysis/method-call-path-notes.cpp
  test/Analysis/model-file.cpp
  test/Analysis/null-deref-path-notes.m
  test/Analysis/nullability-notes.m
  test/Analysis/objc-arc.m
  test/Analysis/objc-radar17039661.m
  test/Analysis/plist-macros-with-expansion.cpp
  test/Analysis/plist-macros.cpp
  test/Analysis/plist-output-alternate.m
  test/Analysis/plist-output.m
  test/Analysis/retain-release-path-notes.m
  test/Analysis/retain-release.m

Index: test/Analysis/retain-release.m
===
--- test/Analysis/retain-release.m
+++ test/Analysis/retain-release.m
@@ -17,8 +17,8 @@
 // RUN: -Wno-objc-root-class -x objective-c++ -std=gnu++98\
 // RUN: -analyzer-config osx.cocoa.RetainCount:TrackNSCFStartParam=true\
 // RUN: -DTRACK_START_PARAM
-// RUN: cat %t.objcpp.plist | %diff_plist %S/Inputs/expected-plists/retain-release.m.objcpp.plist -
-// RUN: cat %t.objc.plist | %diff_plist %S/Inputs/expected-plists/retain-release.m.objc.plist -
+// RUN: %normalize_plist <%S/Inputs/expected-plists/retain-release.m.objcpp.plist >%t.objcpp.expected.sed.plist && echo >>%t.objcpp.plist && %normalize_plist <%t.objcpp.plist | diff -u %t.objcpp.expected.sed.plist -
+// RUN: %normalize_plist <%S/Inputs/expected-plists/retain-release.m.objc.plist >%t.objc.expected.sed.plist && echo >>%t.objc.plist && %normalize_plist <%t.objc.plist | diff -u %t.objc.expected.sed.plist -
 
 void clang_analyzer_eval(int);
 
Index: test/Analysis/retain-release-path-notes.m
===
--- test/Analysis/retain-release-path-notes.m
+++ test/Analysis/retain-release-path-notes.m
@@ -1,6 +1,6 @@
 // RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core,osx.coreFoundation.CFRetainRelease,osx.cocoa.ClassRelease,osx.cocoa.RetainCount -analyzer-store=region -analyzer-output=text -verify %s
 // RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core,osx.coreFoundation.CFRetainRelease,osx.cocoa.ClassRelease,osx.cocoa.RetainCount -analyzer-store=region -analyzer-output=plist-multi-file %s -o %t
-// RUN: cat %t | %diff_plist %S/Inputs/expected-plists/retain-release-path-notes.m.plist -
+// RUN: %normalize_plist <%S/Inputs/expected-plists/retain-release-path-notes.m.plist >%t.expected.sed && echo >>%t && %normalize_plist <%t | diff -u %t.expected.sed -
 
 /***
 This file is for testing the path-sensitive notes for retain/release errors.
Index: test/Analysis/plist-output.m
===
--- test/Analysis/plist-output.m
+++ test/Analysis/plist-output.m
@@ -1,5 +1,5 @@
 // RUN: %clang_analyze_cc1 -analyzer-config eagerly-assume=false %s -analyzer-checker=osx.cocoa.RetainCount,deadcode.DeadS

[PATCH] D62951: [analyzer][tests] Use normalize_plist in place of diff_plist (`tail` cases)

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast created this revision.
hubert.reinterpretcast added reviewers: NoQ, sfertile, xingxue, jasonliu, 
daltenty.
Herald added subscribers: jsji, Charusso, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun.
Herald added a project: clang.
hubert.reinterpretcast added parent revisions: D62949: [analyzer][tests] Add 
normalize_plist to replace diff_plist, D62950: [analyzer][tests] Use 
normalize_plist in place of diff_plist (`cat` cases).

The `%diff_plist` lit substitution invokes `diff` with a non-portable `-I` 
option. The intended effect can be achieved by normalizing the inputs to `diff` 
beforehand. Such normalization can be done with `grep -Ev`, which is also used 
by other tests.

This patch applies the change described in 
http://lists.llvm.org/pipermail/cfe-dev/2019-April/061904.html mechanically to 
the cases where the output file is piped to `%diff_plist` via `tail`. 
`%diff_plist` is then, being unused, removed.

The changes were applied via a script.

Note that `grep` expects text files (ending with a newline) as input. The 
`echo` command is used to generate a newline for the test output files, which 
do not have such newlines.


Repository:
  rC Clang

https://reviews.llvm.org/D62951

Files:
  test/Analysis/MismatchedDeallocator-path-notes.cpp
  test/Analysis/diagnostics/plist-diagnostics-include-check.cpp
  test/Analysis/diagnostics/plist-multi-file.c
  test/Analysis/lambda-notes.cpp
  test/Analysis/lit.local.cfg
  test/Analysis/malloc-plist.c


Index: test/Analysis/malloc-plist.c
===
--- test/Analysis/malloc-plist.c
+++ test/Analysis/malloc-plist.c
@@ -1,6 +1,6 @@
 // RUN: rm -f %t
 // RUN: %clang_analyze_cc1 -fblocks -analyzer-checker=core,unix.Malloc 
-analyzer-output=plist -verify -o %t -analyzer-config eagerly-assume=false %s
-// RUN: tail -n +11 %t | %diff_plist 
%S/Inputs/expected-plists/malloc-plist.c.plist -
+// RUN: %normalize_plist <%S/Inputs/expected-plists/malloc-plist.c.plist 
>%t.expected.sed && echo >>%t && tail -n +11 %t | %normalize_plist | diff -u 
%t.expected.sed -
 
 typedef __typeof(sizeof(int)) size_t;
 void *malloc(size_t);
Index: test/Analysis/lit.local.cfg
===
--- test/Analysis/lit.local.cfg
+++ test/Analysis/lit.local.cfg
@@ -9,11 +9,6 @@
 config.test_format = analyzer_test.AnalyzerTest(
 config.test_format.execute_external, config.use_z3_solver)
 
-# Diff command used by Clang Analyzer tests (when comparing .plist files
-# with reference output)
-config.substitutions.append(('%diff_plist',
-'diff -u -w -I "/" -I ".:" -I "version"'))
-
 # Diff command for testing SARIF output to reference output.
 config.substitutions.append(('%diff_sarif',
 '''diff -U1 -w -I ".*file:.*%basename_t" -I '"version":' -I 
"2\.0\.0\-csd\.[0-9]*\.beta\."'''))
Index: test/Analysis/lambda-notes.cpp
===
--- test/Analysis/lambda-notes.cpp
+++ test/Analysis/lambda-notes.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core.DivideZero 
-analyzer-config inline-lambdas=true -analyzer-output plist -verify %s -o %t
-// RUN: tail -n +11 %t | %diff_plist 
%S/Inputs/expected-plists/lambda-notes.cpp.plist -
+// RUN: %normalize_plist <%S/Inputs/expected-plists/lambda-notes.cpp.plist 
>%t.expected.sed && echo >>%t && tail -n +11 %t | %normalize_plist | diff -u 
%t.expected.sed -
 
 
 // Diagnostic inside a lambda
Index: test/Analysis/diagnostics/plist-multi-file.c
===
--- test/Analysis/diagnostics/plist-multi-file.c
+++ test/Analysis/diagnostics/plist-multi-file.c
@@ -1,5 +1,5 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core 
-analyzer-output=plist-multi-file -o %t.plist -verify %s
-// RUN: tail -n +11 %t.plist | %diff_plist 
%S/Inputs/expected-plists/plist-multi-file.c.plist -
+// RUN: %normalize_plist <%S/Inputs/expected-plists/plist-multi-file.c.plist 
>%t.expected.sed.plist && echo >>%t.plist && tail -n +11 %t.plist | 
%normalize_plist | diff -u %t.expected.sed.plist -
 
 #include "plist-multi-file.h"
 
Index: test/Analysis/diagnostics/plist-diagnostics-include-check.cpp
===
--- test/Analysis/diagnostics/plist-diagnostics-include-check.cpp
+++ test/Analysis/diagnostics/plist-diagnostics-include-check.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection 
-analyzer-output=plist-multi-file %s -o %t.plist
-// RUN: tail -n +11 %t.plist | %diff_plist 
%S/Inputs/expected-plists/plist-diagnostics-include-check.cpp.plist -
+// RUN: %normalize_plist 
<%S/Inputs/expected-plists/plist-diagnostics-include-check.cpp.plist 
>%t.expected.sed.plist && echo >>%t.plist && tail -n +11 %t.plist | 
%normalize_plist | diff -u %t.expected.sed.plist -
 
 #include "I

[PATCH] D62952: [analyzer][tests] Use normalize_sarif in place of diff_sarif

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast created this revision.
hubert.reinterpretcast added reviewers: NoQ, sfertile, xingxue, jasonliu, 
daltenty.
Herald added subscribers: jsji, Charusso, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun.
Herald added a project: clang.

The `%diff_sarif` lit substitution invokes `diff` with a non-portable `-I` 
option. The intended effect can be achieved by normalizing the inputs to `diff` 
beforehand. Such normalization can be done with `grep -Ev`, which is also used 
by other tests.

This patch applies a change similar to the one described in 
http://lists.llvm.org/pipermail/cfe-dev/2019-April/061904.html mechanically. 
`%diff_sarif` is then, being unused, removed.

The changes were applied via a script, except that the expected files required 
small updates for changes in the length of the test files.

Note that `grep` expects text files (ending with a newline) as input. The 
`echo` command is used to generate a newline for the test output files, which 
do not have such newlines.


Repository:
  rC Clang

https://reviews.llvm.org/D62952

Files:
  
test/Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif
  
test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
  test/Analysis/diagnostics/sarif-diagnostics-taint-test.c
  test/Analysis/diagnostics/sarif-multi-diagnostic-test.c
  test/Analysis/lit.local.cfg


Index: test/Analysis/lit.local.cfg
===
--- test/Analysis/lit.local.cfg
+++ test/Analysis/lit.local.cfg
@@ -14,9 +14,12 @@
 config.substitutions.append(('%diff_plist',
 'diff -u -w -I "/" -I ".:" -I "version"'))
 
-# Diff command for testing SARIF output to reference output.
-config.substitutions.append(('%diff_sarif',
-'''diff -U1 -w -I ".*file:.*%basename_t" -I '"version":' -I 
"2\.0\.0\-csd\.[0-9]*\.beta\."'''))
+# Filtering command for testing SARIF output against reference output.
+config.substitutions.append(('%normalize_sarif',
+"grep -Ev '%s|%s|%s'" %
+('^[[:space:]]*"uri": "file:.*%basename_t"$',
+ '^[[:space:]]*"version": ".* version .*"$',
+ '^[[:space:]]*"version": "2\.0\.0-csd\.[0-9]*\.beta\.[0-9-]{10}"$')))
 
 if not config.root.clang_staticanalyzer:
 config.unsupported = True
Index: test/Analysis/diagnostics/sarif-multi-diagnostic-test.c
===
--- test/Analysis/diagnostics/sarif-multi-diagnostic-test.c
+++ test/Analysis/diagnostics/sarif-multi-diagnostic-test.c
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 
-analyzer-checker=core,alpha.security.taint,debug.TaintTest %s -verify 
-analyzer-output=sarif -o - | %diff_sarif 
%S/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif -
+// RUN: %clang_analyze_cc1 
-analyzer-checker=core,alpha.security.taint,debug.TaintTest %s -verify 
-analyzer-output=sarif -o %t && echo >>%t && %normalize_sarif 
<%S/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif 
>%t.expected.sed.sarif && %normalize_sarif <%t | diff -U1 -b 
%t.expected.sed.sarif -
 #include "../Inputs/system-header-simulator.h"
 
 int atoi(const char *nptr);
Index: test/Analysis/diagnostics/sarif-diagnostics-taint-test.c
===
--- test/Analysis/diagnostics/sarif-diagnostics-taint-test.c
+++ test/Analysis/diagnostics/sarif-diagnostics-taint-test.c
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 
-analyzer-checker=alpha.security.taint,debug.TaintTest %s -verify 
-analyzer-output=sarif -o - | %diff_sarif 
%S/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif -
+// RUN: %clang_analyze_cc1 
-analyzer-checker=alpha.security.taint,debug.TaintTest %s -verify 
-analyzer-output=sarif -o %t && echo >>%t && %normalize_sarif 
<%S/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif 
>%t.expected.sed.sarif && %normalize_sarif <%t | diff -U1 -b 
%t.expected.sed.sarif -
 #include "../Inputs/system-header-simulator.h"
 
 int atoi(const char *nptr);
Index: 
test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
===
--- 
test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
+++ 
test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
@@ -7,7 +7,7 @@
   "fileLocation": {
 "uri": "file:sarif-multi-diagnostic-test.c"
   },
-  "length": 667,
+  "length": 771,
   "mimeType": "text/plain",
   "roles": [
 "resultFile"
Index: 
test/Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif
===
--- 
test/Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif
+++ 
test/Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics

[PATCH] D62949: [analyzer][tests] Add normalize_plist to replace diff_plist

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

In D62949#1533574 , @NoQ wrote:

> I think we should:
>
> - Pre-normalize our expected outputs so that we didn't have to normalize them 
> in every run-line.


Okay; I think this might be possible to do in a separate patch that does not 
depend on this set.

> - Treat the lack of newline in plist output as a bug and try to fix it.

A quick look at this seems to point at `clang/lib/ARCMigrate/PlistReporter.cpp` 
and `clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp`.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62949/new/

https://reviews.llvm.org/D62949



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

[PATCH] D62949: [analyzer][tests] Add normalize_plist to replace diff_plist

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

In D62949#1533605 , @NoQ wrote:

> Ok! I'll be happy to have this addressed incrementally.


I think it should be safe to at least commit the pre-normalization directly 
first. I'll take a look, and update the patch if the pre-normalization lands.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62949/new/

https://reviews.llvm.org/D62949



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

[PATCH] D63029: [NFC][CUDA] Avoid undefined grep in cuda-types.cu

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast created this revision.
hubert.reinterpretcast added reviewers: jlebar, daltenty, xingxue, jasonliu.
Herald added subscribers: jsji, jfb.
Herald added a project: clang.

vertical-line is not a BRE special character.

POSIX.1-2017 XBD Section 9.3.2 indicates that the interpretation of `\|` is 
undefined. This patch uses EREs instead.

Note that this patch keeps the property that `SIZEOF` and `WIDTH` are 
considered uninteresting except when appearing in `SIZEOF|WIDTH`.


Repository:
  rC Clang

https://reviews.llvm.org/D63029

Files:
  test/Preprocessor/cuda-types.cu


Index: test/Preprocessor/cuda-types.cu
===
--- test/Preprocessor/cuda-types.cu
+++ test/Preprocessor/cuda-types.cu
@@ -8,41 +8,41 @@
 // RUN: mkdir -p %t
 
 // RUN: %clang --cuda-host-only -nocudainc -target i386-unknown-linux-gnu -x 
cuda -E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > %t/i386-host-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF\|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/i386-host-defines-filtered
 // RUN: %clang --cuda-device-only -nocudainc -nocudalib -target 
i386-unknown-linux-gnu -x cuda -E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > %t/i386-device-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF\|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/i386-device-defines-filtered
 // RUN: diff %t/i386-host-defines-filtered %t/i386-device-defines-filtered
 
 // RUN: %clang --cuda-host-only -nocudainc -target x86_64-unknown-linux-gnu -x 
cuda -E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > %t/x86_64-host-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF\|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/x86_64-host-defines-filtered
 // RUN: %clang --cuda-device-only -nocudainc -nocudalib -target 
x86_64-unknown-linux-gnu -x cuda -E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > %t/x86_64-device-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF\|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/x86_64-device-defines-filtered
 // RUN: diff %t/x86_64-host-defines-filtered %t/x86_64-device-defines-filtered
 
 // RUN: %clang --cuda-host-only -nocudainc -target powerpc64-unknown-linux-gnu 
-x cuda -E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > %t/powerpc64-host-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF\|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/powerpc64-host-defines-filtered
 // RUN: %clang --cuda-device-only -nocudainc -nocudalib -target 
powerpc64-unknown-linux-gnu -x cuda -E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > 
%t/powerpc64-device-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF\|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > 
%t/powerpc64-device-defines-filtered
 // RUN: diff %t/powerpc64-host-defines-filtered 
%t/powerpc64-device-defines-filtered
 
 // RUN: %clang --cuda-host-only -nocudainc -target i386-windows-msvc -x cuda 
-E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > %t/i386-msvc-host-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF\|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/i386-msvc-host-defines-filtered
 // RUN: %clang --cuda-device-only -nocudainc -nocudalib -target 
i386-windows-msvc -x cuda -E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > 
%t/i386-msvc-device-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF\|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > 
%t/i386-msvc-device-defines-filtered
 // RUN: diff %t/i386-msvc-host-defines-filtered 
%t/i386-msvc-device-defines-filtered
 
 // RUN: %clang --cuda-host-only -nocudainc -target x86_64-windows-msvc -x cuda 
-E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > 
%t/x86_64-msvc-host-defines-filter

[PATCH] D63029: [NFC][CUDA] Avoid undefined grep in cuda-types.cu

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: test/Preprocessor/cuda-types.cu:11
 // RUN: %clang --cuda-host-only -nocudainc -target i386-unknown-linux-gnu -x 
cuda -E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > %t/i386-host-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF\|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/i386-host-defines-filtered

tra wrote:
> I believe that was a typo in the original test. It should be 
> `TYPE|MAX|SIZEOF|WIDTH` now.
> `SIZEOF` and `WIDTH` are parts of different defines we're looking for.
> 
> E.g
> ```
> #define __SIZEOF_DOUBLE__ 8
> ...
> #define __INTPTR_WIDTH__ 32
> 
> ```
I can give that a try. I'll update on the status of whether the test passes 
once `SIZEOF` and `WIDTH` is actually tested.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63029/new/

https://reviews.llvm.org/D63029



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

[PATCH] D63029: [NFC][CUDA] Avoid undefined grep in cuda-types.cu

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast updated this revision to Diff 203659.
hubert.reinterpretcast added a comment.

Fix grep pattern in cuda-types.cu

As requested, further fix so that the `SIZEOF` and `WIDTH` macros are checked.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63029/new/

https://reviews.llvm.org/D63029

Files:
  test/Preprocessor/cuda-types.cu


Index: test/Preprocessor/cuda-types.cu
===
--- test/Preprocessor/cuda-types.cu
+++ test/Preprocessor/cuda-types.cu
@@ -8,41 +8,41 @@
 // RUN: mkdir -p %t
 
 // RUN: %clang --cuda-host-only -nocudainc -target i386-unknown-linux-gnu -x 
cuda -E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > %t/i386-host-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/i386-host-defines-filtered
 // RUN: %clang --cuda-device-only -nocudainc -nocudalib -target 
i386-unknown-linux-gnu -x cuda -E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > %t/i386-device-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/i386-device-defines-filtered
 // RUN: diff %t/i386-host-defines-filtered %t/i386-device-defines-filtered
 
 // RUN: %clang --cuda-host-only -nocudainc -target x86_64-unknown-linux-gnu -x 
cuda -E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > %t/x86_64-host-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/x86_64-host-defines-filtered
 // RUN: %clang --cuda-device-only -nocudainc -nocudalib -target 
x86_64-unknown-linux-gnu -x cuda -E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > %t/x86_64-device-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/x86_64-device-defines-filtered
 // RUN: diff %t/x86_64-host-defines-filtered %t/x86_64-device-defines-filtered
 
 // RUN: %clang --cuda-host-only -nocudainc -target powerpc64-unknown-linux-gnu 
-x cuda -E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > %t/powerpc64-host-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/powerpc64-host-defines-filtered
 // RUN: %clang --cuda-device-only -nocudainc -nocudalib -target 
powerpc64-unknown-linux-gnu -x cuda -E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > 
%t/powerpc64-device-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > 
%t/powerpc64-device-defines-filtered
 // RUN: diff %t/powerpc64-host-defines-filtered 
%t/powerpc64-device-defines-filtered
 
 // RUN: %clang --cuda-host-only -nocudainc -target i386-windows-msvc -x cuda 
-E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > %t/i386-msvc-host-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/i386-msvc-host-defines-filtered
 // RUN: %clang --cuda-device-only -nocudainc -nocudalib -target 
i386-windows-msvc -x cuda -E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > 
%t/i386-msvc-device-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > 
%t/i386-msvc-device-defines-filtered
 // RUN: diff %t/i386-msvc-host-defines-filtered 
%t/i386-msvc-device-defines-filtered
 
 // RUN: %clang --cuda-host-only -nocudainc -target x86_64-windows-msvc -x cuda 
-E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > 
%t/x86_64-msvc-host-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > 
%t/x86_64-msvc-host-defines-filtered
 // RUN: %clang --cuda-device-only -nocudainc -noc

[PATCH] D63029: [CUDA] Fix grep pattern in cuda-types.cu

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast marked 3 inline comments as done.
hubert.reinterpretcast added inline comments.



Comment at: test/Preprocessor/cuda-types.cu:11
 // RUN: %clang --cuda-host-only -nocudainc -target i386-unknown-linux-gnu -x 
cuda -E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > %t/i386-host-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF\|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/i386-host-defines-filtered

hubert.reinterpretcast wrote:
> tra wrote:
> > I believe that was a typo in the original test. It should be 
> > `TYPE|MAX|SIZEOF|WIDTH` now.
> > `SIZEOF` and `WIDTH` are parts of different defines we're looking for.
> > 
> > E.g
> > ```
> > #define __SIZEOF_DOUBLE__ 8
> > ...
> > #define __INTPTR_WIDTH__ 32
> > 
> > ```
> I can give that a try. I'll update on the status of whether the test passes 
> once `SIZEOF` and `WIDTH` is actually tested.
The test still passes after updating the pattern to pick up `SIZEOF` and 
`WIDTH` macros. I've updated the patch.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63029/new/

https://reviews.llvm.org/D63029



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

[PATCH] D62949: [analyzer][tests] Add normalize_plist to replace diff_plist

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

I've tested the pre-normalization and it looks like I can commit it tomorrow. I 
noticed that the following three files appear to be unreferenced:

  clang/test/Analysis/Inputs/expected-plists/cstring-plist.c.plist
  clang/test/Analysis/Inputs/expected-plists/plist-stats-output.c.plist
  clang/test/Analysis/Inputs/expected-plists/yaccignore.c.plist

I'll commit the deletion of these tomorrow as well.

Modifying the tools to produce a newline at the end of the output also seems to 
work okay with the tests. I've posted the change for that in D63041 
.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62949/new/

https://reviews.llvm.org/D62949



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

[PATCH] D63041: [PlistSupport] Produce a newline to end plist output files

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast created this revision.
hubert.reinterpretcast added reviewers: NoQ, sfertile, xingxue, jasonliu, 
daltenty.
Herald added a subscriber: jsji.
Herald added a project: clang.

As suggested in the review of D62949 , this 
patch updates the plist output to have a newline at the end of the file. This 
makes it so that the plist output file qualifies as a POSIX text file, which 
increases the consumability of the generated plist file in relation to various 
tools.


Repository:
  rC Clang

https://reviews.llvm.org/D63041

Files:
  lib/ARCMigrate/PlistReporter.cpp
  lib/StaticAnalyzer/Core/PlistDiagnostics.cpp


Index: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
===
--- lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
+++ lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
@@ -748,7 +748,7 @@
   }
 
   // Finish.
-  o << "\n";
+  o << "\n\n";
 }
 
 
//===--===//
Index: lib/ARCMigrate/PlistReporter.cpp
===
--- lib/ARCMigrate/PlistReporter.cpp
+++ lib/ARCMigrate/PlistReporter.cpp
@@ -120,5 +120,5 @@
   o << " \n";
 
   // Finish.
-  o << "\n";
+  o << "\n\n";
 }


Index: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
===
--- lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
+++ lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
@@ -748,7 +748,7 @@
   }
 
   // Finish.
-  o << "\n";
+  o << "\n\n";
 }
 
 //===--===//
Index: lib/ARCMigrate/PlistReporter.cpp
===
--- lib/ARCMigrate/PlistReporter.cpp
+++ lib/ARCMigrate/PlistReporter.cpp
@@ -120,5 +120,5 @@
   o << " \n";
 
   // Finish.
-  o << "\n";
+  o << "\n\n";
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62952: [analyzer][tests] Use normalize_sarif in place of diff_sarif

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

@aaron.ballman, for similar cases in the plist output, it has been proposed

- that the reference expected file be committed into the tree pre-normalized, 
and
- that tool be modified such that the output file has a newline at the end of 
the file.

Does that sound good for this format?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62952/new/

https://reviews.llvm.org/D62952



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

[PATCH] D63048: Update __VERSION__ to remove the hardcoded 4.2.1 version

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: docs/ReleaseNotes.rst:79
 - ``clang -dumpversion`` now returns the version of Clang itself.
+  Similarly, __VERSION__ returns the Clang version instead of 4.2.1.
 

Use code font for `__VERSION__`.



Comment at: lib/Frontend/InitPreprocessor.cpp:607
 
-  // As sad as it is, enough software depends on the __VERSION__ for version
-  // checks that it is necessary to report 4.2.1 (the base GCC version we claim
-  // compatibility with) first.
-  Builder.defineMacro("__VERSION__", "\"4.2.1 Compatible " +
-  Twine(getClangFullCPPVersion()) + "\"");
+  // We used to report 4.2.1 as compatibility with gcc
+  // Now, we returns the clang version.

s/as/for/; s/gcc/GCC/;
Add a period to the end of the sentence.



Comment at: lib/Frontend/InitPreprocessor.cpp:608
+  // We used to report 4.2.1 as compatibility with gcc
+  // Now, we returns the clang version.
+  Builder.defineMacro("__VERSION__", "\"" CLANG_VERSION_STRING

s/returns/return/; s/clang/Clang/;


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63048/new/

https://reviews.llvm.org/D63048



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

[PATCH] D63048: Update __VERSION__ to remove the hardcoded 4.2.1 version

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: docs/ReleaseNotes.rst:79
 - ``clang -dumpversion`` now returns the version of Clang itself.
+  Similarly, `__VERSION__` returns the Clang version instead of 4.2.1.
 

I think this needs double backquotes on both sides.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63048/new/

https://reviews.llvm.org/D63048



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

[PATCH] D62949: [analyzer][tests] Add normalize_plist to replace diff_plist

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast updated this revision to Diff 203706.
hubert.reinterpretcast added a comment.

Update based on review comments, building from rL362877 
 and D63041 


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62949/new/

https://reviews.llvm.org/D62949

Files:
  test/Analysis/lit.local.cfg
  test/Analysis/unix-fns.c


Index: test/Analysis/unix-fns.c
===
--- test/Analysis/unix-fns.c
+++ test/Analysis/unix-fns.c
@@ -1,5 +1,5 @@
 // RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 
-analyzer-checker=core,unix.API,osx.API,optin.portability %s 
-analyzer-store=region -analyzer-output=plist -analyzer-config faux-bodies=true 
 -fblocks -verify -o %t.plist
-// RUN: cat %t.plist | %diff_plist %S/Inputs/expected-plists/unix-fns.c.plist -
+// RUN: %normalize_plist <%t.plist | diff -u 
%S/Inputs/expected-plists/unix-fns.c.plist -
 // RUN: mkdir -p %t.dir
 // RUN: %clang_analyze_cc1 
-analyzer-checker=core,unix.API,osx.API,optin.portability -analyzer-output=html 
-analyzer-config faux-bodies=true -fblocks -o %t.dir %s
 // RUN: rm -fR %t.dir
Index: test/Analysis/lit.local.cfg
===
--- test/Analysis/lit.local.cfg
+++ test/Analysis/lit.local.cfg
@@ -14,6 +14,14 @@
 config.substitutions.append(('%diff_plist',
 'diff -u -w -I "/" -I ".:" -I "version"'))
 
+# Filtering command used by Clang Analyzer tests (when comparing .plist files
+# with reference output)
+config.substitutions.append(('%normalize_plist',
+"grep -Ev '%s|%s|%s'" %
+('^[[:space:]]*.* version .*$',
+ '^[[:space:]]*/.*$',
+ '^[[:space:]]*.:.*$')))
+
 # Diff command for testing SARIF output to reference output.
 config.substitutions.append(('%diff_sarif',
 '''diff -U1 -w -I ".*file:.*%basename_t" -I '"version":' -I 
"2\.0\.0\-csd\.[0-9]*\.beta\."'''))


Index: test/Analysis/unix-fns.c
===
--- test/Analysis/unix-fns.c
+++ test/Analysis/unix-fns.c
@@ -1,5 +1,5 @@
 // RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core,unix.API,osx.API,optin.portability %s -analyzer-store=region -analyzer-output=plist -analyzer-config faux-bodies=true  -fblocks -verify -o %t.plist
-// RUN: cat %t.plist | %diff_plist %S/Inputs/expected-plists/unix-fns.c.plist -
+// RUN: %normalize_plist <%t.plist | diff -u %S/Inputs/expected-plists/unix-fns.c.plist -
 // RUN: mkdir -p %t.dir
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.API,osx.API,optin.portability -analyzer-output=html -analyzer-config faux-bodies=true -fblocks -o %t.dir %s
 // RUN: rm -fR %t.dir
Index: test/Analysis/lit.local.cfg
===
--- test/Analysis/lit.local.cfg
+++ test/Analysis/lit.local.cfg
@@ -14,6 +14,14 @@
 config.substitutions.append(('%diff_plist',
 'diff -u -w -I "/" -I ".:" -I "version"'))
 
+# Filtering command used by Clang Analyzer tests (when comparing .plist files
+# with reference output)
+config.substitutions.append(('%normalize_plist',
+"grep -Ev '%s|%s|%s'" %
+('^[[:space:]]*.* version .*$',
+ '^[[:space:]]*/.*$',
+ '^[[:space:]]*.:.*$')))
+
 # Diff command for testing SARIF output to reference output.
 config.substitutions.append(('%diff_sarif',
 '''diff -U1 -w -I ".*file:.*%basename_t" -I '"version":' -I "2\.0\.0\-csd\.[0-9]*\.beta\."'''))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62950: [analyzer][tests] Use normalize_plist in place of diff_plist (`cat` cases)

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast updated this revision to Diff 203707.
hubert.reinterpretcast added a comment.

Update based on review comments, building from rL362877 
 and D63041 


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62950/new/

https://reviews.llvm.org/D62950

Files:
  test/Analysis/NewDelete-path-notes.cpp
  test/Analysis/conditional-path-notes.c
  test/Analysis/copypaste/plist-diagnostics-notes-as-events.cpp
  test/Analysis/copypaste/plist-diagnostics.cpp
  test/Analysis/cxx-for-range.cpp
  test/Analysis/diagnostics/deref-track-symbolic-region.c
  test/Analysis/diagnostics/report-issues-within-main-file.cpp
  test/Analysis/diagnostics/undef-value-caller.c
  test/Analysis/diagnostics/undef-value-param.c
  test/Analysis/diagnostics/undef-value-param.m
  test/Analysis/edges-new.mm
  test/Analysis/generics.m
  test/Analysis/inline-plist.c
  test/Analysis/inline-unique-reports.c
  test/Analysis/inlining/eager-reclamation-path-notes.c
  test/Analysis/inlining/eager-reclamation-path-notes.cpp
  test/Analysis/inlining/path-notes.c
  test/Analysis/inlining/path-notes.cpp
  test/Analysis/inlining/path-notes.m
  test/Analysis/method-call-path-notes.cpp
  test/Analysis/model-file.cpp
  test/Analysis/null-deref-path-notes.m
  test/Analysis/nullability-notes.m
  test/Analysis/objc-arc.m
  test/Analysis/objc-radar17039661.m
  test/Analysis/plist-macros-with-expansion.cpp
  test/Analysis/plist-macros.cpp
  test/Analysis/plist-output-alternate.m
  test/Analysis/plist-output.m
  test/Analysis/retain-release-path-notes.m
  test/Analysis/retain-release.m

Index: test/Analysis/retain-release.m
===
--- test/Analysis/retain-release.m
+++ test/Analysis/retain-release.m
@@ -17,8 +17,8 @@
 // RUN: -Wno-objc-root-class -x objective-c++ -std=gnu++98\
 // RUN: -analyzer-config osx.cocoa.RetainCount:TrackNSCFStartParam=true\
 // RUN: -DTRACK_START_PARAM
-// RUN: cat %t.objcpp.plist | %diff_plist %S/Inputs/expected-plists/retain-release.m.objcpp.plist -
-// RUN: cat %t.objc.plist | %diff_plist %S/Inputs/expected-plists/retain-release.m.objc.plist -
+// RUN: %normalize_plist <%t.objcpp.plist | diff -u %S/Inputs/expected-plists/retain-release.m.objcpp.plist -
+// RUN: %normalize_plist <%t.objc.plist | diff -u %S/Inputs/expected-plists/retain-release.m.objc.plist -
 
 void clang_analyzer_eval(int);
 
Index: test/Analysis/retain-release-path-notes.m
===
--- test/Analysis/retain-release-path-notes.m
+++ test/Analysis/retain-release-path-notes.m
@@ -1,6 +1,6 @@
 // RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core,osx.coreFoundation.CFRetainRelease,osx.cocoa.ClassRelease,osx.cocoa.RetainCount -analyzer-store=region -analyzer-output=text -verify %s
 // RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core,osx.coreFoundation.CFRetainRelease,osx.cocoa.ClassRelease,osx.cocoa.RetainCount -analyzer-store=region -analyzer-output=plist-multi-file %s -o %t
-// RUN: cat %t | %diff_plist %S/Inputs/expected-plists/retain-release-path-notes.m.plist -
+// RUN: %normalize_plist <%t | diff -u %S/Inputs/expected-plists/retain-release-path-notes.m.plist -
 
 /***
 This file is for testing the path-sensitive notes for retain/release errors.
Index: test/Analysis/plist-output.m
===
--- test/Analysis/plist-output.m
+++ test/Analysis/plist-output.m
@@ -1,5 +1,5 @@
 // RUN: %clang_analyze_cc1 -analyzer-config eagerly-assume=false %s -analyzer-checker=osx.cocoa.RetainCount,deadcode.DeadStores,core -analyzer-output=plist -o %t.plist
-// RUN: cat %t.plist | %diff_plist %S/Inputs/expected-plists/plist-output.m.plist -
+// RUN: %normalize_plist <%t.plist | diff -u %S/Inputs/expected-plists/plist-output.m.plist -
 
 void test_null_init(void) {
   int *p = 0;
Index: test/Analysis/plist-output-alternate.m
===
--- test/Analysis/plist-output-alternate.m
+++ test/Analysis/plist-output-alternate.m
@@ -1,5 +1,5 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,osx.cocoa.RetainCount,alpha.core -fblocks -analyzer-output=plist -o %t %s
-// RUN: cat %t | %diff_plist %S/Inputs/expected-plists/plist-output-alternate.m.plist -
+// RUN: %normalize_plist <%t | diff -u %S/Inputs/expected-plists/plist-output-alternate.m.plist -
 
 void test_null_init(void) {
   int *p = 0;
Index: test/Analysis/plist-macros.cpp
===
--- test/Analysis/plist-macros.cpp
+++ test/Analysis/plist-macros.cpp
@@ -1,6 +1,6 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -verify %s
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -analyzer-output=plist-multi-file %s -o %t.plist
-// RUN: cat %t.plist | %diff_plist %S/I

[PATCH] D62951: [analyzer][tests] Use normalize_plist in place of diff_plist (`tail` cases)

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast updated this revision to Diff 203708.
hubert.reinterpretcast added a comment.

Update based on review comments, building from rL362877 
 and D63041 


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62951/new/

https://reviews.llvm.org/D62951

Files:
  test/Analysis/MismatchedDeallocator-path-notes.cpp
  test/Analysis/diagnostics/plist-diagnostics-include-check.cpp
  test/Analysis/diagnostics/plist-multi-file.c
  test/Analysis/lambda-notes.cpp
  test/Analysis/lit.local.cfg
  test/Analysis/malloc-plist.c


Index: test/Analysis/malloc-plist.c
===
--- test/Analysis/malloc-plist.c
+++ test/Analysis/malloc-plist.c
@@ -1,6 +1,6 @@
 // RUN: rm -f %t
 // RUN: %clang_analyze_cc1 -fblocks -analyzer-checker=core,unix.Malloc 
-analyzer-output=plist -verify -o %t -analyzer-config eagerly-assume=false %s
-// RUN: tail -n +11 %t | %diff_plist 
%S/Inputs/expected-plists/malloc-plist.c.plist -
+// RUN: tail -n +11 %t | %normalize_plist | diff -u 
%S/Inputs/expected-plists/malloc-plist.c.plist -
 
 typedef __typeof(sizeof(int)) size_t;
 void *malloc(size_t);
Index: test/Analysis/lit.local.cfg
===
--- test/Analysis/lit.local.cfg
+++ test/Analysis/lit.local.cfg
@@ -9,10 +9,13 @@
 config.test_format = analyzer_test.AnalyzerTest(
 config.test_format.execute_external, config.use_z3_solver)
 
-# Diff command used by Clang Analyzer tests (when comparing .plist files
+# Filtering command used by Clang Analyzer tests (when comparing .plist files
 # with reference output)
-config.substitutions.append(('%diff_plist',
-'diff -u -w -I "/" -I ".:" -I "version"'))
+config.substitutions.append(('%normalize_plist',
+"grep -Ev '%s|%s|%s'" %
+('^[[:space:]]*.* version .*$',
+ '^[[:space:]]*/.*$',
+ '^[[:space:]]*.:.*$')))
 
 # Diff command for testing SARIF output to reference output.
 config.substitutions.append(('%diff_sarif',
Index: test/Analysis/lambda-notes.cpp
===
--- test/Analysis/lambda-notes.cpp
+++ test/Analysis/lambda-notes.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core.DivideZero 
-analyzer-config inline-lambdas=true -analyzer-output plist -verify %s -o %t
-// RUN: tail -n +11 %t | %diff_plist 
%S/Inputs/expected-plists/lambda-notes.cpp.plist -
+// RUN: tail -n +11 %t | %normalize_plist | diff -u 
%S/Inputs/expected-plists/lambda-notes.cpp.plist -
 
 
 // Diagnostic inside a lambda
Index: test/Analysis/diagnostics/plist-multi-file.c
===
--- test/Analysis/diagnostics/plist-multi-file.c
+++ test/Analysis/diagnostics/plist-multi-file.c
@@ -1,5 +1,5 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core 
-analyzer-output=plist-multi-file -o %t.plist -verify %s
-// RUN: tail -n +11 %t.plist | %diff_plist 
%S/Inputs/expected-plists/plist-multi-file.c.plist -
+// RUN: tail -n +11 %t.plist | %normalize_plist | diff -u 
%S/Inputs/expected-plists/plist-multi-file.c.plist -
 
 #include "plist-multi-file.h"
 
Index: test/Analysis/diagnostics/plist-diagnostics-include-check.cpp
===
--- test/Analysis/diagnostics/plist-diagnostics-include-check.cpp
+++ test/Analysis/diagnostics/plist-diagnostics-include-check.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection 
-analyzer-output=plist-multi-file %s -o %t.plist
-// RUN: tail -n +11 %t.plist | %diff_plist 
%S/Inputs/expected-plists/plist-diagnostics-include-check.cpp.plist -
+// RUN: tail -n +11 %t.plist | %normalize_plist | diff -u 
%S/Inputs/expected-plists/plist-diagnostics-include-check.cpp.plist -
 
 #include "Inputs/include/plist-diagnostics-include-check-macro.h"
 
Index: test/Analysis/MismatchedDeallocator-path-notes.cpp
===
--- test/Analysis/MismatchedDeallocator-path-notes.cpp
+++ test/Analysis/MismatchedDeallocator-path-notes.cpp
@@ -1,6 +1,6 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.MismatchedDeallocator 
-analyzer-output=text -verify %s
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.MismatchedDeallocator 
-analyzer-output=plist %s -o %t.plist
-// RUN: tail -n +11 %t.plist | %diff_plist 
%S/copypaste/Inputs/expected-plists/MismatchedDeallocator-path-notes.cpp.plist -
+// RUN: tail -n +11 %t.plist | %normalize_plist | diff -u 
%S/copypaste/Inputs/expected-plists/MismatchedDeallocator-path-notes.cpp.plist -
 
 void changePointee(int *p);
 int *allocIntArray(unsigned c) {


Index: test/Analysis/malloc-plist.c
===
--- test/Analysis/malloc-plist.c
+++ test/Analysis/malloc-plist.c
@@ -1,6 +1,6 @@
 // R

[PATCH] D62949: [analyzer][tests] Add normalize_plist to replace diff_plist

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

@NoQ, I've updated the patches based on the changes you suggested. This patch, 
D62950 , and D62951 
 now looks much cleaner. These are now 
dependent on the approval of D63041 .


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62949/new/

https://reviews.llvm.org/D62949



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

[PATCH] D63029: [CUDA] Fix grep pattern in cuda-types.cu

[Hubert Tong via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
hubert.reinterpretcast marked an inline comment as done.
Closed by commit rL362991: [CUDA] Fix grep pattern in cuda-types.cu (authored 
by hubert.reinterpretcast, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63029/new/

https://reviews.llvm.org/D63029

Files:
  cfe/trunk/test/Preprocessor/cuda-types.cu


Index: cfe/trunk/test/Preprocessor/cuda-types.cu
===
--- cfe/trunk/test/Preprocessor/cuda-types.cu
+++ cfe/trunk/test/Preprocessor/cuda-types.cu
@@ -8,41 +8,41 @@
 // RUN: mkdir -p %t
 
 // RUN: %clang --cuda-host-only -nocudainc -target i386-unknown-linux-gnu -x 
cuda -E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > %t/i386-host-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/i386-host-defines-filtered
 // RUN: %clang --cuda-device-only -nocudainc -nocudalib -target 
i386-unknown-linux-gnu -x cuda -E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > %t/i386-device-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/i386-device-defines-filtered
 // RUN: diff %t/i386-host-defines-filtered %t/i386-device-defines-filtered
 
 // RUN: %clang --cuda-host-only -nocudainc -target x86_64-unknown-linux-gnu -x 
cuda -E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > %t/x86_64-host-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/x86_64-host-defines-filtered
 // RUN: %clang --cuda-device-only -nocudainc -nocudalib -target 
x86_64-unknown-linux-gnu -x cuda -E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > %t/x86_64-device-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/x86_64-device-defines-filtered
 // RUN: diff %t/x86_64-host-defines-filtered %t/x86_64-device-defines-filtered
 
 // RUN: %clang --cuda-host-only -nocudainc -target powerpc64-unknown-linux-gnu 
-x cuda -E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > %t/powerpc64-host-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/powerpc64-host-defines-filtered
 // RUN: %clang --cuda-device-only -nocudainc -nocudalib -target 
powerpc64-unknown-linux-gnu -x cuda -E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > 
%t/powerpc64-device-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > 
%t/powerpc64-device-defines-filtered
 // RUN: diff %t/powerpc64-host-defines-filtered 
%t/powerpc64-device-defines-filtered
 
 // RUN: %clang --cuda-host-only -nocudainc -target i386-windows-msvc -x cuda 
-E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > %t/i386-msvc-host-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/i386-msvc-host-defines-filtered
 // RUN: %clang --cuda-device-only -nocudainc -nocudalib -target 
i386-windows-msvc -x cuda -E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > 
%t/i386-msvc-device-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > 
%t/i386-msvc-device-defines-filtered
 // RUN: diff %t/i386-msvc-host-defines-filtered 
%t/i386-msvc-device-defines-filtered
 
 // RUN: %clang --cuda-host-only -nocudainc -target x86_64-windows-msvc -x cuda 
-E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > 
%t/x86_64-msvc-host-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GC

[PATCH] D63041: [PlistSupport] Produce a newline to end plist output files

[Hubert Tong via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL362992: [PlistSupport] Produce a newline to end plist output 
files (authored by hubert.reinterpretcast, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63041/new/

https://reviews.llvm.org/D63041

Files:
  cfe/trunk/lib/ARCMigrate/PlistReporter.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp


Index: cfe/trunk/lib/ARCMigrate/PlistReporter.cpp
===
--- cfe/trunk/lib/ARCMigrate/PlistReporter.cpp
+++ cfe/trunk/lib/ARCMigrate/PlistReporter.cpp
@@ -120,5 +120,5 @@
   o << " \n";
 
   // Finish.
-  o << "\n";
+  o << "\n\n";
 }
Index: cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
@@ -748,7 +748,7 @@
   }
 
   // Finish.
-  o << "\n";
+  o << "\n\n";
 }
 
 
//===--===//


Index: cfe/trunk/lib/ARCMigrate/PlistReporter.cpp
===
--- cfe/trunk/lib/ARCMigrate/PlistReporter.cpp
+++ cfe/trunk/lib/ARCMigrate/PlistReporter.cpp
@@ -120,5 +120,5 @@
   o << " \n";
 
   // Finish.
-  o << "\n";
+  o << "\n\n";
 }
Index: cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
@@ -748,7 +748,7 @@
   }
 
   // Finish.
-  o << "\n";
+  o << "\n\n";
 }
 
 //===--===//
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62949: [analyzer][tests] Add normalize_plist to replace diff_plist

[Hubert Tong via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL362994: [analyzer][tests] Add normalize_plist to replace 
diff_plist (authored by hubert.reinterpretcast, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62949/new/

https://reviews.llvm.org/D62949

Files:
  cfe/trunk/test/Analysis/lit.local.cfg
  cfe/trunk/test/Analysis/unix-fns.c


Index: cfe/trunk/test/Analysis/unix-fns.c
===
--- cfe/trunk/test/Analysis/unix-fns.c
+++ cfe/trunk/test/Analysis/unix-fns.c
@@ -1,5 +1,5 @@
 // RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 
-analyzer-checker=core,unix.API,osx.API,optin.portability %s 
-analyzer-store=region -analyzer-output=plist -analyzer-config faux-bodies=true 
 -fblocks -verify -o %t.plist
-// RUN: cat %t.plist | %diff_plist %S/Inputs/expected-plists/unix-fns.c.plist -
+// RUN: %normalize_plist <%t.plist | diff -u 
%S/Inputs/expected-plists/unix-fns.c.plist -
 // RUN: mkdir -p %t.dir
 // RUN: %clang_analyze_cc1 
-analyzer-checker=core,unix.API,osx.API,optin.portability -analyzer-output=html 
-analyzer-config faux-bodies=true -fblocks -o %t.dir %s
 // RUN: rm -fR %t.dir
Index: cfe/trunk/test/Analysis/lit.local.cfg
===
--- cfe/trunk/test/Analysis/lit.local.cfg
+++ cfe/trunk/test/Analysis/lit.local.cfg
@@ -14,6 +14,14 @@
 config.substitutions.append(('%diff_plist',
 'diff -u -w -I "/" -I ".:" -I "version"'))
 
+# Filtering command used by Clang Analyzer tests (when comparing .plist files
+# with reference output)
+config.substitutions.append(('%normalize_plist',
+"grep -Ev '%s|%s|%s'" %
+('^[[:space:]]*.* version .*$',
+ '^[[:space:]]*/.*$',
+ '^[[:space:]]*.:.*$')))
+
 # Diff command for testing SARIF output to reference output.
 config.substitutions.append(('%diff_sarif',
 '''diff -U1 -w -I ".*file:.*%basename_t" -I '"version":' -I 
"2\.0\.0\-csd\.[0-9]*\.beta\."'''))


Index: cfe/trunk/test/Analysis/unix-fns.c
===
--- cfe/trunk/test/Analysis/unix-fns.c
+++ cfe/trunk/test/Analysis/unix-fns.c
@@ -1,5 +1,5 @@
 // RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core,unix.API,osx.API,optin.portability %s -analyzer-store=region -analyzer-output=plist -analyzer-config faux-bodies=true  -fblocks -verify -o %t.plist
-// RUN: cat %t.plist | %diff_plist %S/Inputs/expected-plists/unix-fns.c.plist -
+// RUN: %normalize_plist <%t.plist | diff -u %S/Inputs/expected-plists/unix-fns.c.plist -
 // RUN: mkdir -p %t.dir
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.API,osx.API,optin.portability -analyzer-output=html -analyzer-config faux-bodies=true -fblocks -o %t.dir %s
 // RUN: rm -fR %t.dir
Index: cfe/trunk/test/Analysis/lit.local.cfg
===
--- cfe/trunk/test/Analysis/lit.local.cfg
+++ cfe/trunk/test/Analysis/lit.local.cfg
@@ -14,6 +14,14 @@
 config.substitutions.append(('%diff_plist',
 'diff -u -w -I "/" -I ".:" -I "version"'))
 
+# Filtering command used by Clang Analyzer tests (when comparing .plist files
+# with reference output)
+config.substitutions.append(('%normalize_plist',
+"grep -Ev '%s|%s|%s'" %
+('^[[:space:]]*.* version .*$',
+ '^[[:space:]]*/.*$',
+ '^[[:space:]]*.:.*$')))
+
 # Diff command for testing SARIF output to reference output.
 config.substitutions.append(('%diff_sarif',
 '''diff -U1 -w -I ".*file:.*%basename_t" -I '"version":' -I "2\.0\.0\-csd\.[0-9]*\.beta\."'''))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62950: [analyzer][tests] Use normalize_plist in place of diff_plist (`cat` cases)

[Hubert Tong via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL362996: [analyzer][tests] Use normalize_plist in place of 
diff_plist (`cat` cases) (authored by hubert.reinterpretcast, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62950/new/

https://reviews.llvm.org/D62950

Files:
  cfe/trunk/test/Analysis/NewDelete-path-notes.cpp
  cfe/trunk/test/Analysis/conditional-path-notes.c
  cfe/trunk/test/Analysis/copypaste/plist-diagnostics-notes-as-events.cpp
  cfe/trunk/test/Analysis/copypaste/plist-diagnostics.cpp
  cfe/trunk/test/Analysis/cxx-for-range.cpp
  cfe/trunk/test/Analysis/diagnostics/deref-track-symbolic-region.c
  cfe/trunk/test/Analysis/diagnostics/report-issues-within-main-file.cpp
  cfe/trunk/test/Analysis/diagnostics/undef-value-caller.c
  cfe/trunk/test/Analysis/diagnostics/undef-value-param.c
  cfe/trunk/test/Analysis/diagnostics/undef-value-param.m
  cfe/trunk/test/Analysis/edges-new.mm
  cfe/trunk/test/Analysis/generics.m
  cfe/trunk/test/Analysis/inline-plist.c
  cfe/trunk/test/Analysis/inline-unique-reports.c
  cfe/trunk/test/Analysis/inlining/eager-reclamation-path-notes.c
  cfe/trunk/test/Analysis/inlining/eager-reclamation-path-notes.cpp
  cfe/trunk/test/Analysis/inlining/path-notes.c
  cfe/trunk/test/Analysis/inlining/path-notes.cpp
  cfe/trunk/test/Analysis/inlining/path-notes.m
  cfe/trunk/test/Analysis/method-call-path-notes.cpp
  cfe/trunk/test/Analysis/model-file.cpp
  cfe/trunk/test/Analysis/null-deref-path-notes.m
  cfe/trunk/test/Analysis/nullability-notes.m
  cfe/trunk/test/Analysis/objc-arc.m
  cfe/trunk/test/Analysis/objc-radar17039661.m
  cfe/trunk/test/Analysis/plist-macros-with-expansion.cpp
  cfe/trunk/test/Analysis/plist-macros.cpp
  cfe/trunk/test/Analysis/plist-output-alternate.m
  cfe/trunk/test/Analysis/plist-output.m
  cfe/trunk/test/Analysis/retain-release-path-notes.m
  cfe/trunk/test/Analysis/retain-release.m

Index: cfe/trunk/test/Analysis/retain-release-path-notes.m
===
--- cfe/trunk/test/Analysis/retain-release-path-notes.m
+++ cfe/trunk/test/Analysis/retain-release-path-notes.m
@@ -1,6 +1,6 @@
 // RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core,osx.coreFoundation.CFRetainRelease,osx.cocoa.ClassRelease,osx.cocoa.RetainCount -analyzer-store=region -analyzer-output=text -verify %s
 // RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core,osx.coreFoundation.CFRetainRelease,osx.cocoa.ClassRelease,osx.cocoa.RetainCount -analyzer-store=region -analyzer-output=plist-multi-file %s -o %t
-// RUN: cat %t | %diff_plist %S/Inputs/expected-plists/retain-release-path-notes.m.plist -
+// RUN: %normalize_plist <%t | diff -u %S/Inputs/expected-plists/retain-release-path-notes.m.plist -
 
 /***
 This file is for testing the path-sensitive notes for retain/release errors.
Index: cfe/trunk/test/Analysis/null-deref-path-notes.m
===
--- cfe/trunk/test/Analysis/null-deref-path-notes.m
+++ cfe/trunk/test/Analysis/null-deref-path-notes.m
@@ -1,6 +1,6 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-store=region -analyzer-output=text -fblocks -verify -Wno-objc-root-class %s
 // RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-store=region -analyzer-output=plist-multi-file -fblocks -Wno-objc-root-class %s -o %t
-// RUN: cat %t | %diff_plist %S/Inputs/expected-plists/null-deref-path-notes.m.plist -
+// RUN: %normalize_plist <%t | diff -u %S/Inputs/expected-plists/null-deref-path-notes.m.plist -
 
 @interface Root {
 @public
Index: cfe/trunk/test/Analysis/conditional-path-notes.c
===
--- cfe/trunk/test/Analysis/conditional-path-notes.c
+++ cfe/trunk/test/Analysis/conditional-path-notes.c
@@ -1,6 +1,6 @@
 // RUN: %clang_analyze_cc1 %s -analyzer-checker=core.NullDereference -analyzer-output=text -verify
 // RUN: %clang_analyze_cc1 %s -analyzer-checker=core.NullDereference -analyzer-output=plist -o %t
-// RUN: cat %t | %diff_plist %S/Inputs/expected-plists/conditional-path-notes.c.plist -
+// RUN: %normalize_plist <%t | diff -u %S/Inputs/expected-plists/conditional-path-notes.c.plist -
 
 void testCondOp(int *p) {
   int *x = p ? p : p;
Index: cfe/trunk/test/Analysis/plist-output.m
===
--- cfe/trunk/test/Analysis/plist-output.m
+++ cfe/trunk/test/Analysis/plist-output.m
@@ -1,5 +1,5 @@
 // RUN: %clang_analyze_cc1 -analyzer-config eagerly-assume=false %s -analyzer-checker=osx.cocoa.RetainCount,deadcode.DeadStores,core -analyzer-output=plist -o %t.plist
-// RUN: cat %t.plist | %diff_plist %S/Inputs/expected-plists/plist-output.m.plist -
+/

[PATCH] D62951: [analyzer][tests] Use normalize_plist in place of diff_plist (`tail` cases)

[Hubert Tong via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL362998: [analyzer][tests] Use normalize_plist in place of 
diff_plist (`tail` cases) (authored by hubert.reinterpretcast, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D62951?vs=203708&id=203916#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62951/new/

https://reviews.llvm.org/D62951

Files:
  cfe/trunk/test/Analysis/MismatchedDeallocator-path-notes.cpp
  cfe/trunk/test/Analysis/diagnostics/plist-diagnostics-include-check.cpp
  cfe/trunk/test/Analysis/diagnostics/plist-multi-file.c
  cfe/trunk/test/Analysis/lambda-notes.cpp
  cfe/trunk/test/Analysis/lit.local.cfg
  cfe/trunk/test/Analysis/malloc-plist.c


Index: cfe/trunk/test/Analysis/diagnostics/plist-multi-file.c
===
--- cfe/trunk/test/Analysis/diagnostics/plist-multi-file.c
+++ cfe/trunk/test/Analysis/diagnostics/plist-multi-file.c
@@ -1,5 +1,5 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core 
-analyzer-output=plist-multi-file -o %t.plist -verify %s
-// RUN: tail -n +11 %t.plist | %diff_plist 
%S/Inputs/expected-plists/plist-multi-file.c.plist -
+// RUN: tail -n +11 %t.plist | %normalize_plist | diff -u 
%S/Inputs/expected-plists/plist-multi-file.c.plist -
 
 #include "plist-multi-file.h"
 
Index: cfe/trunk/test/Analysis/diagnostics/plist-diagnostics-include-check.cpp
===
--- cfe/trunk/test/Analysis/diagnostics/plist-diagnostics-include-check.cpp
+++ cfe/trunk/test/Analysis/diagnostics/plist-diagnostics-include-check.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection 
-analyzer-output=plist-multi-file %s -o %t.plist
-// RUN: tail -n +11 %t.plist | %diff_plist 
%S/Inputs/expected-plists/plist-diagnostics-include-check.cpp.plist -
+// RUN: tail -n +11 %t.plist | %normalize_plist | diff -u 
%S/Inputs/expected-plists/plist-diagnostics-include-check.cpp.plist -
 
 #include "Inputs/include/plist-diagnostics-include-check-macro.h"
 
Index: cfe/trunk/test/Analysis/malloc-plist.c
===
--- cfe/trunk/test/Analysis/malloc-plist.c
+++ cfe/trunk/test/Analysis/malloc-plist.c
@@ -1,6 +1,6 @@
 // RUN: rm -f %t
 // RUN: %clang_analyze_cc1 -fblocks -analyzer-checker=core,unix.Malloc 
-analyzer-output=plist -verify -o %t -analyzer-config eagerly-assume=false %s
-// RUN: tail -n +11 %t | %diff_plist 
%S/Inputs/expected-plists/malloc-plist.c.plist -
+// RUN: tail -n +11 %t | %normalize_plist | diff -u 
%S/Inputs/expected-plists/malloc-plist.c.plist -
 
 typedef __typeof(sizeof(int)) size_t;
 void *malloc(size_t);
Index: cfe/trunk/test/Analysis/lit.local.cfg
===
--- cfe/trunk/test/Analysis/lit.local.cfg
+++ cfe/trunk/test/Analysis/lit.local.cfg
@@ -9,11 +9,6 @@
 config.test_format = analyzer_test.AnalyzerTest(
 config.test_format.execute_external, config.use_z3_solver)
 
-# Diff command used by Clang Analyzer tests (when comparing .plist files
-# with reference output)
-config.substitutions.append(('%diff_plist',
-'diff -u -w -I "/" -I ".:" -I "version"'))
-
 # Filtering command used by Clang Analyzer tests (when comparing .plist files
 # with reference output)
 config.substitutions.append(('%normalize_plist',
Index: cfe/trunk/test/Analysis/lambda-notes.cpp
===
--- cfe/trunk/test/Analysis/lambda-notes.cpp
+++ cfe/trunk/test/Analysis/lambda-notes.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core.DivideZero 
-analyzer-config inline-lambdas=true -analyzer-output plist -verify %s -o %t
-// RUN: tail -n +11 %t | %diff_plist 
%S/Inputs/expected-plists/lambda-notes.cpp.plist -
+// RUN: tail -n +11 %t | %normalize_plist | diff -u 
%S/Inputs/expected-plists/lambda-notes.cpp.plist -
 
 
 // Diagnostic inside a lambda
Index: cfe/trunk/test/Analysis/MismatchedDeallocator-path-notes.cpp
===
--- cfe/trunk/test/Analysis/MismatchedDeallocator-path-notes.cpp
+++ cfe/trunk/test/Analysis/MismatchedDeallocator-path-notes.cpp
@@ -1,6 +1,6 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.MismatchedDeallocator 
-analyzer-output=text -verify %s
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.MismatchedDeallocator 
-analyzer-output=plist %s -o %t.plist
-// RUN: tail -n +11 %t.plist | %diff_plist 
%S/copypaste/Inputs/expected-plists/MismatchedDeallocator-path-notes.cpp.plist -
+// RUN: tail -n +11 %t.plist | %normalize_plist | diff -u 
%S/copypaste/Inputs/expected-plists/MismatchedDeallocator-path-notes.cpp.plist -
 
 void 

[PATCH] D62952: [analyzer][tests] Use normalize_sarif in place of diff_sarif

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

Ping.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62952/new/

https://reviews.llvm.org/D62952



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

[PATCH] D62952: [analyzer][tests] Use normalize_sarif in place of diff_sarif

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

In D62952#1548377 , @aaron.ballman 
wrote:

> In general, that seems reasonable, but I would prefer to take care of more of 
> the work in lit.local.cfg than have to deal with that atrocious RUN line in 
> every test case. Is there a way to retain a similarly succinct solution as 
> diff_sarif?


There'd be no atrocious RUN line if we went with modifying the expected files 
beforehand and having the tool output a newline.

The unchanged:

  // RUN: %clang_analyze_cc1 
-analyzer-checker=alpha.security.taint,debug.TaintTest %s -verify 
-analyzer-output=sarif -o - | %diff_sarif 
%S/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif -

becomes:

  // RUN: %clang_analyze_cc1 
-analyzer-checker=alpha.security.taint,debug.TaintTest %s -verify 
-analyzer-output=sarif -o - | %normalize_sarif | diff -U1 -b 
%S/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif -

As in, `%diff_sarif` gets replaced with `%normalize_sarif | diff -U1 -b` and 
that's it.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62952/new/

https://reviews.llvm.org/D62952



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

[PATCH] D62952: [analyzer][tests] Use normalize_sarif in place of diff_sarif

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

In D62952#1548593 , @aaron.ballman 
wrote:

> But is there a reason to not keep `%diff_sarif` and define it in terms of 
> `%normalize_sarif | diff -U1 -b` within lit.local.cfg? I guess I don't see 
> the benefit to exposing the call to diff (I don't anticipate anyone needing 
> to change the options passed to diff).


Yes: The normalization runs on stdin. `%diff_sarif` would then only make sense 
if stdin is one of the inputs, and I think the only input we can hardcode to be 
stdin is the one conventionally considered to be the reference input (which is 
not what we want).


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62952/new/

https://reviews.llvm.org/D62952



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

[PATCH] D59253: [AIX][libcxx] AIX system headers need stdint.h and inttypes.h to be re-enterable when macro _STD_TYPES_T is defined

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast accepted this revision.
hubert.reinterpretcast added a comment.

LGTM.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59253/new/

https://reviews.llvm.org/D59253



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

[PATCH] D62952: [analyzer][tests] Use normalize_sarif in place of diff_sarif

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast updated this revision to Diff 205590.
hubert.reinterpretcast added a comment.

Update based on review comments from D62949  
as confirmed in D62952 

Normalized versions of the reference expected SARIF output files were
checked in under r363788. The patch has been updated with that revision
as the base. Made the SARIF output generation produce a newline at the
end of the file, and modified the RUN lines in the manner discussed.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62952/new/

https://reviews.llvm.org/D62952

Files:
  lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
  
test/Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif
  
test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
  test/Analysis/diagnostics/sarif-diagnostics-taint-test.c
  test/Analysis/diagnostics/sarif-multi-diagnostic-test.c
  test/Analysis/lit.local.cfg


Index: test/Analysis/lit.local.cfg
===
--- test/Analysis/lit.local.cfg
+++ test/Analysis/lit.local.cfg
@@ -17,9 +17,12 @@
  '^[[:space:]]*/.*[[:space:]]*$',
  '^[[:space:]]*.:.*[[:space:]]*$')))
 
-# Diff command for testing SARIF output to reference output.
-config.substitutions.append(('%diff_sarif',
-'''diff -U1 -w -I ".*file:.*%basename_t" -I '"version":' -I 
"2\.0\.0\-csd\.[0-9]*\.beta\."'''))
+# Filtering command for testing SARIF output against reference output.
+config.substitutions.append(('%normalize_sarif',
+"grep -Ev '^[[:space:]]*(%s|%s|%s)[[:space:]]*$'" %
+('"uri": "file:.*%basename_t"',
+ '"version": ".* version .*"',
+ '"version": "2\.0\.0-csd\.[0-9]*\.beta\.[0-9-]{10}"')))
 
 if not config.root.clang_staticanalyzer:
 config.unsupported = True
Index: test/Analysis/diagnostics/sarif-multi-diagnostic-test.c
===
--- test/Analysis/diagnostics/sarif-multi-diagnostic-test.c
+++ test/Analysis/diagnostics/sarif-multi-diagnostic-test.c
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 
-analyzer-checker=core,alpha.security.taint,debug.TaintTest %s -verify 
-analyzer-output=sarif -o - | %diff_sarif 
%S/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif -
+// RUN: %clang_analyze_cc1 
-analyzer-checker=core,alpha.security.taint,debug.TaintTest %s -verify 
-analyzer-output=sarif -o - | %normalize_sarif | diff -U1 -b 
%S/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif -
 #include "../Inputs/system-header-simulator.h"
 
 int atoi(const char *nptr);
Index: test/Analysis/diagnostics/sarif-diagnostics-taint-test.c
===
--- test/Analysis/diagnostics/sarif-diagnostics-taint-test.c
+++ test/Analysis/diagnostics/sarif-diagnostics-taint-test.c
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 
-analyzer-checker=alpha.security.taint,debug.TaintTest %s -verify 
-analyzer-output=sarif -o - | %diff_sarif 
%S/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif -
+// RUN: %clang_analyze_cc1 
-analyzer-checker=alpha.security.taint,debug.TaintTest %s -verify 
-analyzer-output=sarif -o - | %normalize_sarif | diff -U1 -b 
%S/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif -
 #include "../Inputs/system-header-simulator.h"
 
 int atoi(const char *nptr);
Index: 
test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
===
--- 
test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
+++ 
test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
@@ -6,7 +6,7 @@
 {
   "fileLocation": {
   },
-  "length": 667,
+  "length": 686,
   "mimeType": "text/plain",
   "roles": [
 "resultFile"
Index: 
test/Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif
===
--- 
test/Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif
+++ 
test/Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif
@@ -6,7 +6,7 @@
 {
   "fileLocation": {
   },
-  "length": 415,
+  "length": 434,
   "mimeType": "text/plain",
   "roles": [
 "resultFile"
Index: lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
===
--- lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
+++ lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
@@ -345,5 +345,5 @@
"http://json.schemastore.org/sarif-2.0.0-csd.2.beta.2018-11-28"},
   {"version", "2.0.0-csd.2.beta.2018-11-28"},
   {"runs", json::Array{createRun(Diags)}}};
-  OS << llvm::formatv("{0:2}"

[PATCH] D62952: [analyzer] SARIF: Add EOF newline; replace diff_sarif

[Hubert Tong via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL363822: [analyzer] SARIF: Add EOF newline; replace 
diff_sarif (authored by hubert.reinterpretcast, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62952/new/

https://reviews.llvm.org/D62952

Files:
  cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
  
cfe/trunk/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif
  
cfe/trunk/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
  cfe/trunk/test/Analysis/diagnostics/sarif-diagnostics-taint-test.c
  cfe/trunk/test/Analysis/diagnostics/sarif-multi-diagnostic-test.c
  cfe/trunk/test/Analysis/lit.local.cfg


Index: cfe/trunk/test/Analysis/diagnostics/sarif-diagnostics-taint-test.c
===
--- cfe/trunk/test/Analysis/diagnostics/sarif-diagnostics-taint-test.c
+++ cfe/trunk/test/Analysis/diagnostics/sarif-diagnostics-taint-test.c
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 
-analyzer-checker=alpha.security.taint,debug.TaintTest %s -verify 
-analyzer-output=sarif -o - | %diff_sarif 
%S/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif -
+// RUN: %clang_analyze_cc1 
-analyzer-checker=alpha.security.taint,debug.TaintTest %s -verify 
-analyzer-output=sarif -o - | %normalize_sarif | diff -U1 -b 
%S/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif -
 #include "../Inputs/system-header-simulator.h"
 
 int atoi(const char *nptr);
Index: 
cfe/trunk/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
===
--- 
cfe/trunk/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
+++ 
cfe/trunk/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
@@ -6,7 +6,7 @@
 {
   "fileLocation": {
   },
-  "length": 667,
+  "length": 686,
   "mimeType": "text/plain",
   "roles": [
 "resultFile"
Index: 
cfe/trunk/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif
===
--- 
cfe/trunk/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif
+++ 
cfe/trunk/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif
@@ -6,7 +6,7 @@
 {
   "fileLocation": {
   },
-  "length": 415,
+  "length": 434,
   "mimeType": "text/plain",
   "roles": [
 "resultFile"
Index: cfe/trunk/test/Analysis/diagnostics/sarif-multi-diagnostic-test.c
===
--- cfe/trunk/test/Analysis/diagnostics/sarif-multi-diagnostic-test.c
+++ cfe/trunk/test/Analysis/diagnostics/sarif-multi-diagnostic-test.c
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 
-analyzer-checker=core,alpha.security.taint,debug.TaintTest %s -verify 
-analyzer-output=sarif -o - | %diff_sarif 
%S/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif -
+// RUN: %clang_analyze_cc1 
-analyzer-checker=core,alpha.security.taint,debug.TaintTest %s -verify 
-analyzer-output=sarif -o - | %normalize_sarif | diff -U1 -b 
%S/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif -
 #include "../Inputs/system-header-simulator.h"
 
 int atoi(const char *nptr);
Index: cfe/trunk/test/Analysis/lit.local.cfg
===
--- cfe/trunk/test/Analysis/lit.local.cfg
+++ cfe/trunk/test/Analysis/lit.local.cfg
@@ -17,9 +17,12 @@
  '^[[:space:]]*/.*[[:space:]]*$',
  '^[[:space:]]*.:.*[[:space:]]*$')))
 
-# Diff command for testing SARIF output to reference output.
-config.substitutions.append(('%diff_sarif',
-'''diff -U1 -w -I ".*file:.*%basename_t" -I '"version":' -I 
"2\.0\.0\-csd\.[0-9]*\.beta\."'''))
+# Filtering command for testing SARIF output against reference output.
+config.substitutions.append(('%normalize_sarif',
+"grep -Ev '^[[:space:]]*(%s|%s|%s)[[:space:]]*$'" %
+('"uri": "file:.*%basename_t"',
+ '"version": ".* version .*"',
+ '"version": "2\.0\.0-csd\.[0-9]*\.beta\.[0-9-]{10}"')))
 
 if not config.root.clang_staticanalyzer:
 config.unsupported = True
Index: cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
@@ -345,5 +345,5 @@
"http://json.schemastore.org/sarif-2.0.0-csd.2.beta.2018-11-28"},
   {"version", "2.0.0-csd.2.beta.2018-11-28"},
   {"runs", json::Array{createRun(Diags)}}};
-  OS << llvm::formatv("{0:2}", j