[PATCH] D133375: [CMake] Remove CLANG_DEFAULT_STD_C/CLANG_DEFAULT_STD_CXX

2023-03-22 Thread Raul Tambre via Phabricator via cfe-commits 
tambre added a comment.

I've now run into a snag with this. Having `clang++.cfg` with `-std=c++2b` 
makes `/usr/bin/clang++ -E -dM -` fail with `error: invalid argument 
'-std=c++2b' not allowed with 'C'`.
This is invoked by Meson during its configuration process and causes it to fail.

Am I doing something wrong?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133375

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

[PATCH] D133375: [CMake] Remove CLANG_DEFAULT_STD_C/CLANG_DEFAULT_STD_CXX

2023-01-25 Thread Raul Tambre via Phabricator via cfe-commits 
tambre added a comment.

A Clang 16 release note for this would be nice. I didn't immediately notice and 
got a bit broken by this change. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133375

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

[PATCH] D129362: Undeprecate ATOMIC_FLAG_INIT in C++

2022-07-08 Thread Raul Tambre via Phabricator via cfe-commits 
tambre added a comment.

In D129362#3638896 , @aaron.ballman 
wrote:

> In D129362#3638485 , @tambre wrote:
>
>> Makes sense to me, thanks!
>
> Thanks! Would you mind handling the libc++ side like last time, or do you 
> prefer I handle it as part of the changes in this patch?

Yeah, sure, see D129380 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129362

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

[PATCH] D129362: Undeprecate ATOMIC_FLAG_INIT in C++

2022-07-08 Thread Raul Tambre via Phabricator via cfe-commits 
tambre accepted this revision.
tambre added a comment.
This revision is now accepted and ready to land.

Makes sense to me, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129362

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

[PATCH] D127379: [Lex] Keep track of skipped preprocessor blocks and advance the lexer directly if they are revisited

2022-06-14 Thread Raul Tambre via Phabricator via cfe-commits 
tambre added a comment.

In D127379#3581009 , @tschuett wrote:

> Do you believe an entry in the ReleaseNotes would get this achievement more 
> visibility?

I second that this would be worthy of a release note. Seems like quite a big 
improvement.

I wonder how much is the total speedup for a full LLVM build or such?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127379

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

[PATCH] D122248: [clang][CodeGen]Fix clang crash and add bitfield support in __builtin_dump_struct

2022-03-24 Thread Raul Tambre via Phabricator via cfe-commits 
tambre added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:95-96
+- The builtin function __builtin_dump_struct would crash clang when the target 
+  struct have bitfield. Now it fixed, and __builtin_dump_struct support dump
+  the bitwidth of bitfields.
+  This fixes `Issue 54462 
`_.

Though I'd split the note about supporting dumping of bitfield widths into a 
separate point.



Comment at: clang/docs/ReleaseNotes.rst:144
+  
+  - Improve the dump format, dump both bitwidth(if its a bitfield) and field 
value.
+




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122248

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

[PATCH] D112221: Mark ATOMIC_VAR_INIT and ATOMIC_FLAG_INIT as deprecated

2021-12-15 Thread Raul Tambre via Phabricator via cfe-commits 
tambre added a comment.

In D112221#3195955 , @ldionne wrote:

> @tambre Any appetite for a libc++ patch? :)
>
> Otherwise, let me know and I'll put it in my list of things to fix up.

Sure, I can take it up during the weekend.


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

https://reviews.llvm.org/D112221

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

[PATCH] D112221: Mark ATOMIC_VAR_INIT and ATOMIC_FLAG_INIT as deprecated

2021-12-15 Thread Raul Tambre via Phabricator via cfe-commits 
tambre added a comment.

In D112221#3195506 , @ldionne wrote:

> Sorry, I seemed to have missed this entirely.
>
> Since this is touching only `` and not libc++'s ``, I 
> think it would actually be OK not to make any changes at all inside libc++. 
> Nothing should break. We should separately implement P0883 in libc++ and 
> deprecate the macros that we define in ``. Note that I thought P0883 
> was implemented -- it is definitely marked as such, so we'll have to 
> investigate. CC @tambre who implemented P0883.
>
> So, as far as this patch is concerned, I think we could remove all the libc++ 
> specific stuff and this would LGTM. Thanks for the heads up -- we'll tackle 
> the C++ equivalent of this change in libc++ separately.

I only took care of `std::atomic_init()` and seemingly completely missed 
cleaning up uses of `ATOMIC_VAR_INIT`.
I do remember not being able to add a deprecation notice for `ATOMIC_VAR_INIT` 
at the time as `#pragma deprecated` didn't exist yet.


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

https://reviews.llvm.org/D112221

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

[PATCH] D108482: [Clang] Fix instantiation of OpaqueValueExprs (Bug #45964)

2021-11-06 Thread Raul Tambre 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 rGb5aef90d4656: [Clang] Fix instantiation of OpaqueValueExprs 
(Bug #45964) (authored by ricejasonf, committed by tambre).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108482

Files:
  clang/lib/Sema/TreeTransform.h
  clang/test/CodeGenCXX/pr45964-decomp-transform.cpp


Index: clang/test/CodeGenCXX/pr45964-decomp-transform.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/pr45964-decomp-transform.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -std=c++17 -triple x86_64-linux-gnu -emit-llvm -o - %s | 
FileCheck %s
+
+int a[1];
+// CHECK: @a = global [1 x i32] zeroinitializer
+template 
+void test_transform() {
+  auto [b] = a;
+}
+void (*d)(){test_transform<0>};
+// CHECK-LABEL: define {{.*}} @_Z14test_transformILi0EEvv
+// CHECK:   [[ENTRY:.*]]:
+// CHECK-NEXT:  [[ARR:%.*]] = alloca [1 x i32]
+// CHECK-NEXT:  [[BEGIN:%.*]] = getelementptr inbounds [1 x i32], [1 x i32]* 
[[ARR]], i64 0, i64 0
+// CHECK-NEXT:  br label %[[BODY:.*]]
+// CHECK-EMPTY:
+// CHECK-NEXT:  [[BODY]]:
+// CHECK-NEXT:  [[CUR:%.*]] = phi i64 [ 0, %[[ENTRY]] ], [ [[NEXT:%.*]], 
%[[BODY]] ]
+// CHECK-NEXT:  [[DEST:%.*]] = getelementptr inbounds i32, i32* [[BEGIN]], i64 
[[CUR]]
+// CHECK-NEXT:  [[SRC:%.*]] = getelementptr inbounds [1 x i32], [1 x i32]* @a, 
i64 0, i64 [[CUR]]
+// CHECK-NEXT:  [[X:%.*]] = load i32, i32* [[SRC]]
+// CHECK-NEXT:  store i32 [[X]], i32* [[DEST]]
+// CHECK-NEXT:  [[NEXT]] = add nuw i64 [[CUR]], 1
+// CHECK-NEXT:  [[EQ:%.*]] = icmp eq i64 [[NEXT]], 1
+// CHECK-NEXT:  br i1 [[EQ]], label %[[FIN:.*]], label %[[BODY]]
+// CHECK-EMPTY:
+// CHECK-NEXT:  [[FIN]]:
+// CHECK-NEXT:  ret void
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -3853,8 +3853,10 @@
   if (auto *FE = dyn_cast(Init))
 Init = FE->getSubExpr();
 
-  if (auto *AIL = dyn_cast(Init))
-Init = AIL->getCommonExpr();
+  if (auto *AIL = dyn_cast(Init)) {
+OpaqueValueExpr *OVE = AIL->getCommonExpr();
+Init = OVE->getSourceExpr();
+  }
 
   if (MaterializeTemporaryExpr *MTE = dyn_cast(Init))
 Init = MTE->getSubExpr();


Index: clang/test/CodeGenCXX/pr45964-decomp-transform.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/pr45964-decomp-transform.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -std=c++17 -triple x86_64-linux-gnu -emit-llvm -o - %s | FileCheck %s
+
+int a[1];
+// CHECK: @a = global [1 x i32] zeroinitializer
+template 
+void test_transform() {
+  auto [b] = a;
+}
+void (*d)(){test_transform<0>};
+// CHECK-LABEL: define {{.*}} @_Z14test_transformILi0EEvv
+// CHECK:   [[ENTRY:.*]]:
+// CHECK-NEXT:  [[ARR:%.*]] = alloca [1 x i32]
+// CHECK-NEXT:  [[BEGIN:%.*]] = getelementptr inbounds [1 x i32], [1 x i32]* [[ARR]], i64 0, i64 0
+// CHECK-NEXT:  br label %[[BODY:.*]]
+// CHECK-EMPTY:
+// CHECK-NEXT:  [[BODY]]:
+// CHECK-NEXT:  [[CUR:%.*]] = phi i64 [ 0, %[[ENTRY]] ], [ [[NEXT:%.*]], %[[BODY]] ]
+// CHECK-NEXT:  [[DEST:%.*]] = getelementptr inbounds i32, i32* [[BEGIN]], i64 [[CUR]]
+// CHECK-NEXT:  [[SRC:%.*]] = getelementptr inbounds [1 x i32], [1 x i32]* @a, i64 0, i64 [[CUR]]
+// CHECK-NEXT:  [[X:%.*]] = load i32, i32* [[SRC]]
+// CHECK-NEXT:  store i32 [[X]], i32* [[DEST]]
+// CHECK-NEXT:  [[NEXT]] = add nuw i64 [[CUR]], 1
+// CHECK-NEXT:  [[EQ:%.*]] = icmp eq i64 [[NEXT]], 1
+// CHECK-NEXT:  br i1 [[EQ]], label %[[FIN:.*]], label %[[BODY]]
+// CHECK-EMPTY:
+// CHECK-NEXT:  [[FIN]]:
+// CHECK-NEXT:  ret void
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -3853,8 +3853,10 @@
   if (auto *FE = dyn_cast(Init))
 Init = FE->getSubExpr();
 
-  if (auto *AIL = dyn_cast(Init))
-Init = AIL->getCommonExpr();
+  if (auto *AIL = dyn_cast(Init)) {
+OpaqueValueExpr *OVE = AIL->getCommonExpr();
+Init = OVE->getSourceExpr();
+  }
 
   if (MaterializeTemporaryExpr *MTE = dyn_cast(Init))
 Init = MTE->getSubExpr();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108482: [Clang] Fix instantiation of OpaqueValueExprs (Bug #45964)

2021-11-05 Thread Raul Tambre via Phabricator via cfe-commits 
tambre added a comment.

In D108482#3108013 , @ricejasonf 
wrote:

> In D108482#3105889 , @tambre wrote:
>
>> I presume you lack the permissions to push this to the main repo yourself. 
>> Would you like me to do that for you?
>
> Yes, please.

Please provide the name and email you want it committed as. Unfortunately 
Phabricator diffs don't include this info.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108482

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

[PATCH] D108482: [Clang] Fix instantiation of OpaqueValueExprs (Bug #45964)

2021-11-03 Thread Raul Tambre via Phabricator via cfe-commits 
tambre added a comment.

I presume you lack the permissions to push this to the main repo yourself. 
Would you like me to do that for you?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108482

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

[PATCH] D103395: PR45879: Keep evaluated expression in LValue object

2021-11-03 Thread Raul Tambre via Phabricator via cfe-commits 
tambre added a comment.

In D103395#3105668 , @sepavloff wrote:

> Strange, I see that it cannot be compiled neither by gcc nor by clang: 
> https://godbolt.org/z/1dY9Gs6zM. Do I miss something?

Sorry, should've been more specific. Try in C++20 mode: 
https://godbolt.org/z/4v8b3nsET
I think the difference might be due to P1331R2 
, but I'm 
not sure.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103395

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

[PATCH] D103395: PR45879: Keep evaluated expression in LValue object

2021-10-29 Thread Raul Tambre via Phabricator via cfe-commits 
tambre added a comment.

This breaks the following code:

  struct sub
  {
char data;
  };
  
  struct main
  {
constexpr main()
{
member = {};
}
  
sub member;
  };
  
  constexpr main a{};

With:

  fmt.cpp:16:16: error: constexpr variable 'a' must be initialized by a 
constant expression
  constexpr main a{};
 ^~~
  1 error generated.

Clang trunk and GCC (Debian 11.2.0-10) handle it fine.
Noticed in libfmt 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103395

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

[PATCH] D108482: [Clang] Fix instantiation of OpaqueValueExprs (Bug #45964)

2021-10-05 Thread Raul Tambre via Phabricator via cfe-commits 
tambre added inline comments.



Comment at: clang/test/CodeGenCXX/pr45964-decomp-transform.cpp:1
+// RUN: %clang_cc1 -std=c++1z -triple x86_64-linux-gnu -emit-llvm -o - %s | 
FileCheck %s
+

I'd prefer `-std=c++17` since the standard and flag have been finalized.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108482

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

[PATCH] D108482: [Clang] Fix instantiation of OpaqueValueExprs (Bug #45964)

2021-10-05 Thread Raul Tambre via Phabricator via cfe-commits 
tambre added a comment.

In D108482#3041127 , @ricejasonf 
wrote:

> I rebased onto `main` and made one small style fix. I don't think the rebase 
> changes anything.
>
> I do not understand why random things are failing in the CI. Even on my own 
> build machine with a fresh build directory CMake fails with errors about 
> unregistered files in ExecutionEngine/Orc.
>
> All this stuff is completely unrelated to my very minor change. Is there 
> something about the process that I am missing?

CI looks clean to me except for the Flang failure, which seems to have been 
reverted 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108482

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

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

2021-10-02 Thread Raul Tambre via Phabricator via cfe-commits 
tambre added a comment.

In D77491#3038277 , @ychen wrote:

> In D77491#3038204 , @tambre wrote:
>
>> Abandoning since I don't think there's any further review/actions to be done 
>> here.
>> D58531  would be the solution for the 
>> raised `pthread_create()` issue.
>
> How about making this "Closed" instead of "Abandoned" considering the patch 
> was actually landed?

Good point. I assumed this wasn't possible as the web UI didn't expose an 
option for this.
But I have managed using `arc close-revision`. Needed to reclaim and get it 
into an accepted state to be able to do it though. Sorry for the noise.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77491

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

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

2021-10-02 Thread Raul Tambre via Phabricator via cfe-commits 
tambre abandoned this revision.
tambre added a comment.

Abandoning since I don't think there's any further review/actions to be done 
here.
D58531  would be the solution for the raised 
`pthread_create()` issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77491

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

[PATCH] D110596: [CUDA] Move CUDA SDK include path further down the include search path.

2021-10-02 Thread Raul Tambre via Phabricator via cfe-commits 
tambre added a comment.

Thank you for your quick turnaround. I can confirm that this works for me on 
Debian Unstable with my vendored LLVM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110596

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

[PATCH] D108247: [CUDA] Improve CUDA version detection and diagnostics.

2021-09-27 Thread Raul Tambre via Phabricator via cfe-commits 
tambre added a comment.

In D108247#3025238 , @tra wrote:

> So, what's the current state of affairs regarding CUDA SDK layout in debian?

`/usr/bin`: most executables 
, 
`nvcc` and `nvprof` shims
`/usr/lib/nvidia-cuda-toolkit/bin`: `cicc`, `crt`, `nvcc`, `nvprof`, `g++` and 
`gcc` wrappers for supporting different compilers
`/usr/lib/nvidia-cuda-toolkit/libdevice`: `libdevice.10.bc`
`/usr/lib/cuda/{bin,include,lib64}`: empty directories
`/usr/lib/cuda/nvvm/libdevice`: symlinked to 
`/usr/lib/nvidia-cuda-toolkit/libdevice`
`/usr/include`: headers

> CUDA SDK no longer ships version.txt, so cuda.h is the only reliable 
> mechanism for detecting CUDA version and clang does need to know it.
> In case the version has not been found we also can not choose an arbitrary 
> old version as the default.

Agreed, sensible.

> So, one workaround would be to install CUDA-11.4. AFAICT, we do find the 
> headers and ptxas, but misdetect CUDA version.

I don't think that's going to be acceptable for most distributions, which might 
ship a Clang supporting a newer CUDA than the one they ship.

> Another workaround would be to place a fake /usr/lib/cuda/include/cuda.h with 
> something like this:

My CMake CI bot [[ https://open.cdash.org/test/498767666 | encountered `cmath` 
template errors ]] after I manually symlinked `/usr/lib/cuda/include` to 
`/usr/include`, though that solved the version detection.
Adding a shim like the one you show fixes everything and compiling works.

> Clang does rely on very particular ordering of includes, so having CUDA 
> headers in a location clang does not expect will lead to issues sooner or 
> later.

Having the headers in the system-wide `/usr/include` shouldn't be a problem 
though, right?
I'm not quite sure why simply having `/usr/lib/cuda/include` → `/usr/include` 
symlink breaks as seen on my bot.
Prior to this an empty `/usr/lib/cuda/include` and Clang finding them through 
the regular include search path worked.

> If the headers are not available where --cuda-path points clang to, I'm not 
> sure what clang is supposed to do. Second-guessing explicit user input is not 
> a good idea in general.

I agree that second-guessing is bad, but expecting it to be available in the 
include search path otherwise seems reasonable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108247

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

[PATCH] D108247: [CUDA] Improve CUDA version detection and diagnostics.

2021-09-26 Thread Raul Tambre via Phabricator via cfe-commits 
tambre added a comment.

This unfortunately breaks 
 using Debian 
distribution CUDA.
The Debian/Ubuntu special case 

 is now useless.

On Debian:

- `cuda.h` is in `/usr/include`, as it must be per policy.
- `libdevice.10.bc` is in `/usr/lib/cuda/nvidia-cuda-toolkit/libdevice` and 
`/usr/lib/cuda/nvvm/libdevice` is symlinked to that.

Passing `--cuda-path=/usr/lib/cuda` previously worked, but now results in 
trying to parse from `/usr/lib/cuda/include/cuda.h` (doesn't exist) and Clang 
defaulting to the newest available version, which isn't supported by the 
`ptxas` currently in Debian.
NVCC has no issues with this layout.

Symlinking `/usr/lib/cuda/include` to `/usr/include` on Debian's side seems 
like the obvious short-term solution for trunk.
Could probably be backported to ensure LTSes will work with Clang 14.

If any thoughts for an alternative solution let me know.
I have filed Debian bug #TODO.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108247

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

[PATCH] D108482: [Clang] Fixes instantiation of OpaqueValueExprs (Bug #45964)

2021-08-21 Thread Raul Tambre via Phabricator via cfe-commits 
tambre requested changes to this revision.
tambre added a comment.
This revision now requires changes to proceed.

Please:

- Add a test case based on the one in the bug. Hopefully you'll be able to 
reduce it further now that you understand the bug.
- Use imperative style for the commit message, i.e. "Fix instantiation of 
OpaqueValueExprs".


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

https://reviews.llvm.org/D108482

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

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

2021-07-12 Thread Raul Tambre via Phabricator via cfe-commits 
tambre added a comment.

In D77491#2870291 , @jdoerfert wrote:

> First:
> Do I assume right this this feature was simply disabled without any plan to:
>
> - inform the authors (me)
> - update the documentation
> - re-enable support eventually or provide alternatives
>
> XFAILing a test and calling it a day seems inadequate IMHO.
>
> Second:
> Would an approach like this still work: https://reviews.llvm.org/D58531 ?

Informing you would've probably been appropriate in hindsight.
I'm not aware of any relevant documentation that would've been appropriate to 
update.

Re-enabling the support depends on someone taking up the work to correctly 
implement the prototype recognition, which seems to be done in D58531 
 and would work as-is.
However, at the time it seemed to me that the whole builtin declaration 
recognition could use a rewrite to be able to this support this case without 
hacks and require less manual C++ type juggling.
I considered that work beyond my experience with the codebase and the benefits 
of this work easily seemed to outweigh the lack of additional annotations for 
this single function. I received no further feedback on the disabling of that 
test after explaining the reason.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77491

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

[PATCH] D102531: PR45881: Properly use CXXThisOverride for templated lambda

2021-07-06 Thread Raul Tambre via Phabricator via cfe-commits 
tambre added inline comments.



Comment at: clang/test/SemaCXX/cxx2a-lambda-decltype-this.cpp:1-4
+// RUN: %clang_cc1 -std=c++2a -fsyntax-only -fblocks -emit-llvm-only %s
+// RUN: %clang_cc1 -std=c++2a -fsyntax-only -fblocks 
-fdelayed-template-parsing %s
+// RUN: %clang_cc1 -std=c++2a -fsyntax-only -fblocks -fms-extensions %s
+// RUN: %clang_cc1 -std=c++2a -fsyntax-only -fblocks 
-fdelayed-template-parsing -fms-extensions %s

`-fblocks` doesn't seem like it'd be necessary?

It would probably be a good idea to rename this to 
`cxx20-lambda-decltype-this.cpp` and use `-std=c++20` from the get-go since 
C++20 has been finalized.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102531

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

[PATCH] D100046: [AArch64] ACLE: Fix issue for mismatching enum types with builtins.

2021-04-12 Thread Raul Tambre via Phabricator via cfe-commits 
tambre added a comment.

Summary also looks good. 
You probably don't need to wait for more approvals.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100046

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

[PATCH] D100046: [AArch64] ACLE: Fix issue for mismatching enum types with builtins.

2021-04-12 Thread Raul Tambre via Phabricator via cfe-commits 
tambre accepted this revision.
tambre added a comment.
This revision is now accepted and ready to land.

Thanks, LGTM! Probably worth mentioning what I had in my comment in the commit 
message.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100046

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

[PATCH] D100046: [AArch64] ACLE: Fix issue for mismatching enum types with builtins.

2021-04-10 Thread Raul Tambre via Phabricator via cfe-commits 
tambre added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:10953-10967
 if (unsigned BuiltinID = NewFD->getBuiltinID()) {
   ASTContext::GetBuiltinTypeError Error;
   LookupNecessaryTypesForBuiltin(S, BuiltinID);
   QualType T = Context.GetBuiltinType(BuiltinID, Error);
   // If the type of the builtin differs only in its exception
   // specification, that's OK.
   // FIXME: If the types do differ in this way, it would be better to

This whole block is unnecessary since D77491 and should've been removed, but 
seems I missed it. Reverting/forgetting builtins was problematic and the 
attribute-based approach solved those deficiencies.

Delete this whole block and also remove `forgetBuiltin()`. The builtin-related 
code in `ActOnFunctionDeclarator()` handles everything necessary. No tests 
break and the SVE ones will pass. :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100046

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

[PATCH] D100046: [AArch64] ACLE: Fix issue for mismatching enum types with builtins.

2021-04-09 Thread Raul Tambre via Phabricator via cfe-commits 
tambre added a comment.

In D100046#2674311 , @sdesmalen wrote:

> The fact that Clang chooses to (explicitly) forget about a builtin in 
> SemaDecl.cpp was quite puzzling to me. Maybe that just shows that I don't 
> fully understand how the builtin mechanism is supposed to work.
> @rsmith and @tambre, git history showed me you have more experience in this 
> area and touched some of that code before, so hopefully you can give me some 
> good feedback!

Sorry for the delay, I'm lately quite busy during the week.
I'll have a look over the weekend.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100046

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

[PATCH] D94374: [CMake] Remove dead code setting policies to NEW

2021-01-19 Thread Raul Tambre via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG480643a95cd1: [CMake] Remove dead code setting policies to 
NEW (authored by tambre).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94374

Files:
  clang/CMakeLists.txt
  compiler-rt/CMakeLists.txt
  flang/CMakeLists.txt
  libcxx/CMakeLists.txt
  libcxx/utils/ci/runtimes/CMakeLists.txt
  libcxxabi/CMakeLists.txt
  libunwind/CMakeLists.txt
  lldb/CMakeLists.txt
  llvm/CMakeLists.txt
  mlir/examples/standalone/CMakeLists.txt

Index: mlir/examples/standalone/CMakeLists.txt
===
--- mlir/examples/standalone/CMakeLists.txt
+++ mlir/examples/standalone/CMakeLists.txt
@@ -1,20 +1,8 @@
 cmake_minimum_required(VERSION 3.13.4)
-
-if(POLICY CMP0068)
-  cmake_policy(SET CMP0068 NEW)
-  set(CMAKE_BUILD_WITH_INSTALL_NAME_DIR ON)
-endif()
-
-if(POLICY CMP0075)
-  cmake_policy(SET CMP0075 NEW)
-endif()
-
-if(POLICY CMP0077)
-  cmake_policy(SET CMP0077 NEW)
-endif()
-
 project(standalone-dialect LANGUAGES CXX C)
 
+set(CMAKE_BUILD_WITH_INSTALL_NAME_DIR ON)
+
 set(CMAKE_CXX_STANDARD 14 CACHE STRING "C++ standard to conform to")
 
 find_package(MLIR REQUIRED CONFIG)
Index: llvm/CMakeLists.txt
===
--- llvm/CMakeLists.txt
+++ llvm/CMakeLists.txt
@@ -2,18 +2,7 @@
 
 cmake_minimum_required(VERSION 3.13.4)
 
-if(POLICY CMP0068)
-  cmake_policy(SET CMP0068 NEW)
-  set(CMAKE_BUILD_WITH_INSTALL_NAME_DIR ON)
-endif()
-
-if(POLICY CMP0075)
-  cmake_policy(SET CMP0075 NEW)
-endif()
-
-if(POLICY CMP0077)
-  cmake_policy(SET CMP0077 NEW)
-endif()
+set(CMAKE_BUILD_WITH_INSTALL_NAME_DIR ON)
 
 if(NOT DEFINED LLVM_VERSION_MAJOR)
   set(LLVM_VERSION_MAJOR 12)
Index: lldb/CMakeLists.txt
===
--- lldb/CMakeLists.txt
+++ lldb/CMakeLists.txt
@@ -1,13 +1,5 @@
 cmake_minimum_required(VERSION 3.13.4)
 
-if(POLICY CMP0075)
-  cmake_policy(SET CMP0075 NEW)
-endif()
-
-if(POLICY CMP0077)
-  cmake_policy(SET CMP0077 NEW)
-endif()
-
 # Add path for custom modules.
 set(CMAKE_MODULE_PATH
   ${CMAKE_MODULE_PATH}
Index: libunwind/CMakeLists.txt
===
--- libunwind/CMakeLists.txt
+++ libunwind/CMakeLists.txt
@@ -8,10 +8,6 @@
 
 cmake_minimum_required(VERSION 3.13.4)
 
-if (POLICY CMP0042)
-  cmake_policy(SET CMP0042 NEW) # Set MACOSX_RPATH=YES by default
-endif()
-
 # Add path for custom modules
 set(CMAKE_MODULE_PATH
   "${CMAKE_CURRENT_SOURCE_DIR}/cmake"
Index: libcxxabi/CMakeLists.txt
===
--- libcxxabi/CMakeLists.txt
+++ libcxxabi/CMakeLists.txt
@@ -10,10 +10,6 @@
 
 cmake_minimum_required(VERSION 3.13.4)
 
-if(POLICY CMP0042)
-  cmake_policy(SET CMP0042 NEW) # Set MACOSX_RPATH=YES by default
-endif()
-
 # Add path for custom modules
 set(CMAKE_MODULE_PATH
   "${CMAKE_CURRENT_SOURCE_DIR}/cmake"
Index: libcxx/utils/ci/runtimes/CMakeLists.txt
===
--- libcxx/utils/ci/runtimes/CMakeLists.txt
+++ libcxx/utils/ci/runtimes/CMakeLists.txt
@@ -1,20 +1,8 @@
 cmake_minimum_required(VERSION 3.13.4)
-
-if(POLICY CMP0068)
-  cmake_policy(SET CMP0068 NEW)
-  set(CMAKE_BUILD_WITH_INSTALL_NAME_DIR ON)
-endif()
-
-if(POLICY CMP0075)
-  cmake_policy(SET CMP0075 NEW)
-endif()
-
-if(POLICY CMP0077)
-  cmake_policy(SET CMP0077 NEW)
-endif()
-
 project(LLVM_RUNTIMES)
 
+set(CMAKE_BUILD_WITH_INSTALL_NAME_DIR ON)
+
 find_package(Python3 COMPONENTS Interpreter)
 if(NOT Python3_Interpreter_FOUND)
   message(WARNING "Python3 not found, using python2 as a fallback")
Index: libcxx/CMakeLists.txt
===
--- libcxx/CMakeLists.txt
+++ libcxx/CMakeLists.txt
@@ -10,16 +10,7 @@
 #===
 cmake_minimum_required(VERSION 3.13.4)
 
-if(POLICY CMP0042)
-  cmake_policy(SET CMP0042 NEW) # Set MACOSX_RPATH=YES by default
-endif()
-if(POLICY CMP0022)
-  cmake_policy(SET CMP0022 NEW) # Required when interacting with LLVM and Clang
-endif()
-if(POLICY CMP0068)
-  cmake_policy(SET CMP0068 NEW)
-  set(CMAKE_BUILD_WITH_INSTALL_NAME_DIR ON)
-endif()
+set(CMAKE_BUILD_WITH_INSTALL_NAME_DIR ON)
 
 # Add path for custom modules
 set(CMAKE_MODULE_PATH
Index: flang/CMakeLists.txt
===
--- flang/CMakeLists.txt
+++ flang/CMakeLists.txt
@@ -1,20 +1,6 @@
 cmake_minimum_required(VERSION 3.13.4)
 
-# RPATH settings on macOS do not affect INSTALL_NAME.
-if (POLICY CMP0068)
-  cmake_policy(SET CMP0068 NEW)
-  set(CMAKE_BUILD_WITH_INSTALL_NAME_DIR ON)
-endif()
-
-# Include file check macros honor CMAKE_REQUIRED_LIBRARIES.
-if(POLICY CMP0075)
-  cmake_policy(SET CMP0075 NEW)

[PATCH] D94374: [CMake] Remove dead code setting policies to NEW

2021-01-19 Thread Raul Tambre via Phabricator via cfe-commits 
tambre marked an inline comment as done.
tambre added a comment.

ldionne: ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94374

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

[PATCH] D94374: [CMake] Remove dead code setting policies to NEW

2021-01-11 Thread Raul Tambre via Phabricator via cfe-commits 
tambre updated this revision to Diff 315847.
tambre added a comment.

Don't modify third-party checked-in libraries.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94374

Files:
  clang/CMakeLists.txt
  compiler-rt/CMakeLists.txt
  flang/CMakeLists.txt
  libcxx/CMakeLists.txt
  libcxx/utils/ci/runtimes/CMakeLists.txt
  libcxxabi/CMakeLists.txt
  libunwind/CMakeLists.txt
  lldb/CMakeLists.txt
  llvm/CMakeLists.txt
  mlir/examples/standalone/CMakeLists.txt

Index: mlir/examples/standalone/CMakeLists.txt
===
--- mlir/examples/standalone/CMakeLists.txt
+++ mlir/examples/standalone/CMakeLists.txt
@@ -1,20 +1,8 @@
 cmake_minimum_required(VERSION 3.13.4)
-
-if(POLICY CMP0068)
-  cmake_policy(SET CMP0068 NEW)
-  set(CMAKE_BUILD_WITH_INSTALL_NAME_DIR ON)
-endif()
-
-if(POLICY CMP0075)
-  cmake_policy(SET CMP0075 NEW)
-endif()
-
-if(POLICY CMP0077)
-  cmake_policy(SET CMP0077 NEW)
-endif()
-
 project(standalone-dialect LANGUAGES CXX C)
 
+set(CMAKE_BUILD_WITH_INSTALL_NAME_DIR ON)
+
 set(CMAKE_CXX_STANDARD 14 CACHE STRING "C++ standard to conform to")
 
 find_package(MLIR REQUIRED CONFIG)
Index: llvm/CMakeLists.txt
===
--- llvm/CMakeLists.txt
+++ llvm/CMakeLists.txt
@@ -2,18 +2,7 @@
 
 cmake_minimum_required(VERSION 3.13.4)
 
-if(POLICY CMP0068)
-  cmake_policy(SET CMP0068 NEW)
-  set(CMAKE_BUILD_WITH_INSTALL_NAME_DIR ON)
-endif()
-
-if(POLICY CMP0075)
-  cmake_policy(SET CMP0075 NEW)
-endif()
-
-if(POLICY CMP0077)
-  cmake_policy(SET CMP0077 NEW)
-endif()
+set(CMAKE_BUILD_WITH_INSTALL_NAME_DIR ON)
 
 if(NOT DEFINED LLVM_VERSION_MAJOR)
   set(LLVM_VERSION_MAJOR 12)
Index: lldb/CMakeLists.txt
===
--- lldb/CMakeLists.txt
+++ lldb/CMakeLists.txt
@@ -1,13 +1,5 @@
 cmake_minimum_required(VERSION 3.13.4)
 
-if(POLICY CMP0075)
-  cmake_policy(SET CMP0075 NEW)
-endif()
-
-if(POLICY CMP0077)
-  cmake_policy(SET CMP0077 NEW)
-endif()
-
 # Add path for custom modules.
 set(CMAKE_MODULE_PATH
   ${CMAKE_MODULE_PATH}
Index: libunwind/CMakeLists.txt
===
--- libunwind/CMakeLists.txt
+++ libunwind/CMakeLists.txt
@@ -8,10 +8,6 @@
 
 cmake_minimum_required(VERSION 3.13.4)
 
-if (POLICY CMP0042)
-  cmake_policy(SET CMP0042 NEW) # Set MACOSX_RPATH=YES by default
-endif()
-
 # Add path for custom modules
 set(CMAKE_MODULE_PATH
   "${CMAKE_CURRENT_SOURCE_DIR}/cmake"
Index: libcxxabi/CMakeLists.txt
===
--- libcxxabi/CMakeLists.txt
+++ libcxxabi/CMakeLists.txt
@@ -10,10 +10,6 @@
 
 cmake_minimum_required(VERSION 3.13.4)
 
-if(POLICY CMP0042)
-  cmake_policy(SET CMP0042 NEW) # Set MACOSX_RPATH=YES by default
-endif()
-
 # Add path for custom modules
 set(CMAKE_MODULE_PATH
   "${CMAKE_CURRENT_SOURCE_DIR}/cmake"
Index: libcxx/utils/ci/runtimes/CMakeLists.txt
===
--- libcxx/utils/ci/runtimes/CMakeLists.txt
+++ libcxx/utils/ci/runtimes/CMakeLists.txt
@@ -1,20 +1,8 @@
 cmake_minimum_required(VERSION 3.13.4)
-
-if(POLICY CMP0068)
-  cmake_policy(SET CMP0068 NEW)
-  set(CMAKE_BUILD_WITH_INSTALL_NAME_DIR ON)
-endif()
-
-if(POLICY CMP0075)
-  cmake_policy(SET CMP0075 NEW)
-endif()
-
-if(POLICY CMP0077)
-  cmake_policy(SET CMP0077 NEW)
-endif()
-
 project(LLVM_RUNTIMES)
 
+set(CMAKE_BUILD_WITH_INSTALL_NAME_DIR ON)
+
 find_package(Python3 COMPONENTS Interpreter)
 if(NOT Python3_Interpreter_FOUND)
   message(WARNING "Python3 not found, using python2 as a fallback")
Index: libcxx/CMakeLists.txt
===
--- libcxx/CMakeLists.txt
+++ libcxx/CMakeLists.txt
@@ -10,16 +10,7 @@
 #===
 cmake_minimum_required(VERSION 3.13.4)
 
-if(POLICY CMP0042)
-  cmake_policy(SET CMP0042 NEW) # Set MACOSX_RPATH=YES by default
-endif()
-if(POLICY CMP0022)
-  cmake_policy(SET CMP0022 NEW) # Required when interacting with LLVM and Clang
-endif()
-if(POLICY CMP0068)
-  cmake_policy(SET CMP0068 NEW)
-  set(CMAKE_BUILD_WITH_INSTALL_NAME_DIR ON)
-endif()
+set(CMAKE_BUILD_WITH_INSTALL_NAME_DIR ON)
 
 # Add path for custom modules
 set(CMAKE_MODULE_PATH
Index: flang/CMakeLists.txt
===
--- flang/CMakeLists.txt
+++ flang/CMakeLists.txt
@@ -1,20 +1,6 @@
 cmake_minimum_required(VERSION 3.13.4)
 
-# RPATH settings on macOS do not affect INSTALL_NAME.
-if (POLICY CMP0068)
-  cmake_policy(SET CMP0068 NEW)
-  set(CMAKE_BUILD_WITH_INSTALL_NAME_DIR ON)
-endif()
-
-# Include file check macros honor CMAKE_REQUIRED_LIBRARIES.
-if(POLICY CMP0075)
-  cmake_policy(SET CMP0075 NEW)
-endif()
-
-# option() honors normal variables.
-if (POLICY

[PATCH] D94374: [CMake] Remove dead code setting policies to NEW

2021-01-11 Thread Raul Tambre via Phabricator via cfe-commits 
tambre marked an inline comment as done.
tambre added inline comments.



Comment at: libcxx/utils/google-benchmark/CMakeLists.txt:3
-
-project (benchmark)
-

ldionne wrote:
> I don't think we want to change this. It's a third-party project (which is 
> inconveniently checked-in as-is into our tree..).
Good catch. Did the same for `llvm/utils/benchmark/CMakeLists.txt`, though that 
one seems to have the `cmake_minimum_required(VERSION)` updated to 3.13.4 
unlike this one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94374

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

[PATCH] D94374: [CMake] Remove dead code setting policies to NEW

2021-01-11 Thread Raul Tambre via Phabricator via cfe-commits 
tambre added a reviewer: ldionne.
tambre added a comment.

ldionne: for libc++ review


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94374

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

[PATCH] D94374: [CMake] Remove dead code setting policies to NEW

2021-01-11 Thread Raul Tambre via Phabricator via cfe-commits 
tambre created this revision.
tambre added a reviewer: phosek.
Herald added subscribers: libcxx-commits, teijeong, rdzhabarov, tatianashp, 
msifontes, jurahul, Kayjukh, grosul1, Joonsoo, liufengdb, aartbik, lucyrfox, 
mgester, arpith-jacob, antiagainst, shauheen, rriddle, mehdi_amini, lebedev.ri, 
arichardson, mgorny.
Herald added a reviewer: lebedev.ri.
Herald added a project: libunwind.
Herald added a reviewer: libunwind.
tambre requested review of this revision.
Herald added subscribers: llvm-commits, lldb-commits, Sanitizers, cfe-commits, 
stephenneuendorffer, nicolasvasilache.
Herald added projects: clang, Sanitizers, LLDB, libc++, libc++abi, MLIR, LLVM.
Herald added a reviewer: libc++.
Herald added a reviewer: libc++abi.

cmake_minimum_required(VERSION) calls cmake_policy(VERSION),
which sets all policies up to VERSION to NEW.
LLVM started requiring CMake 3.13 last year, so we can remove
a bunch of code setting policies prior to 3.13 to NEW as it
no longer has any effect.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94374

Files:
  clang/CMakeLists.txt
  compiler-rt/CMakeLists.txt
  flang/CMakeLists.txt
  libcxx/CMakeLists.txt
  libcxx/utils/ci/runtimes/CMakeLists.txt
  libcxx/utils/google-benchmark/CMakeLists.txt
  libcxxabi/CMakeLists.txt
  libunwind/CMakeLists.txt
  lldb/CMakeLists.txt
  llvm/CMakeLists.txt
  llvm/utils/benchmark/CMakeLists.txt
  mlir/examples/standalone/CMakeLists.txt

Index: mlir/examples/standalone/CMakeLists.txt
===
--- mlir/examples/standalone/CMakeLists.txt
+++ mlir/examples/standalone/CMakeLists.txt
@@ -1,20 +1,8 @@
 cmake_minimum_required(VERSION 3.13.4)
-
-if(POLICY CMP0068)
-  cmake_policy(SET CMP0068 NEW)
-  set(CMAKE_BUILD_WITH_INSTALL_NAME_DIR ON)
-endif()
-
-if(POLICY CMP0075)
-  cmake_policy(SET CMP0075 NEW)
-endif()
-
-if(POLICY CMP0077)
-  cmake_policy(SET CMP0077 NEW)
-endif()
-
 project(standalone-dialect LANGUAGES CXX C)
 
+set(CMAKE_BUILD_WITH_INSTALL_NAME_DIR ON)
+
 set(CMAKE_CXX_STANDARD 14 CACHE STRING "C++ standard to conform to")
 
 find_package(MLIR REQUIRED CONFIG)
Index: llvm/utils/benchmark/CMakeLists.txt
===
--- llvm/utils/benchmark/CMakeLists.txt
+++ llvm/utils/benchmark/CMakeLists.txt
@@ -1,22 +1,5 @@
 cmake_minimum_required(VERSION 3.13.4)
-
-# Tell cmake 3.0+ that it's safe to clear the PROJECT_VERSION variable in the
-# call to project() below.
-if(POLICY CMP0048)
-  cmake_policy(SET CMP0048 NEW)
-endif()
-
-project (benchmark)
-
-foreach(p
-CMP0054 # CMake 3.1
-CMP0056 # export EXE_LINKER_FLAGS to try_run
-CMP0057 # Support no if() IN_LIST operator
-)
-  if(POLICY ${p})
-cmake_policy(SET ${p} NEW)
-  endif()
-endforeach()
+project(benchmark)
 
 option(BENCHMARK_ENABLE_TESTING "Enable testing of the benchmark library." ON)
 option(BENCHMARK_ENABLE_EXCEPTIONS "Enable the use of exceptions in the benchmark library." ON)
Index: llvm/CMakeLists.txt
===
--- llvm/CMakeLists.txt
+++ llvm/CMakeLists.txt
@@ -2,18 +2,7 @@
 
 cmake_minimum_required(VERSION 3.13.4)
 
-if(POLICY CMP0068)
-  cmake_policy(SET CMP0068 NEW)
-  set(CMAKE_BUILD_WITH_INSTALL_NAME_DIR ON)
-endif()
-
-if(POLICY CMP0075)
-  cmake_policy(SET CMP0075 NEW)
-endif()
-
-if(POLICY CMP0077)
-  cmake_policy(SET CMP0077 NEW)
-endif()
+set(CMAKE_BUILD_WITH_INSTALL_NAME_DIR ON)
 
 if(NOT DEFINED LLVM_VERSION_MAJOR)
   set(LLVM_VERSION_MAJOR 12)
Index: lldb/CMakeLists.txt
===
--- lldb/CMakeLists.txt
+++ lldb/CMakeLists.txt
@@ -1,13 +1,5 @@
 cmake_minimum_required(VERSION 3.13.4)
 
-if(POLICY CMP0075)
-  cmake_policy(SET CMP0075 NEW)
-endif()
-
-if(POLICY CMP0077)
-  cmake_policy(SET CMP0077 NEW)
-endif()
-
 # Add path for custom modules.
 set(CMAKE_MODULE_PATH
   ${CMAKE_MODULE_PATH}
Index: libunwind/CMakeLists.txt
===
--- libunwind/CMakeLists.txt
+++ libunwind/CMakeLists.txt
@@ -8,10 +8,6 @@
 
 cmake_minimum_required(VERSION 3.13.4)
 
-if (POLICY CMP0042)
-  cmake_policy(SET CMP0042 NEW) # Set MACOSX_RPATH=YES by default
-endif()
-
 # Add path for custom modules
 set(CMAKE_MODULE_PATH
   "${CMAKE_CURRENT_SOURCE_DIR}/cmake"
Index: libcxxabi/CMakeLists.txt
===
--- libcxxabi/CMakeLists.txt
+++ libcxxabi/CMakeLists.txt
@@ -10,10 +10,6 @@
 
 cmake_minimum_required(VERSION 3.13.4)
 
-if(POLICY CMP0042)
-  cmake_policy(SET CMP0042 NEW) # Set MACOSX_RPATH=YES by default
-endif()
-
 # Add path for custom modules
 set(CMAKE_MODULE_PATH
   "${CMAKE_CURRENT_SOURCE_DIR}/cmake"
Index: libcxx/utils/google-benchmark/CMakeLists.txt
===
--- libcxx/utils/google-benchmark/CMakeLists.txt
+++

[PATCH] D86308: [CMake][compiler-rt][libunwind] Compile assembly files as ASM not C, unify workarounds

2020-11-28 Thread Raul Tambre via Phabricator via cfe-commits 
tambre added a comment.

In D86308#2421249 , @aaronpuchert 
wrote:

> In D86308#2421237 , @tambre wrote:
>
>> This has already been backported to 11.0.1: 
>> https://github.com/llvm/llvm-project/commit/03565ffd5da8370f5b89b69cd9868f32e2d75403
>
> Thanks! I didn't see any mention of that here so I assumed it wasn't 
> backported without checking.

Also, the issue was an accidental breaking change 
 in CMake 3.19.0 and has 
been reverted in 3.19.1.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86308

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

[PATCH] D86308: [CMake][compiler-rt][libunwind] Compile assembly files as ASM not C, unify workarounds

2020-11-28 Thread Raul Tambre via Phabricator via cfe-commits 
tambre added a comment.

In D86308#2421228 , @aaronpuchert 
wrote:

> @tstellar, can we have this in 11.0.1? I've already ported it back in 
> openSUSE because we were having problems with the CMake 3.19 update.

This has already been backported to 11.0.1: 
https://github.com/llvm/llvm-project/commit/03565ffd5da8370f5b89b69cd9868f32e2d75403


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86308

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

[PATCH] D89974: [driver][CUDA] Use CMake's FindCUDA as default --cuda-path.

2020-10-22 Thread Raul Tambre via Phabricator via cfe-commits 
tambre added a comment.

I echo @tra's concerns.
Having an option for a vendor to append/prepend a toolkit search location seems 
useful, but currently this seems more like a workaround for an explicit CUDA 
toolkit path not being passed to the testsuite.
Shortcomings in the testrunner don't seem like a reason to introduce new 
build-time configured default search paths into the driver.

Aside, here's an overview of CMake+CUDA:

- FindCUDA is deprecated since 3.10 and obsolete since 3.17. Replaced by native 
language support and FindCUDAToolkit 
.
- CMake supports Clang as a CUDA compiler since 3.18.
- CMake supports separable compilation with Clang since 3.19.
- CMake doesn't support CUDA with Clang on Windows 
. I hope to work on it 
for 3.20.

Ideally LLVM would use the native CMake CUDA support and the testsuite would 
use the CUDA toolkit found by CMake where necessary.
Users would be able to override the selected toolkit by setting 
`CUDAToolkit_ROOT` when configuring Clang.
Unfortunately this would require CMake 3.18, so this'll have to wait a bit.

If there are any questions/issues on the CMake side let me know. I maintain 
Clang and CUDA support in CMake.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89974

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

[PATCH] D88518: Recognize setjmp and friends as builtins even if jmp_buf is not declared yet.

2020-09-29 Thread Raul Tambre via Phabricator via cfe-commits 
tambre added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88518

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

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

2020-09-29 Thread Raul Tambre via Phabricator via cfe-commits 
tambre added a comment.

In D77491#2299938 , @rsmith wrote:

> We've hit a fairly subtle miscompile caused by this patch.
>
> glibc's setjmp.h looks like this (irrelevant parts removed):
>
>   struct __jmp_buf_tag { /*...*/ };
>   extern int __sigsetjmp(struct __jmp_buf_tag __env[1], int);
>   typedef struct __jmp_buf_tag sigjmp_buf[1];
>   #define sigsetjmp __sigsetjmp
>
> This worked fine with the old approach. But with the new approach, we decide 
> the declaration of `__sigsetjmp` is not a builtin, because at its point of 
> declaration, we can't compute the "proper" type because `sigjmp_buf` has not 
> been declared yet. As a result, we don't add a `BuiltinAttr` to 
> `__sigsetjmp`, but much more critically, we don't add a `ReturnsTwiceAttr`, 
> which results in miscompiles in calls to this function. (I think `sigsetjmp` 
> is the only affected function with glibc. `jmp_buf` is declared prior to 
> `__setjmp` and friends.)
>
> I suppose we don't actually care what the parameter types for `__sigsetjmp` 
> are, and it would be fine (and much safer) to treat any function with that 
> name as a builtin, like we used to. Perhaps we should have a way of marking 
> builtins as "the given type is what we expect / what we will implicitly 
> declare, but it's OK if it doesn't actually match"?

Marking `__sigsetjmp` as having custom typechecking should suffice (`t` 
attribute in Builtins.def), no? Though a case in 
`Sema::CheckBuiltinFunctionCall()` might also then be necessary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77491

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

[PATCH] D87983: [Sema] Split special builtin type lookups into a separate function

2020-09-21 Thread Raul Tambre via Phabricator via cfe-commits 
tambre added inline comments.



Comment at: clang/lib/Sema/SemaLookup.cpp:924
+void Sema::LookupNecessaryTypesForBuiltin(Scope *S, unsigned ID) {
+  if (getLangOpts().ObjC && ID == Builtin::BIobjc_msgSendSuper)
+LookupPredefedObjCSuperType(*this, S);

rjmccall wrote:
> I'm pretty sure this builtin only exists in ObjC mode, so there's no need to 
> separately check the language mode.  (Even if you did want to check it, it 
> would be better to check the builtin mode first, as that's a very trivial 
> check.)
Fixed, thanks for the review!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87983

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

[PATCH] D87983: [Sema] Split special builtin type lookups into a separate function

2020-09-21 Thread Raul Tambre via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
tambre marked an inline comment as done.
Closed by commit rGf91f28c350df: [Sema] Split special builtin type lookups into 
a separate function (authored by tambre).

Changed prior to commit:
  https://reviews.llvm.org/D87983?vs=293008=293193#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87983

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaLookup.cpp


Index: clang/lib/Sema/SemaLookup.cpp
===
--- clang/lib/Sema/SemaLookup.cpp
+++ clang/lib/Sema/SemaLookup.cpp
@@ -907,6 +907,24 @@
   return false;
 }
 
+/// Looks up the declaration of "struct objc_super" and
+/// saves it for later use in building builtin declaration of
+/// objc_msgSendSuper and objc_msgSendSuper_stret.
+static void LookupPredefedObjCSuperType(Sema , Scope *S) {
+  ASTContext  = Sema.Context;
+  LookupResult Result(Sema, ("objc_super"), 
SourceLocation(),
+  Sema::LookupTagName);
+  Sema.LookupName(Result, S);
+  if (Result.getResultKind() == LookupResult::Found)
+if (const TagDecl *TD = Result.getAsSingle())
+  Context.setObjCSuperType(Context.getTagDeclType(TD));
+}
+
+void Sema::LookupNecessaryTypesForBuiltin(Scope *S, unsigned ID) {
+  if (ID == Builtin::BIobjc_msgSendSuper)
+LookupPredefedObjCSuperType(*this, S);
+}
+
 /// Determine whether we can declare a special member function within
 /// the class at this point.
 static bool CanDeclareSpecialMemberFunction(const CXXRecordDecl *Class) {
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -2035,24 +2035,6 @@
   return S;
 }
 
-/// Looks up the declaration of "struct objc_super" and
-/// saves it for later use in building builtin declaration of
-/// objc_msgSendSuper and objc_msgSendSuper_stret. If no such
-/// pre-existing declaration exists no action takes place.
-static void LookupPredefedObjCSuperType(Sema , Scope *S,
-IdentifierInfo *II) {
-  if (!II->isStr("objc_msgSendSuper"))
-return;
-  ASTContext  = ThisSema.Context;
-
-  LookupResult Result(ThisSema, ("objc_super"),
-  SourceLocation(), Sema::LookupTagName);
-  ThisSema.LookupName(Result, S);
-  if (Result.getResultKind() == LookupResult::Found)
-if (const TagDecl *TD = Result.getAsSingle())
-  Context.setObjCSuperType(Context.getTagDeclType(TD));
-}
-
 static StringRef getHeaderName(Builtin::Context , unsigned ID,
ASTContext::GetBuiltinTypeError Error) {
   switch (Error) {
@@ -2113,7 +2095,7 @@
 NamedDecl *Sema::LazilyCreateBuiltin(IdentifierInfo *II, unsigned ID,
  Scope *S, bool ForRedeclaration,
  SourceLocation Loc) {
-  LookupPredefedObjCSuperType(*this, S, II);
+  LookupNecessaryTypesForBuiltin(S, ID);
 
   ASTContext::GetBuiltinTypeError Error;
   QualType R = Context.GetBuiltinType(ID, Error);
@@ -9671,7 +9653,7 @@
 NewFD->addAttr(BuiltinAttr::CreateImplicit(Context, BuiltinID));
   } else {
 ASTContext::GetBuiltinTypeError Error;
-LookupPredefedObjCSuperType(*this, S, NewFD->getIdentifier());
+LookupNecessaryTypesForBuiltin(S, BuiltinID);
 QualType BuiltinType = Context.GetBuiltinType(BuiltinID, Error);
 
 if (!Error && !BuiltinType.isNull() &&
@@ -10880,7 +10862,7 @@
 // declaration against the expected type for the builtin.
 if (unsigned BuiltinID = NewFD->getBuiltinID()) {
   ASTContext::GetBuiltinTypeError Error;
-  LookupPredefedObjCSuperType(*this, S, NewFD->getIdentifier());
+  LookupNecessaryTypesForBuiltin(S, BuiltinID);
   QualType T = Context.GetBuiltinType(BuiltinID, Error);
   // If the type of the builtin differs only in its exception
   // specification, that's OK.
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -3813,6 +3813,7 @@
   RedeclarationKind Redecl
 = NotForRedeclaration);
   bool LookupBuiltin(LookupResult );
+  void LookupNecessaryTypesForBuiltin(Scope *S, unsigned ID);
   bool LookupName(LookupResult , Scope *S,
   bool AllowBuiltinCreation = false);
   bool LookupQualifiedName(LookupResult , DeclContext *LookupCtx,


Index: clang/lib/Sema/SemaLookup.cpp
===
--- clang/lib/Sema/SemaLookup.cpp
+++ clang/lib/Sema/SemaLookup.cpp
@@ -907,6 +907,24 @@
   return false;
 }
 
+/// Looks up the declaration of "struct

[PATCH] D87983: [Sema] Split special builtin type lookups into a separate function

2020-09-20 Thread Raul Tambre via Phabricator via cfe-commits 
tambre created this revision.
tambre added reviewers: rsmith, rjmccall.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
tambre requested review of this revision.

In case further such cases appear in the future we've got a generic function to 
add them to.
Additionally changed the ObjC special case to check the language and the 
identifier builtin ID instead of the name.

Addresses the cleanup suggestion from D87917 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87983

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaLookup.cpp


Index: clang/lib/Sema/SemaLookup.cpp
===
--- clang/lib/Sema/SemaLookup.cpp
+++ clang/lib/Sema/SemaLookup.cpp
@@ -907,6 +907,24 @@
   return false;
 }
 
+/// Looks up the declaration of "struct objc_super" and
+/// saves it for later use in building builtin declaration of
+/// objc_msgSendSuper and objc_msgSendSuper_stret.
+static void LookupPredefedObjCSuperType(Sema , Scope *S) {
+  ASTContext  = Sema.Context;
+  LookupResult Result(Sema, ("objc_super"), 
SourceLocation(),
+  Sema::LookupTagName);
+  Sema.LookupName(Result, S);
+  if (Result.getResultKind() == LookupResult::Found)
+if (const TagDecl *TD = Result.getAsSingle())
+  Context.setObjCSuperType(Context.getTagDeclType(TD));
+}
+
+void Sema::LookupNecessaryTypesForBuiltin(Scope *S, unsigned ID) {
+  if (getLangOpts().ObjC && ID == Builtin::BIobjc_msgSendSuper)
+LookupPredefedObjCSuperType(*this, S);
+}
+
 /// Determine whether we can declare a special member function within
 /// the class at this point.
 static bool CanDeclareSpecialMemberFunction(const CXXRecordDecl *Class) {
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -2035,24 +2035,6 @@
   return S;
 }
 
-/// Looks up the declaration of "struct objc_super" and
-/// saves it for later use in building builtin declaration of
-/// objc_msgSendSuper and objc_msgSendSuper_stret. If no such
-/// pre-existing declaration exists no action takes place.
-static void LookupPredefedObjCSuperType(Sema , Scope *S,
-IdentifierInfo *II) {
-  if (!II->isStr("objc_msgSendSuper"))
-return;
-  ASTContext  = ThisSema.Context;
-
-  LookupResult Result(ThisSema, ("objc_super"),
-  SourceLocation(), Sema::LookupTagName);
-  ThisSema.LookupName(Result, S);
-  if (Result.getResultKind() == LookupResult::Found)
-if (const TagDecl *TD = Result.getAsSingle())
-  Context.setObjCSuperType(Context.getTagDeclType(TD));
-}
-
 static StringRef getHeaderName(Builtin::Context , unsigned ID,
ASTContext::GetBuiltinTypeError Error) {
   switch (Error) {
@@ -2113,7 +2095,7 @@
 NamedDecl *Sema::LazilyCreateBuiltin(IdentifierInfo *II, unsigned ID,
  Scope *S, bool ForRedeclaration,
  SourceLocation Loc) {
-  LookupPredefedObjCSuperType(*this, S, II);
+  LookupNecessaryTypesForBuiltin(S, ID);
 
   ASTContext::GetBuiltinTypeError Error;
   QualType R = Context.GetBuiltinType(ID, Error);
@@ -9671,7 +9653,7 @@
 NewFD->addAttr(BuiltinAttr::CreateImplicit(Context, BuiltinID));
   } else {
 ASTContext::GetBuiltinTypeError Error;
-LookupPredefedObjCSuperType(*this, S, NewFD->getIdentifier());
+LookupNecessaryTypesForBuiltin(S, BuiltinID);
 QualType BuiltinType = Context.GetBuiltinType(BuiltinID, Error);
 
 if (!Error && !BuiltinType.isNull() &&
@@ -10880,7 +10862,7 @@
 // declaration against the expected type for the builtin.
 if (unsigned BuiltinID = NewFD->getBuiltinID()) {
   ASTContext::GetBuiltinTypeError Error;
-  LookupPredefedObjCSuperType(*this, S, NewFD->getIdentifier());
+  LookupNecessaryTypesForBuiltin(S, BuiltinID);
   QualType T = Context.GetBuiltinType(BuiltinID, Error);
   // If the type of the builtin differs only in its exception
   // specification, that's OK.
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -3813,6 +3813,7 @@
   RedeclarationKind Redecl
 = NotForRedeclaration);
   bool LookupBuiltin(LookupResult );
+  void LookupNecessaryTypesForBuiltin(Scope *S, unsigned ID);
   bool LookupName(LookupResult , Scope *S,
   bool AllowBuiltinCreation = false);
   bool LookupQualifiedName(LookupResult , DeclContext *LookupCtx,


Index: clang/lib/Sema/SemaLookup.cpp
===
--- clang/lib/Sema/SemaLookup.cpp
+++

[PATCH] D87917: [Sema] Handle objc_super special lookup when checking builtin compatibility

2020-09-18 Thread Raul Tambre via Phabricator via cfe-commits 
tambre added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:9674
 ASTContext::GetBuiltinTypeError Error;
+LookupPredefedObjCSuperType(*this, S, NewFD->getIdentifier());
 QualType BuiltinType = Context.GetBuiltinType(BuiltinID, Error);

rjmccall wrote:
> Can we avoid doing this except for the small number of builtins that use 
> `struct objc_super`?  We already have the BuiltinID.
`LookupPredefedObjCSuperType()` already checks if the identifier is 
`"objc_msgSendSuper"`. But it'd probably be more efficient to check the 
`BuiltinID`. I'll follow this up tomorrow.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87917

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

[PATCH] D87917: [Sema] Handle objc_super special lookup when checking builtin compatibility

2020-09-18 Thread Raul Tambre 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 rGa1aa330b202f: [Sema] Handle objc_super special lookup when 
checking builtin compatibility (authored by tambre).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87917

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/SemaObjCXX/builtin-objcsuper.mm


Index: clang/test/SemaObjCXX/builtin-objcsuper.mm
===
--- /dev/null
+++ clang/test/SemaObjCXX/builtin-objcsuper.mm
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -verify %s
+// expected-no-diagnostics
+
+// objc_super has special lookup rules for compatibility with macOS headers, so
+// the following should compile.
+struct objc_super {};
+extern "C" id objc_msgSendSuper(struct objc_super *super, SEL op, ...);
+extern "C" void objc_msgSendSuper_stret(struct objc_super *super, SEL op, ...);
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -9671,6 +9671,7 @@
 NewFD->addAttr(BuiltinAttr::CreateImplicit(Context, BuiltinID));
   } else {
 ASTContext::GetBuiltinTypeError Error;
+LookupPredefedObjCSuperType(*this, S, NewFD->getIdentifier());
 QualType BuiltinType = Context.GetBuiltinType(BuiltinID, Error);
 
 if (!Error && !BuiltinType.isNull() &&


Index: clang/test/SemaObjCXX/builtin-objcsuper.mm
===
--- /dev/null
+++ clang/test/SemaObjCXX/builtin-objcsuper.mm
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -verify %s
+// expected-no-diagnostics
+
+// objc_super has special lookup rules for compatibility with macOS headers, so
+// the following should compile.
+struct objc_super {};
+extern "C" id objc_msgSendSuper(struct objc_super *super, SEL op, ...);
+extern "C" void objc_msgSendSuper_stret(struct objc_super *super, SEL op, ...);
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -9671,6 +9671,7 @@
 NewFD->addAttr(BuiltinAttr::CreateImplicit(Context, BuiltinID));
   } else {
 ASTContext::GetBuiltinTypeError Error;
+LookupPredefedObjCSuperType(*this, S, NewFD->getIdentifier());
 QualType BuiltinType = Context.GetBuiltinType(BuiltinID, Error);
 
 if (!Error && !BuiltinType.isNull() &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87917: [Sema] Handle objc_super special lookup when checking builtin compatibility

2020-09-18 Thread Raul Tambre via Phabricator via cfe-commits 
tambre added a comment.

I'll go ahead and commit this, as it's affecting an user and the fix is simple 
and obvious. Hopefully post-commit review is acceptable per my understanding of 
the rules.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87917

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

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

2020-09-18 Thread Raul Tambre via Phabricator via cfe-commits 
tambre added a comment.

In D77491#2282219 , @tambre wrote:

> In D77491#2282166 , @dmajor wrote:
>
>> This commit broke Firefox builds on Mac with an error in the SDK headers. 
>> Could you please revert if a fix is not readily available?
>>
>> Reproducer:
>>
>>   struct objc_super {};
>>   extern "C" id objc_msgSendSuper(struct objc_super *super, SEL op, ...);
>>   extern "C" void objc_msgSendSuper_stret(struct objc_super *super, SEL op, 
>> ...);
>>
>> Result:
>>
>>   $ clang -c test.mm
>>   test.mm:3:48: error: reference to 'objc_super' is ambiguous
>>   extern "C" void objc_msgSendSuper_stret(struct objc_super *super, SEL op, 
>> ...);
>>  ^
>>   test.mm:1:8: note: candidate found by name lookup is 'objc_super'
>>   struct objc_super {};
>>  ^
>>   note: candidate found by name lookup is 'objc_super'
>>   1 error generated.
>
> Looking into this, got an idea what might be wrong. Gimme an hour or two. :)

Fix in D87917 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77491

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

[PATCH] D87917: [Sema] Handle objc_super special lookup when checking builtin compatibility

2020-09-18 Thread Raul Tambre via Phabricator via cfe-commits 
tambre created this revision.
tambre added a reviewer: rsmith.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
tambre requested review of this revision.

objc_super is special and needs LookupPredefedObjCSuperType() called before 
performing builtin type comparisons.
This fixes an error when compiling macOS headers. A test is added.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87917

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/SemaObjCXX/builtin-objcsuper.mm


Index: clang/test/SemaObjCXX/builtin-objcsuper.mm
===
--- /dev/null
+++ clang/test/SemaObjCXX/builtin-objcsuper.mm
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -verify %s
+// expected-no-diagnostics
+
+// objc_super has special lookup rules for compatibility with macOS headers, so
+// the following should compile.
+struct objc_super {};
+extern "C" id objc_msgSendSuper(struct objc_super *super, SEL op, ...);
+extern "C" void objc_msgSendSuper_stret(struct objc_super *super, SEL op, ...);
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -9671,6 +9671,7 @@
 NewFD->addAttr(BuiltinAttr::CreateImplicit(Context, BuiltinID));
   } else {
 ASTContext::GetBuiltinTypeError Error;
+LookupPredefedObjCSuperType(*this, S, NewFD->getIdentifier());
 QualType BuiltinType = Context.GetBuiltinType(BuiltinID, Error);
 
 if (!Error && !BuiltinType.isNull() &&


Index: clang/test/SemaObjCXX/builtin-objcsuper.mm
===
--- /dev/null
+++ clang/test/SemaObjCXX/builtin-objcsuper.mm
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -verify %s
+// expected-no-diagnostics
+
+// objc_super has special lookup rules for compatibility with macOS headers, so
+// the following should compile.
+struct objc_super {};
+extern "C" id objc_msgSendSuper(struct objc_super *super, SEL op, ...);
+extern "C" void objc_msgSendSuper_stret(struct objc_super *super, SEL op, ...);
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -9671,6 +9671,7 @@
 NewFD->addAttr(BuiltinAttr::CreateImplicit(Context, BuiltinID));
   } else {
 ASTContext::GetBuiltinTypeError Error;
+LookupPredefedObjCSuperType(*this, S, NewFD->getIdentifier());
 QualType BuiltinType = Context.GetBuiltinType(BuiltinID, Error);
 
 if (!Error && !BuiltinType.isNull() &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

2020-09-18 Thread Raul Tambre via Phabricator via cfe-commits 
tambre added a comment.

In D77491#2282166 , @dmajor wrote:

> This commit broke Firefox builds on Mac with an error in the SDK headers. 
> Could you please revert if a fix is not readily available?
>
> Reproducer:
>
>   struct objc_super {};
>   extern "C" id objc_msgSendSuper(struct objc_super *super, SEL op, ...);
>   extern "C" void objc_msgSendSuper_stret(struct objc_super *super, SEL op, 
> ...);
>
> Result:
>
>   $ clang -c test.mm
>   test.mm:3:48: error: reference to 'objc_super' is ambiguous
>   extern "C" void objc_msgSendSuper_stret(struct objc_super *super, SEL op, 
> ...);
>  ^
>   test.mm:1:8: note: candidate found by name lookup is 'objc_super'
>   struct objc_super {};
>  ^
>   note: candidate found by name lookup is 'objc_super'
>   1 error generated.

Looking into this, got an idea what might be wrong. Gimme an hour or two. :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77491

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

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

2020-09-17 Thread Raul Tambre via Phabricator via cfe-commits 
This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe09107ab80dc: [Sema] Introduce BuiltinAttr, per-declaration 
builtin-ness (authored by tambre).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77491

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Basic/IdentifierTable.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Decl.cpp
  clang/lib/Headers/intrin.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/AST/ast-dump-attr.cpp
  clang/test/CodeGen/builtin-redeclaration.c
  clang/test/CodeGen/callback_pthread_create.c
  clang/test/CodeGenCXX/builtins.cpp
  clang/test/Sema/implicit-builtin-decl.c
  clang/test/Sema/warn-fortify-source.c
  clang/test/SemaCXX/cxx11-compat.cpp
  clang/test/SemaCXX/warn-unused-local-typedef.cpp

Index: clang/test/SemaCXX/warn-unused-local-typedef.cpp
===
--- clang/test/SemaCXX/warn-unused-local-typedef.cpp
+++ clang/test/SemaCXX/warn-unused-local-typedef.cpp
@@ -67,10 +67,10 @@
 
 void test() {
   typedef signed long int superint; // no diag
-  printf("%f", (superint) 42);
+  printf("%ld", (superint)42);
 
   typedef signed long int superint2; // no diag
-  printf("%f", static_cast(42));
+  printf("%ld", static_cast(42));
 
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wunused-local-typedef"
Index: clang/test/SemaCXX/cxx11-compat.cpp
===
--- clang/test/SemaCXX/cxx11-compat.cpp
+++ clang/test/SemaCXX/cxx11-compat.cpp
@@ -31,7 +31,7 @@
 s = { n }, // expected-warning {{non-constant-expression cannot be narrowed from type 'int' to 'char' in initializer list in C++11}} expected-note {{explicit cast}}
 t = { 1234 }; // expected-warning {{constant expression evaluates to 1234 which cannot be narrowed to type 'char' in C++11}} expected-warning {{changes value}} expected-note {{explicit cast}}
 
-#define PRIuS "uS"
+#define PRIuS "zu"
 int printf(const char *, ...);
 typedef __typeof(sizeof(int)) size_t;
 void h(size_t foo, size_t bar) {
Index: clang/test/Sema/warn-fortify-source.c
===
--- clang/test/Sema/warn-fortify-source.c
+++ clang/test/Sema/warn-fortify-source.c
@@ -1,8 +1,6 @@
 // RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify
-// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_PASS_OBJECT_SIZE
 // RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_BUILTINS
 // RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify
-// RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_PASS_OBJECT_SIZE
 // RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_BUILTINS
 
 typedef unsigned long size_t;
@@ -13,13 +11,7 @@
 
 extern int sprintf(char *str, const char *format, ...);
 
-#if defined(USE_PASS_OBJECT_SIZE)
-void *memcpy(void *dst, const void *src, size_t c);
-static void *memcpy(void *dst __attribute__((pass_object_size(1))), const void *src, size_t c) __attribute__((overloadable)) __asm__("merp");
-static void *memcpy(void *const dst __attribute__((pass_object_size(1))), const void *src, size_t c) __attribute__((overloadable)) {
-  return 0;
-}
-#elif defined(USE_BUILTINS)
+#if defined(USE_BUILTINS)
 #define memcpy(x,y,z) __builtin_memcpy(x,y,z)
 #else
 void *memcpy(void *dst, const void *src, size_t c);
@@ -45,14 +37,7 @@
   };
   struct pair p;
   char buf[20];
-  memcpy(, buf, 20);
-#ifdef USE_PASS_OBJECT_SIZE
-  // Use the more strict checking mode on the pass_object_size attribute:
-  // expected-warning@-3 {{memcpy' will always overflow; destination buffer has size 4, but size argument is 20}}
-#else
-  // Or just fallback to type 0:
-  // expected-warning@-6 {{memcpy' will always overflow; destination buffer has size 8, but size argument is 20}}
-#endif
+  memcpy(, buf, 20); // expected-warning {{memcpy' will always overflow; destination buffer has size 8, but size argument is 20}}
 }
 
 void call_strncat() {
Index: clang/test/Sema/implicit-builtin-decl.c
===
--- clang/test/Sema/implicit-builtin-decl.c
+++ clang/test/Sema/implicit-builtin-decl.c
@@ -1,5 +1,4 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
-// RUN: not %clang_cc1 -fsyntax-only -ast-dump %s | FileCheck %s
 
 void f() {
   int *ptr = malloc(sizeof(int) * 10); // expected-warning{{implicitly declaring library function 'malloc' with type}} \
@@ -63,9 +62,5 @@
 struct __jmp_buf_tag {};
 void

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

2020-09-14 Thread Raul Tambre via Phabricator via cfe-commits 
tambre added a comment.

If there are no further comments I'll commit this in a few days.




Comment at: clang/lib/Sema/SemaDecl.cpp:2088
+
+  if (Error || R.isNull())
+return nullptr;

aaron.ballman wrote:
> Should we do this check *before* we create the C linkage decl spec above?
We should. However, since now only `LazilyCreateBuiltin()` calls this and 
checks these before, we can simply removed them here and pass in the `QualType`.



Comment at: clang/lib/Sema/SemaDecl.cpp:9689
+   Context.getTargetInfo().getCXXABI().isMicrosoft()) {
+  NewFD->addAttr(BuiltinAttr::CreateImplicit(Context, BuiltinID));
+}

rsmith wrote:
> Please can you add a `// FIXME` here that we should probably only recognize 
> this as a builtin in the scope where the MS headers actually declare it, 
> rather than in every scope.
Added.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77491

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

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

2020-09-14 Thread Raul Tambre via Phabricator via cfe-commits 
tambre updated this revision to Diff 291778.
tambre marked 2 inline comments as done.
tambre added a comment.

Address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77491

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Basic/IdentifierTable.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Decl.cpp
  clang/lib/Headers/intrin.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/AST/ast-dump-attr.cpp
  clang/test/CodeGen/builtin-redeclaration.c
  clang/test/CodeGen/callback_pthread_create.c
  clang/test/CodeGenCXX/builtins.cpp
  clang/test/Sema/implicit-builtin-decl.c
  clang/test/Sema/warn-fortify-source.c
  clang/test/SemaCXX/cxx11-compat.cpp
  clang/test/SemaCXX/warn-unused-local-typedef.cpp

Index: clang/test/SemaCXX/warn-unused-local-typedef.cpp
===
--- clang/test/SemaCXX/warn-unused-local-typedef.cpp
+++ clang/test/SemaCXX/warn-unused-local-typedef.cpp
@@ -67,10 +67,10 @@
 
 void test() {
   typedef signed long int superint; // no diag
-  printf("%f", (superint) 42);
+  printf("%ld", (superint)42);
 
   typedef signed long int superint2; // no diag
-  printf("%f", static_cast(42));
+  printf("%ld", static_cast(42));
 
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wunused-local-typedef"
Index: clang/test/SemaCXX/cxx11-compat.cpp
===
--- clang/test/SemaCXX/cxx11-compat.cpp
+++ clang/test/SemaCXX/cxx11-compat.cpp
@@ -31,7 +31,7 @@
 s = { n }, // expected-warning {{non-constant-expression cannot be narrowed from type 'int' to 'char' in initializer list in C++11}} expected-note {{explicit cast}}
 t = { 1234 }; // expected-warning {{constant expression evaluates to 1234 which cannot be narrowed to type 'char' in C++11}} expected-warning {{changes value}} expected-note {{explicit cast}}
 
-#define PRIuS "uS"
+#define PRIuS "zu"
 int printf(const char *, ...);
 typedef __typeof(sizeof(int)) size_t;
 void h(size_t foo, size_t bar) {
Index: clang/test/Sema/warn-fortify-source.c
===
--- clang/test/Sema/warn-fortify-source.c
+++ clang/test/Sema/warn-fortify-source.c
@@ -1,8 +1,6 @@
 // RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify
-// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_PASS_OBJECT_SIZE
 // RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_BUILTINS
 // RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify
-// RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_PASS_OBJECT_SIZE
 // RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_BUILTINS
 
 typedef unsigned long size_t;
@@ -13,13 +11,7 @@
 
 extern int sprintf(char *str, const char *format, ...);
 
-#if defined(USE_PASS_OBJECT_SIZE)
-void *memcpy(void *dst, const void *src, size_t c);
-static void *memcpy(void *dst __attribute__((pass_object_size(1))), const void *src, size_t c) __attribute__((overloadable)) __asm__("merp");
-static void *memcpy(void *const dst __attribute__((pass_object_size(1))), const void *src, size_t c) __attribute__((overloadable)) {
-  return 0;
-}
-#elif defined(USE_BUILTINS)
+#if defined(USE_BUILTINS)
 #define memcpy(x,y,z) __builtin_memcpy(x,y,z)
 #else
 void *memcpy(void *dst, const void *src, size_t c);
@@ -45,14 +37,7 @@
   };
   struct pair p;
   char buf[20];
-  memcpy(, buf, 20);
-#ifdef USE_PASS_OBJECT_SIZE
-  // Use the more strict checking mode on the pass_object_size attribute:
-  // expected-warning@-3 {{memcpy' will always overflow; destination buffer has size 4, but size argument is 20}}
-#else
-  // Or just fallback to type 0:
-  // expected-warning@-6 {{memcpy' will always overflow; destination buffer has size 8, but size argument is 20}}
-#endif
+  memcpy(, buf, 20); // expected-warning {{memcpy' will always overflow; destination buffer has size 8, but size argument is 20}}
 }
 
 void call_strncat() {
Index: clang/test/Sema/implicit-builtin-decl.c
===
--- clang/test/Sema/implicit-builtin-decl.c
+++ clang/test/Sema/implicit-builtin-decl.c
@@ -1,5 +1,4 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
-// RUN: not %clang_cc1 -fsyntax-only -ast-dump %s | FileCheck %s
 
 void f() {
   int *ptr = malloc(sizeof(int) * 10); // expected-warning{{implicitly declaring library function 'malloc' with type}} \
@@ -63,9 +62,5 @@
 struct __jmp_buf_tag {};
 void sigsetjmp(struct __jmp_buf_tag[1], int); // expected-warning{{declaration of built-in function 'sigsetjmp' requires the declaration of the 'jmp_buf' type, commonly provided in the header .}}
 
-//

[PATCH] D86877: [Clang][Driver] Use full path to builtins in bare-metal toolchain

2020-09-11 Thread Raul Tambre via Phabricator via cfe-commits 
tambre abandoned this revision.
tambre added a comment.

In D86877#2262448 , @phosek wrote:

> It's not clear why couldn't we support per-target runtime directory? It uses 
> the standard multiarch layout, so in you'd end up using a directory like 
> `[path/to/resource-dir]/lib/armv6m-unknown-eabi/libclang_rt.builtins.a`. I'd 
> find this more preferable for consistency with other targets, it'd also 
> enable the use of runtimes build for baremetal.

Agreed. Unfortunately I'm unable to actually get the builtins built for 
baremetal and I originally encountered this issue under circumstances I can no 
longer reproduce.
Abandoning this as a result. Hopefully someone else will be able to take it up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86877

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

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

2020-09-11 Thread Raul Tambre via Phabricator via cfe-commits 
tambre added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77491

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

[PATCH] D87189: [CMake] Remove dead FindPythonInterp code

2020-09-08 Thread Raul Tambre via Phabricator via cfe-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG86bd8f82cc74: [CMake] Remove dead FindPythonInterp code 
(authored by tambre).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87189

Files:
  clang/CMakeLists.txt
  compiler-rt/CMakeLists.txt
  libcxx/CMakeLists.txt
  lld/CMakeLists.txt
  llvm/CMakeLists.txt

Index: llvm/CMakeLists.txt
===
--- llvm/CMakeLists.txt
+++ llvm/CMakeLists.txt
@@ -696,38 +696,19 @@
 
 include(HandleLLVMOptions)
 
-if(CMAKE_VERSION VERSION_LESS 3.12)
-  include(FindPythonInterp)
-  if( NOT PYTHONINTERP_FOUND )
-message(FATAL_ERROR
-  "Unable to find Python interpreter, required for builds and testing.
-
-  Please install Python or specify the PYTHON_EXECUTABLE CMake variable.")
-  endif()
-
-  if( ${PYTHON_VERSION_STRING} VERSION_LESS 2.7 )
-message(FATAL_ERROR "Python 2.7 or newer is required")
+find_package(Python3 COMPONENTS Interpreter)
+if(NOT Python3_Interpreter_FOUND)
+  message(WARNING "Python3 not found, using python2 as a fallback")
+  find_package(Python2 COMPONENTS Interpreter REQUIRED)
+  if(Python2_VERSION VERSION_LESS 2.7)
+message(SEND_ERROR "Python 2.7 or newer is required")
   endif()
 
+  # Treat python2 as python3
   add_executable(Python3::Interpreter IMPORTED)
   set_target_properties(Python3::Interpreter PROPERTIES
-IMPORTED_LOCATION ${PYTHON_EXECUTABLE})
-  set(Python3_EXECUTABLE ${PYTHON_EXECUTABLE})
-else()
-  find_package(Python3 COMPONENTS Interpreter)
-  if(NOT Python3_Interpreter_FOUND)
-message(WARNING "Python3 not found, using python2 as a fallback")
-find_package(Python2 COMPONENTS Interpreter REQUIRED)
-if(Python2_VERSION VERSION_LESS 2.7)
-  message(SEND_ERROR "Python 2.7 or newer is required")
-endif()
-
-# Treat python2 as python3
-add_executable(Python3::Interpreter IMPORTED)
-set_target_properties(Python3::Interpreter PROPERTIES
-  IMPORTED_LOCATION ${Python2_EXECUTABLE})
-set(Python3_EXECUTABLE ${Python2_EXECUTABLE})
-  endif()
+IMPORTED_LOCATION ${Python2_EXECUTABLE})
+  set(Python3_EXECUTABLE ${Python2_EXECUTABLE})
 endif()
 
 ##
Index: lld/CMakeLists.txt
===
--- lld/CMakeLists.txt
+++ lld/CMakeLists.txt
@@ -57,38 +57,19 @@
   include(CheckAtomic)
 
   if(LLVM_INCLUDE_TESTS)
-if(CMAKE_VERSION VERSION_LESS 3.12)
-  include(FindPythonInterp)
-  if(NOT PYTHONINTERP_FOUND)
-message(FATAL_ERROR
-  "Unable to find Python interpreter, required for testing.
-
-  Please install Python or specify the PYTHON_EXECUTABLE CMake variable.")
-  endif()
-
-  if(${PYTHON_VERSION_STRING} VERSION_LESS 2.7)
-message(FATAL_ERROR "Python 2.7 or newer is required")
+find_package(Python3 COMPONENTS Interpreter)
+if(NOT Python3_Interpreter_FOUND)
+  message(WARNING "Python3 not found, using python2 as a fallback")
+  find_package(Python2 COMPONENTS Interpreter REQUIRED)
+  if(Python2_VERSION VERSION_LESS 2.7)
+message(SEND_ERROR "Python 2.7 or newer is required")
   endif()
 
-  add_executable(Python3::Interpeter IMPORTED)
+  # Treat python2 as python3
+  add_executable(Python3::Interpreter IMPORTED)
   set_target_properties(Python3::Interpreter PROPERTIES
-IMPORTED_LOCATION ${PYTHON_EXECUTABLE})
-  set(Python3_EXECUTABLE ${PYTHON_EXECUTABLE})
-else()
-  find_package(Python3 COMPONENTS Interpreter)
-  if(NOT Python3_Interpreter_FOUND)
-message(WARNING "Python3 not found, using python2 as a fallback")
-find_package(Python2 COMPONENTS Interpreter REQUIRED)
-if(Python2_VERSION VERSION_LESS 2.7)
-  message(SEND_ERROR "Python 2.7 or newer is required")
-endif()
-
-# Treat python2 as python3
-add_executable(Python3::Interpreter IMPORTED)
-set_target_properties(Python3::Interpreter PROPERTIES
-  IMPORTED_LOCATION ${Python2_EXECUTABLE})
-set(Python3_EXECUTABLE ${Python2_EXECUTABLE})
-  endif()
+IMPORTED_LOCATION ${Python2_EXECUTABLE})
+  set(Python3_EXECUTABLE ${Python2_EXECUTABLE})
 endif()
 
 # Check prebuilt llvm/utils.
Index: libcxx/CMakeLists.txt
===
--- libcxx/CMakeLists.txt
+++ libcxx/CMakeLists.txt
@@ -41,33 +41,19 @@
 endif()
 
 if (LIBCXX_STANDALONE_BUILD)
-  if(CMAKE_VERSION VERSION_LESS 3.12)
-include(FindPythonInterp)
-if( NOT PYTHONINTERP_FOUND )
-  message(WARNING "Failed to find python interpreter. "
-  "The libc++ test suite will be disabled.")
-  set(LLVM_INCLUDE_TESTS OFF)
-else()
-  add_executable(Python3::Interpreter IMPORTED)
-

[PATCH] D86877: [Clang][Driver] Use full path to builtins in bare-metal toolchain

2020-09-07 Thread Raul Tambre via Phabricator via cfe-commits 
tambre added a comment.

phosek: ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86877

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

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

2020-09-07 Thread Raul Tambre via Phabricator via cfe-commits 
tambre added a comment.

Thanks for the review. All tests still pass, should be good for another round.




Comment at: clang/lib/Sema/SemaDecl.cpp:9672-9673
+  if (unsigned BuiltinID = II->getBuiltinID()) {
+const auto *LinkageDecl =
+dyn_cast(NewFD->getDeclContext());
+

rsmith wrote:
> This will give the wrong answer for
> ```
> extern "C" {
> namespace X {
> void __builtin_foo();
> }
> }
> ```
> ... which does have C language linkage. Instead, please call 
> `FunctionDecl::getLanguageLinkage()`, which knows how to handle these cases.
Good suggestion. This fixes the long-standing FIXME inherited from 
`getBuiltinID()`. I've added a test for this.



Comment at: clang/lib/Serialization/ASTReader.cpp:914
+  return II.hadMacroDefinition() || II.isPoisoned() ||
+ II.getObjCOrBuiltinID() || II.hasRevertedTokenIDToIdentifier() ||
  (!(IsModule && Reader.getPreprocessor().getLangOpts().CPlusPlus) &&

rsmith wrote:
> We now consider `getObjCOrBuiltinID()` here for the `IsModule` case, where we 
> didn't before. Is that an intentional change?
Unintentional, fixed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77491

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

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

2020-09-07 Thread Raul Tambre via Phabricator via cfe-commits 
tambre updated this revision to Diff 290232.
tambre marked an inline comment as done.
tambre added a comment.

Remove now obsolete FIXME.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77491

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Basic/IdentifierTable.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Decl.cpp
  clang/lib/Headers/intrin.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/AST/ast-dump-attr.cpp
  clang/test/CodeGen/builtin-redeclaration.c
  clang/test/CodeGen/callback_pthread_create.c
  clang/test/CodeGenCXX/builtins.cpp
  clang/test/Sema/implicit-builtin-decl.c
  clang/test/Sema/warn-fortify-source.c
  clang/test/SemaCXX/cxx11-compat.cpp
  clang/test/SemaCXX/warn-unused-local-typedef.cpp

Index: clang/test/SemaCXX/warn-unused-local-typedef.cpp
===
--- clang/test/SemaCXX/warn-unused-local-typedef.cpp
+++ clang/test/SemaCXX/warn-unused-local-typedef.cpp
@@ -67,10 +67,10 @@
 
 void test() {
   typedef signed long int superint; // no diag
-  printf("%f", (superint) 42);
+  printf("%ld", (superint)42);
 
   typedef signed long int superint2; // no diag
-  printf("%f", static_cast(42));
+  printf("%ld", static_cast(42));
 
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wunused-local-typedef"
Index: clang/test/SemaCXX/cxx11-compat.cpp
===
--- clang/test/SemaCXX/cxx11-compat.cpp
+++ clang/test/SemaCXX/cxx11-compat.cpp
@@ -31,7 +31,7 @@
 s = { n }, // expected-warning {{non-constant-expression cannot be narrowed from type 'int' to 'char' in initializer list in C++11}} expected-note {{explicit cast}}
 t = { 1234 }; // expected-warning {{constant expression evaluates to 1234 which cannot be narrowed to type 'char' in C++11}} expected-warning {{changes value}} expected-note {{explicit cast}}
 
-#define PRIuS "uS"
+#define PRIuS "zu"
 int printf(const char *, ...);
 typedef __typeof(sizeof(int)) size_t;
 void h(size_t foo, size_t bar) {
Index: clang/test/Sema/warn-fortify-source.c
===
--- clang/test/Sema/warn-fortify-source.c
+++ clang/test/Sema/warn-fortify-source.c
@@ -1,8 +1,6 @@
 // RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify
-// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_PASS_OBJECT_SIZE
 // RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_BUILTINS
 // RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify
-// RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_PASS_OBJECT_SIZE
 // RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_BUILTINS
 
 typedef unsigned long size_t;
@@ -13,13 +11,7 @@
 
 extern int sprintf(char *str, const char *format, ...);
 
-#if defined(USE_PASS_OBJECT_SIZE)
-void *memcpy(void *dst, const void *src, size_t c);
-static void *memcpy(void *dst __attribute__((pass_object_size(1))), const void *src, size_t c) __attribute__((overloadable)) __asm__("merp");
-static void *memcpy(void *const dst __attribute__((pass_object_size(1))), const void *src, size_t c) __attribute__((overloadable)) {
-  return 0;
-}
-#elif defined(USE_BUILTINS)
+#if defined(USE_BUILTINS)
 #define memcpy(x,y,z) __builtin_memcpy(x,y,z)
 #else
 void *memcpy(void *dst, const void *src, size_t c);
@@ -45,14 +37,7 @@
   };
   struct pair p;
   char buf[20];
-  memcpy(, buf, 20);
-#ifdef USE_PASS_OBJECT_SIZE
-  // Use the more strict checking mode on the pass_object_size attribute:
-  // expected-warning@-3 {{memcpy' will always overflow; destination buffer has size 4, but size argument is 20}}
-#else
-  // Or just fallback to type 0:
-  // expected-warning@-6 {{memcpy' will always overflow; destination buffer has size 8, but size argument is 20}}
-#endif
+  memcpy(, buf, 20); // expected-warning {{memcpy' will always overflow; destination buffer has size 8, but size argument is 20}}
 }
 
 void call_strncat() {
Index: clang/test/Sema/implicit-builtin-decl.c
===
--- clang/test/Sema/implicit-builtin-decl.c
+++ clang/test/Sema/implicit-builtin-decl.c
@@ -1,5 +1,4 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
-// RUN: not %clang_cc1 -fsyntax-only -ast-dump %s | FileCheck %s
 
 void f() {
   int *ptr = malloc(sizeof(int) * 10); // expected-warning{{implicitly declaring library function 'malloc' with type}} \
@@ -63,9 +62,5 @@
 struct __jmp_buf_tag {};
 void sigsetjmp(struct __jmp_buf_tag[1], int); // expected-warning{{declaration of built-in function 'sigsetjmp' requires the declaration of the 'jmp_buf' type, commonly provided in the header

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

2020-09-07 Thread Raul Tambre via Phabricator via cfe-commits 
tambre updated this revision to Diff 290229.
tambre marked 3 inline comments as done.
tambre added a comment.

Address review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77491

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Basic/IdentifierTable.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Decl.cpp
  clang/lib/Headers/intrin.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/AST/ast-dump-attr.cpp
  clang/test/CodeGen/builtin-redeclaration.c
  clang/test/CodeGen/callback_pthread_create.c
  clang/test/CodeGenCXX/builtins.cpp
  clang/test/Sema/implicit-builtin-decl.c
  clang/test/Sema/warn-fortify-source.c
  clang/test/SemaCXX/cxx11-compat.cpp
  clang/test/SemaCXX/warn-unused-local-typedef.cpp

Index: clang/test/SemaCXX/warn-unused-local-typedef.cpp
===
--- clang/test/SemaCXX/warn-unused-local-typedef.cpp
+++ clang/test/SemaCXX/warn-unused-local-typedef.cpp
@@ -67,10 +67,10 @@
 
 void test() {
   typedef signed long int superint; // no diag
-  printf("%f", (superint) 42);
+  printf("%ld", (superint)42);
 
   typedef signed long int superint2; // no diag
-  printf("%f", static_cast(42));
+  printf("%ld", static_cast(42));
 
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wunused-local-typedef"
Index: clang/test/SemaCXX/cxx11-compat.cpp
===
--- clang/test/SemaCXX/cxx11-compat.cpp
+++ clang/test/SemaCXX/cxx11-compat.cpp
@@ -31,7 +31,7 @@
 s = { n }, // expected-warning {{non-constant-expression cannot be narrowed from type 'int' to 'char' in initializer list in C++11}} expected-note {{explicit cast}}
 t = { 1234 }; // expected-warning {{constant expression evaluates to 1234 which cannot be narrowed to type 'char' in C++11}} expected-warning {{changes value}} expected-note {{explicit cast}}
 
-#define PRIuS "uS"
+#define PRIuS "zu"
 int printf(const char *, ...);
 typedef __typeof(sizeof(int)) size_t;
 void h(size_t foo, size_t bar) {
Index: clang/test/Sema/warn-fortify-source.c
===
--- clang/test/Sema/warn-fortify-source.c
+++ clang/test/Sema/warn-fortify-source.c
@@ -1,8 +1,6 @@
 // RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify
-// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_PASS_OBJECT_SIZE
 // RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_BUILTINS
 // RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify
-// RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_PASS_OBJECT_SIZE
 // RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_BUILTINS
 
 typedef unsigned long size_t;
@@ -13,13 +11,7 @@
 
 extern int sprintf(char *str, const char *format, ...);
 
-#if defined(USE_PASS_OBJECT_SIZE)
-void *memcpy(void *dst, const void *src, size_t c);
-static void *memcpy(void *dst __attribute__((pass_object_size(1))), const void *src, size_t c) __attribute__((overloadable)) __asm__("merp");
-static void *memcpy(void *const dst __attribute__((pass_object_size(1))), const void *src, size_t c) __attribute__((overloadable)) {
-  return 0;
-}
-#elif defined(USE_BUILTINS)
+#if defined(USE_BUILTINS)
 #define memcpy(x,y,z) __builtin_memcpy(x,y,z)
 #else
 void *memcpy(void *dst, const void *src, size_t c);
@@ -45,14 +37,7 @@
   };
   struct pair p;
   char buf[20];
-  memcpy(, buf, 20);
-#ifdef USE_PASS_OBJECT_SIZE
-  // Use the more strict checking mode on the pass_object_size attribute:
-  // expected-warning@-3 {{memcpy' will always overflow; destination buffer has size 4, but size argument is 20}}
-#else
-  // Or just fallback to type 0:
-  // expected-warning@-6 {{memcpy' will always overflow; destination buffer has size 8, but size argument is 20}}
-#endif
+  memcpy(, buf, 20); // expected-warning {{memcpy' will always overflow; destination buffer has size 8, but size argument is 20}}
 }
 
 void call_strncat() {
Index: clang/test/Sema/implicit-builtin-decl.c
===
--- clang/test/Sema/implicit-builtin-decl.c
+++ clang/test/Sema/implicit-builtin-decl.c
@@ -1,5 +1,4 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
-// RUN: not %clang_cc1 -fsyntax-only -ast-dump %s | FileCheck %s
 
 void f() {
   int *ptr = malloc(sizeof(int) * 10); // expected-warning{{implicitly declaring library function 'malloc' with type}} \
@@ -63,9 +62,5 @@
 struct __jmp_buf_tag {};
 void sigsetjmp(struct __jmp_buf_tag[1], int); // expected-warning{{declaration of built-in function 'sigsetjmp' requires the declaration of the 'jmp_buf' type, commonly provided in the header .}}

[PATCH] D87189: [CMake] Remove dead FindPythonInterp code

2020-09-05 Thread Raul Tambre via Phabricator via cfe-commits 
tambre created this revision.
tambre added reviewers: compnerd, phosek.
Herald added subscribers: llvm-commits, libcxx-commits, Sanitizers, 
cfe-commits, mgorny.
Herald added projects: clang, Sanitizers, libc++, LLVM.
Herald added a reviewer: libc++.
tambre requested review of this revision.

LLVM has bumped the minimum required CMake version to 3.13.4, so this has 
become dead code.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87189

Files:
  clang/CMakeLists.txt
  compiler-rt/CMakeLists.txt
  libcxx/CMakeLists.txt
  lld/CMakeLists.txt
  llvm/CMakeLists.txt

Index: llvm/CMakeLists.txt
===
--- llvm/CMakeLists.txt
+++ llvm/CMakeLists.txt
@@ -696,38 +696,19 @@
 
 include(HandleLLVMOptions)
 
-if(CMAKE_VERSION VERSION_LESS 3.12)
-  include(FindPythonInterp)
-  if( NOT PYTHONINTERP_FOUND )
-message(FATAL_ERROR
-  "Unable to find Python interpreter, required for builds and testing.
-
-  Please install Python or specify the PYTHON_EXECUTABLE CMake variable.")
-  endif()
-
-  if( ${PYTHON_VERSION_STRING} VERSION_LESS 2.7 )
-message(FATAL_ERROR "Python 2.7 or newer is required")
+find_package(Python3 COMPONENTS Interpreter)
+if(NOT Python3_Interpreter_FOUND)
+  message(WARNING "Python3 not found, using python2 as a fallback")
+  find_package(Python2 COMPONENTS Interpreter REQUIRED)
+  if(Python2_VERSION VERSION_LESS 2.7)
+message(SEND_ERROR "Python 2.7 or newer is required")
   endif()
 
+  # Treat python2 as python3
   add_executable(Python3::Interpreter IMPORTED)
   set_target_properties(Python3::Interpreter PROPERTIES
-IMPORTED_LOCATION ${PYTHON_EXECUTABLE})
-  set(Python3_EXECUTABLE ${PYTHON_EXECUTABLE})
-else()
-  find_package(Python3 COMPONENTS Interpreter)
-  if(NOT Python3_Interpreter_FOUND)
-message(WARNING "Python3 not found, using python2 as a fallback")
-find_package(Python2 COMPONENTS Interpreter REQUIRED)
-if(Python2_VERSION VERSION_LESS 2.7)
-  message(SEND_ERROR "Python 2.7 or newer is required")
-endif()
-
-# Treat python2 as python3
-add_executable(Python3::Interpreter IMPORTED)
-set_target_properties(Python3::Interpreter PROPERTIES
-  IMPORTED_LOCATION ${Python2_EXECUTABLE})
-set(Python3_EXECUTABLE ${Python2_EXECUTABLE})
-  endif()
+IMPORTED_LOCATION ${Python2_EXECUTABLE})
+  set(Python3_EXECUTABLE ${Python2_EXECUTABLE})
 endif()
 
 ##
Index: lld/CMakeLists.txt
===
--- lld/CMakeLists.txt
+++ lld/CMakeLists.txt
@@ -57,38 +57,19 @@
   include(CheckAtomic)
 
   if(LLVM_INCLUDE_TESTS)
-if(CMAKE_VERSION VERSION_LESS 3.12)
-  include(FindPythonInterp)
-  if(NOT PYTHONINTERP_FOUND)
-message(FATAL_ERROR
-  "Unable to find Python interpreter, required for testing.
-
-  Please install Python or specify the PYTHON_EXECUTABLE CMake variable.")
-  endif()
-
-  if(${PYTHON_VERSION_STRING} VERSION_LESS 2.7)
-message(FATAL_ERROR "Python 2.7 or newer is required")
+find_package(Python3 COMPONENTS Interpreter)
+if(NOT Python3_Interpreter_FOUND)
+  message(WARNING "Python3 not found, using python2 as a fallback")
+  find_package(Python2 COMPONENTS Interpreter REQUIRED)
+  if(Python2_VERSION VERSION_LESS 2.7)
+message(SEND_ERROR "Python 2.7 or newer is required")
   endif()
 
-  add_executable(Python3::Interpeter IMPORTED)
+  # Treat python2 as python3
+  add_executable(Python3::Interpreter IMPORTED)
   set_target_properties(Python3::Interpreter PROPERTIES
-IMPORTED_LOCATION ${PYTHON_EXECUTABLE})
-  set(Python3_EXECUTABLE ${PYTHON_EXECUTABLE})
-else()
-  find_package(Python3 COMPONENTS Interpreter)
-  if(NOT Python3_Interpreter_FOUND)
-message(WARNING "Python3 not found, using python2 as a fallback")
-find_package(Python2 COMPONENTS Interpreter REQUIRED)
-if(Python2_VERSION VERSION_LESS 2.7)
-  message(SEND_ERROR "Python 2.7 or newer is required")
-endif()
-
-# Treat python2 as python3
-add_executable(Python3::Interpreter IMPORTED)
-set_target_properties(Python3::Interpreter PROPERTIES
-  IMPORTED_LOCATION ${Python2_EXECUTABLE})
-set(Python3_EXECUTABLE ${Python2_EXECUTABLE})
-  endif()
+IMPORTED_LOCATION ${Python2_EXECUTABLE})
+  set(Python3_EXECUTABLE ${Python2_EXECUTABLE})
 endif()
 
 # Check prebuilt llvm/utils.
Index: libcxx/CMakeLists.txt
===
--- libcxx/CMakeLists.txt
+++ libcxx/CMakeLists.txt
@@ -41,33 +41,19 @@
 endif()
 
 if (LIBCXX_STANDALONE_BUILD)
-  if(CMAKE_VERSION VERSION_LESS 3.12)
-include(FindPythonInterp)
-if( NOT PYTHONINTERP_FOUND )
-  message(WARNING "Failed to find python interpreter. "
-  "The libc++ test suite will be disabled.")
-  set(LLVM_INCLUDE_TESTS OFF)
-else()
-

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

2020-09-04 Thread Raul Tambre via Phabricator via cfe-commits 
tambre added a comment.

In D77491#2254967 , @rjmccall wrote:

> I didn't see the specific example, sorry.  I agree that my description is 
> more applicable to builtins in the `__builtin` namespace, which describes 
> most of the builtins with custom typechecking.  Is the problem just 
> `__va_start`?

I grepped for all builtins with custom typechecking.
I found two occurences in MSVC's headers: `__va_start` and 
`__GetExceptionInfo`, but none on my Debian Unstable machine.

> If we have to treat all declarations as builtins for the custom-typechecking 
> builtins just to land this patch, I don't think that's the worst result in 
> the world, and we can incrementally go from there.  `__va_start` actually has 
> a signature, it just effectively has optional arguments, which is something 
> we could definitely teach the builtins database and signature-matcher about.

I'd still prefer we go incrementally.

It would be nice to receive another round of code review after having fixed the 
issues pointed out by Richard.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77491

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

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

2020-09-04 Thread Raul Tambre via Phabricator via cfe-commits 
tambre updated this revision to Diff 289986.
tambre added a comment.

Improve comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77491

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Basic/IdentifierTable.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Decl.cpp
  clang/lib/Headers/intrin.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/AST/ast-dump-attr.cpp
  clang/test/CodeGen/builtin-redeclaration.c
  clang/test/CodeGen/callback_pthread_create.c
  clang/test/Sema/implicit-builtin-decl.c
  clang/test/Sema/warn-fortify-source.c
  clang/test/SemaCXX/cxx11-compat.cpp
  clang/test/SemaCXX/warn-unused-local-typedef.cpp

Index: clang/test/SemaCXX/warn-unused-local-typedef.cpp
===
--- clang/test/SemaCXX/warn-unused-local-typedef.cpp
+++ clang/test/SemaCXX/warn-unused-local-typedef.cpp
@@ -67,10 +67,10 @@
 
 void test() {
   typedef signed long int superint; // no diag
-  printf("%f", (superint) 42);
+  printf("%ld", (superint)42);
 
   typedef signed long int superint2; // no diag
-  printf("%f", static_cast(42));
+  printf("%ld", static_cast(42));
 
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wunused-local-typedef"
Index: clang/test/SemaCXX/cxx11-compat.cpp
===
--- clang/test/SemaCXX/cxx11-compat.cpp
+++ clang/test/SemaCXX/cxx11-compat.cpp
@@ -31,7 +31,7 @@
 s = { n }, // expected-warning {{non-constant-expression cannot be narrowed from type 'int' to 'char' in initializer list in C++11}} expected-note {{explicit cast}}
 t = { 1234 }; // expected-warning {{constant expression evaluates to 1234 which cannot be narrowed to type 'char' in C++11}} expected-warning {{changes value}} expected-note {{explicit cast}}
 
-#define PRIuS "uS"
+#define PRIuS "zu"
 int printf(const char *, ...);
 typedef __typeof(sizeof(int)) size_t;
 void h(size_t foo, size_t bar) {
Index: clang/test/Sema/warn-fortify-source.c
===
--- clang/test/Sema/warn-fortify-source.c
+++ clang/test/Sema/warn-fortify-source.c
@@ -1,8 +1,6 @@
 // RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify
-// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_PASS_OBJECT_SIZE
 // RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_BUILTINS
 // RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify
-// RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_PASS_OBJECT_SIZE
 // RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_BUILTINS
 
 typedef unsigned long size_t;
@@ -13,13 +11,7 @@
 
 extern int sprintf(char *str, const char *format, ...);
 
-#if defined(USE_PASS_OBJECT_SIZE)
-void *memcpy(void *dst, const void *src, size_t c);
-static void *memcpy(void *dst __attribute__((pass_object_size(1))), const void *src, size_t c) __attribute__((overloadable)) __asm__("merp");
-static void *memcpy(void *const dst __attribute__((pass_object_size(1))), const void *src, size_t c) __attribute__((overloadable)) {
-  return 0;
-}
-#elif defined(USE_BUILTINS)
+#if defined(USE_BUILTINS)
 #define memcpy(x,y,z) __builtin_memcpy(x,y,z)
 #else
 void *memcpy(void *dst, const void *src, size_t c);
@@ -45,14 +37,7 @@
   };
   struct pair p;
   char buf[20];
-  memcpy(, buf, 20);
-#ifdef USE_PASS_OBJECT_SIZE
-  // Use the more strict checking mode on the pass_object_size attribute:
-  // expected-warning@-3 {{memcpy' will always overflow; destination buffer has size 4, but size argument is 20}}
-#else
-  // Or just fallback to type 0:
-  // expected-warning@-6 {{memcpy' will always overflow; destination buffer has size 8, but size argument is 20}}
-#endif
+  memcpy(, buf, 20); // expected-warning {{memcpy' will always overflow; destination buffer has size 8, but size argument is 20}}
 }
 
 void call_strncat() {
Index: clang/test/Sema/implicit-builtin-decl.c
===
--- clang/test/Sema/implicit-builtin-decl.c
+++ clang/test/Sema/implicit-builtin-decl.c
@@ -1,5 +1,4 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
-// RUN: not %clang_cc1 -fsyntax-only -ast-dump %s | FileCheck %s
 
 void f() {
   int *ptr = malloc(sizeof(int) * 10); // expected-warning{{implicitly declaring library function 'malloc' with type}} \
@@ -63,9 +62,5 @@
 struct __jmp_buf_tag {};
 void sigsetjmp(struct __jmp_buf_tag[1], int); // expected-warning{{declaration of built-in function 'sigsetjmp' requires the declaration of the 'jmp_buf' type, commonly provided in the header .}}
 
-// CHECK: FunctionDecl {{.*}}  col:6 sigsetjmp '
-// CHECK-NOT: FunctionDecl

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

2020-09-03 Thread Raul Tambre via Phabricator via cfe-commits 
tambre added a comment.

In D77491#2254065 , @rjmccall wrote:

> The builtins with custom type-checking are all true intrinsics like 
> `__builtin_operator_new` and so on.  They really can't be validly declared by 
> the user program.  The thing that seems most likely to avoid random compiler 
> crashes would be to either forbid explicit declarations of them or treat 
> those as no longer being builtins.  If we need to maintain compatibility with 
> people making custom declarations, we would need to always treat them as 
> builtins and run the risk of crashing if someone declares one with a bad 
> signature.  But I don't think it's unfair of us to break those people; that 
> is seriously not reasonable user behavior.
>
> It's possible that custom declarations are people trying to create 
> compatibility shims for compilers that don't provide these as builtins.  
> Those people should be guarding their custom declarations, preferably with 
> `__has_builtin`.

I fully agree.

However, I believe you forget to account for the example that I brought up.
In particular, MSVC's header `vadefs.h` includes a declaration of 
`__va_start()`, which would cause almost any code including standard headers to 
fail to compile on Windows.
This issue isn't isolated to some old MSVC version, as the declaration is still 
there in the latest Visual Studio Preview version 14.28.29213.

How about turning it into an error only on non-Windows?
Though keeping that as a followup might be even better, as this will probably 
be merged into 11.0.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77491

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

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

2020-09-02 Thread Raul Tambre via Phabricator via cfe-commits 
tambre added a comment.

Thanks for the review!
I believe I've managed to address your comments.

In D77491#2248454 , @rsmith wrote:

> What happens for builtins with the "t" (custom typechecking) flag, for which 
> the signature is intended to have no meaning? Do we always give them builtin 
> semantics, or never, or ... something else? I think it might be reasonable to 
> require them to always be declared as taking unspecified arguments -- `()` in 
> C and `(...)` in C++, or to simply say that the user cannot declare such 
> functions themselves.

The options I see are:

1. Disallow.
2. Run the custom typechecking.
3. Require as taking unspecified arguments.
4. Simply check the name.

Implementing running custom typechecking seems it'd be tricky and not worth it.
Disallowing isn't an option as a cursory search shows that this is used in the 
wild.
Requiring unspecified arguments isn't feasible either as the real-world usages 
seem to include arguments in their declarations. E.g. __va_start from MSVC's 
vadefs.h 
,
 which is also tested by `clang/test/SemaCXX/microsoft-varargs.cpp`.

I've thus gone with simply marking as them builtins if the name matches and 
they're `extern "C"`. The C linkage requirement didn't exist before, but seems 
a safe improvement to make.




Comment at: clang/lib/Headers/intrin.h:148
 void __writemsr(unsigned long, unsigned __int64);
-static __inline__
-void *_AddressOfReturnAddress(void);
-static __inline__
-unsigned char _BitScanForward(unsigned long *_Index, unsigned long _Mask);
-static __inline__
-unsigned char _BitScanReverse(unsigned long *_Index, unsigned long _Mask);
+__inline__ void *_AddressOfReturnAddress(void);
+__inline__ unsigned char _BitScanForward(unsigned long *_Index,

rsmith wrote:
> Does the `__inline__` here do anything for a builtin function? Can we remove 
> it along with the `static`?
It does not. Removed from all.



Comment at: clang/lib/Headers/intrin.h:200
 void __incgsword(unsigned long);
-static __inline__
-void __movsq(unsigned long long *, unsigned long long const *, size_t);
+static __inline__ void __movsq(unsigned long long *, unsigned long long const 
*,
+   size_t);

rsmith wrote:
> Why is `static` being removed from some of the functions in this header but 
> not others?
I removed `static` from builtins that showed up as problematic in tests. But 
like `inline`, there's really no effect. I've removed both from all builtins in 
this header.



Comment at: clang/lib/Sema/SemaDecl.cpp:9668-9694
+  // In C builtins get merged with implicitly lazily created declarations.
+  // In C++ we need to check if it's a builtin and add the BuiltinAttr here.
+  if (getLangOpts().CPlusPlus) {
+if (IdentifierInfo *II = Previous.getLookupName().getAsIdentifierInfo()) {
+  if (unsigned BuiltinID = II->getBuiltinID()) {
+FunctionDecl *D = CreateBuiltin(II, BuiltinID, NewFD->getLocation());
+

rsmith wrote:
> I think this needs more refinement:
> 
> * You appear to be creating and throwing away a new builtin function 
> declaration (plus parameter declarations etc) each time you see a declaration 
> with a matching name, even if one was already created. Given that you don't 
> actually use `D` for anything other than its type, creating the declaration 
> seems redundant and using `ASTContext::GetBuiltinType` would be more 
> appropriate.
> * There are no checks of which scope the new function is declared in; this 
> appears to apply in all scopes, but some builtin names are only reserved in 
> the global scope (those beginning with an underscore followed by a lowercase 
> letter such as `_bittest`), so that doesn't seem appropriate. The old code in 
> `FunctionDecl::getBuiltinID` checked that the declaration is given C language 
> linkage (except for `_GetExceptionInfo`, which was permitted to have C++ 
> language linkage), and that check still seems appropriate to me.
> * The special case permitting `_GetExceptionInfo` to be declared with *any* 
> type seems suspect; the old code permitted it to have different language 
> linkage, not the wrong type.
> * Using `typesAreCompatible` in C++-specific code is weird, since C++ doesn't 
> have a notion of "compatible types".
> * You appear to be creating and throwing away a new builtin function 
> declaration (plus parameter declarations etc) each time you see a declaration 
> with a matching name, even if one was already created. Given that you don't 
> actually use `D` for anything other than its type, creating the declaration 
> seems redundant and using `ASTContext::GetBuiltinType` would be more 
> appropriate.
Fixed.

> * There are no checks of which scope the new function is declared

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

2020-09-02 Thread Raul Tambre via Phabricator via cfe-commits 
tambre updated this revision to Diff 289631.
tambre marked 6 inline comments as done.
tambre added a comment.

Remove __inline__ and static from all builtins in intrin.h.
Remove ASTReader/ASTWriter placeholders.
During builtin recognition:

- Use ASTContext::GetBuiltinType() instead of creating a new function 
declaration every time.
- Check for C linkage.
- Handle builtins with custom typechecking.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77491

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Basic/IdentifierTable.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Decl.cpp
  clang/lib/Headers/intrin.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/AST/ast-dump-attr.cpp
  clang/test/CodeGen/builtin-redeclaration.c
  clang/test/CodeGen/callback_pthread_create.c
  clang/test/Sema/implicit-builtin-decl.c
  clang/test/Sema/warn-fortify-source.c
  clang/test/SemaCXX/cxx11-compat.cpp
  clang/test/SemaCXX/warn-unused-local-typedef.cpp

Index: clang/test/SemaCXX/warn-unused-local-typedef.cpp
===
--- clang/test/SemaCXX/warn-unused-local-typedef.cpp
+++ clang/test/SemaCXX/warn-unused-local-typedef.cpp
@@ -67,10 +67,10 @@
 
 void test() {
   typedef signed long int superint; // no diag
-  printf("%f", (superint) 42);
+  printf("%ld", (superint)42);
 
   typedef signed long int superint2; // no diag
-  printf("%f", static_cast(42));
+  printf("%ld", static_cast(42));
 
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wunused-local-typedef"
Index: clang/test/SemaCXX/cxx11-compat.cpp
===
--- clang/test/SemaCXX/cxx11-compat.cpp
+++ clang/test/SemaCXX/cxx11-compat.cpp
@@ -31,7 +31,7 @@
 s = { n }, // expected-warning {{non-constant-expression cannot be narrowed from type 'int' to 'char' in initializer list in C++11}} expected-note {{explicit cast}}
 t = { 1234 }; // expected-warning {{constant expression evaluates to 1234 which cannot be narrowed to type 'char' in C++11}} expected-warning {{changes value}} expected-note {{explicit cast}}
 
-#define PRIuS "uS"
+#define PRIuS "zu"
 int printf(const char *, ...);
 typedef __typeof(sizeof(int)) size_t;
 void h(size_t foo, size_t bar) {
Index: clang/test/Sema/warn-fortify-source.c
===
--- clang/test/Sema/warn-fortify-source.c
+++ clang/test/Sema/warn-fortify-source.c
@@ -1,8 +1,6 @@
 // RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify
-// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_PASS_OBJECT_SIZE
 // RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_BUILTINS
 // RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify
-// RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_PASS_OBJECT_SIZE
 // RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_BUILTINS
 
 typedef unsigned long size_t;
@@ -13,13 +11,7 @@
 
 extern int sprintf(char *str, const char *format, ...);
 
-#if defined(USE_PASS_OBJECT_SIZE)
-void *memcpy(void *dst, const void *src, size_t c);
-static void *memcpy(void *dst __attribute__((pass_object_size(1))), const void *src, size_t c) __attribute__((overloadable)) __asm__("merp");
-static void *memcpy(void *const dst __attribute__((pass_object_size(1))), const void *src, size_t c) __attribute__((overloadable)) {
-  return 0;
-}
-#elif defined(USE_BUILTINS)
+#if defined(USE_BUILTINS)
 #define memcpy(x,y,z) __builtin_memcpy(x,y,z)
 #else
 void *memcpy(void *dst, const void *src, size_t c);
@@ -45,14 +37,7 @@
   };
   struct pair p;
   char buf[20];
-  memcpy(, buf, 20);
-#ifdef USE_PASS_OBJECT_SIZE
-  // Use the more strict checking mode on the pass_object_size attribute:
-  // expected-warning@-3 {{memcpy' will always overflow; destination buffer has size 4, but size argument is 20}}
-#else
-  // Or just fallback to type 0:
-  // expected-warning@-6 {{memcpy' will always overflow; destination buffer has size 8, but size argument is 20}}
-#endif
+  memcpy(, buf, 20); // expected-warning {{memcpy' will always overflow; destination buffer has size 8, but size argument is 20}}
 }
 
 void call_strncat() {
Index: clang/test/Sema/implicit-builtin-decl.c
===
--- clang/test/Sema/implicit-builtin-decl.c
+++ clang/test/Sema/implicit-builtin-decl.c
@@ -1,5 +1,4 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
-// RUN: not %clang_cc1 -fsyntax-only -ast-dump %s | FileCheck %s
 
 void f() {
   int *ptr = malloc(sizeof(int) * 10); // expected-warning{{implicitly declaring library function 'malloc' with type}} \
@@ -63,9

[PATCH] D86877: [Clang][Driver] Use full path to builtins in bare-metal toolchain

2020-09-01 Thread Raul Tambre via Phabricator via cfe-commits 
tambre added a comment.

@phosek Please review again.
I've overhauled the patch as I realized that per-target runtime directories 
don't make sense for the bare-metal target, since the runtime is only 
distinguished by the //specific// architecture and nothing else.
As a result I've simply changed this into a cleanup doing the same thing as 
D59425 , which seems to have languished.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86877

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

[PATCH] D86877: [Clang][Driver] Support per-target runtime directories in the bare-metal toolchain

2020-09-01 Thread Raul Tambre via Phabricator via cfe-commits 
tambre updated this revision to Diff 289161.
tambre added a comment.

Rework patch to simply cleanup runtime handling in the bare-metal toolchain.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86877

Files:
  clang/lib/Driver/ToolChains/BareMetal.cpp
  clang/lib/Driver/ToolChains/BareMetal.h
  clang/test/Driver/arm-compiler-rt.c
  clang/test/Driver/baremetal.cpp

Index: clang/test/Driver/baremetal.cpp
===
--- clang/test/Driver/baremetal.cpp
+++ clang/test/Driver/baremetal.cpp
@@ -2,6 +2,7 @@
 // RUN: -target armv6m-none-eabi \
 // RUN: -T semihosted.lds \
 // RUN: -L some/directory/user/asked/for \
+// RUN: -resource-dir=%S/Inputs/resource_dir_baremetal \
 // RUN: --sysroot=%S/Inputs/baremetal_arm \
 // RUN:   | FileCheck --check-prefix=CHECK-V6M-C %s
 // CHECK-V6M-C: "[[PREFIX_DIR:.*]]{{[/\\]+}}{{[^/^\\]+}}{{[/\\]+}}clang{{.*}}" "-cc1" "-triple" "thumbv6m-none-unknown-eabi"
@@ -13,12 +14,14 @@
 // CHECK-V6M-C-NEXT: "{{[^"]*}}ld{{(\.(lld|bfd|gold))?}}{{(\.exe)?}}" "{{.*}}.o" "-Bstatic"
 // CHECK-V6M-C-SAME: "-L[[RESOURCE_DIR:[^"]+]]{{[/\\]+}}lib{{[/\\]+}}baremetal"
 // CHECK-V6M-C-SAME: "-T" "semihosted.lds" "-Lsome{{[/\\]+}}directory{{[/\\]+}}user{{[/\\]+}}asked{{[/\\]+}}for"
-// CHECK-V6M-C-SAME: "-lc" "-lm" "-lclang_rt.builtins-armv6m"
+// CHECK-V6M-C-SAME: "-lc" "-lm"
+// CHECK-V6M-C-SAME: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}libclang_rt.builtins-armv6m.a"
 // CHECK-V6M-C-SAME: "-o" "{{.*}}.o"
 
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: -target armv6m-none-eabi \
 // RUN: -nostdlibinc -nobuiltininc \
+// RUN: -resource-dir=%S/Inputs/resource_dir_baremetal \
 // RUN: --sysroot=%S/Inputs/baremetal_arm \
 // RUN:   | FileCheck --check-prefix=CHECK-V6M-LIBINC %s
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
@@ -30,38 +33,47 @@
 
 // RUN: %clangxx -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: -target armv6m-none-eabi \
+// RUN: -resource-dir=%S/Inputs/resource_dir_baremetal \
 // RUN: --sysroot=%S/Inputs/baremetal_arm \
 // RUN:   | FileCheck --check-prefix=CHECK-V6M-DEFAULTCXX %s
+// CHECK-V6M-DEFAULTCXX: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
 // CHECK-V6M-DEFAULTCXX: "{{[^"]*}}ld{{(\.(lld|bfd|gold))?}}{{(\.exe)?}}" "{{.*}}.o" "-Bstatic"
-// CHECK-V6M-DEFAULTCXX-SAME: "-L{{[^"]*}}{{[/\\]+}}lib{{(64)?}}{{[/\\]+}}clang{{[/\\]+}}{{.*}}{{[/\\]+}}lib{{[/\\]+}}baremetal"
+// CHECK-V6M-DEFAULTCXX-SAME: "-L[[RESOURCE_DIR:[^"]+]]{{[/\\]+}}lib{{[/\\]+}}baremetal"
 // CHECK-V6M-DEFAULTCXX-SAME: "-lc++" "-lc++abi" "-lunwind"
-// CHECK-V6M-DEFAULTCXX-SAME: "-lc" "-lm" "-lclang_rt.builtins-armv6m"
+// CHECK-V6M-DEFAULTCXX-SAME: "-lc" "-lm"
+// CHECK-V6M-DEFAULTCXX-SAME: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}libclang_rt.builtins-armv6m.a"
 // CHECK-V6M-DEFAULTCXX-SAME: "-o" "{{.*}}.o"
 
 // RUN: %clangxx -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: -target armv6m-none-eabi \
+// RUN: -resource-dir=%S/Inputs/resource_dir_baremetal \
 // RUN: --sysroot=%S/Inputs/baremetal_arm \
 // RUN: -stdlib=libc++ \
 // RUN:   | FileCheck --check-prefix=CHECK-V6M-LIBCXX %s
 // CHECK-V6M-LIBCXX-NOT: "-internal-isystem" "{{[^"]+}}{{[/\\]+}}include{{[/\\]+}}c++{{[/\\]+}}{{[^v].*}}"
-// CHECK-V6M-LIBCXX: "-internal-isystem" "{{[^"]+}}{{[/\\]+}}include{{[/\\]+}}c++{{[/\\]+}}v1"
+// CHECK-V6M-LIBCXX: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-V6M-LIBCXX-SAME: "-internal-isystem" "{{[^"]+}}{{[/\\]+}}include{{[/\\]+}}c++{{[/\\]+}}v1"
 // CHECK-V6M-LIBCXX: "{{[^"]*}}ld{{(\.(lld|bfd|gold))?}}{{(\.exe)?}}" "{{.*}}.o" "-Bstatic"
-// CHECK-V6M-LIBCXX-SAME: "-L{{[^"]*}}{{[/\\]+}}lib{{(64)?}}{{[/\\]+}}clang{{[/\\]+}}{{.*}}{{[/\\]+}}lib{{[/\\]+}}baremetal"
+// CHECK-V6M-LIBCXX-SAME: "-L[[RESOURCE_DIR:[^"]+]]{{[/\\]+}}lib{{[/\\]+}}baremetal"
 // CHECK-V6M-LIBCXX-SAME: "-lc++" "-lc++abi" "-lunwind"
-// CHECK-V6M-LIBCXX-SAME: "-lc" "-lm" "-lclang_rt.builtins-armv6m"
+// CHECK-V6M-LIBCXX-SAME: "-lc" "-lm"
+// CHECK-V6M-LIBCXX-SAME: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}libclang_rt.builtins-armv6m.a"
 // CHECK-V6M-LIBCXX-SAME: "-o" "{{.*}}.o"
 
 // RUN: %clangxx -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: -target armv6m-none-eabi \
+// RUN: -resource-dir=%S/Inputs/resource_dir_baremetal \
 // RUN: --sysroot=%S/Inputs/baremetal_arm \
 // RUN: -stdlib=libstdc++ \
 // RUN:   | FileCheck --check-prefix=CHECK-V6M-LIBSTDCXX %s
 // CHECK-V6M-LIBSTDCXX-NOT: "-internal-isystem" "{{[^"]+}}{{[/\\]+}}include{{[/\\]+}}c++{{[/\\]+}}v1"
-// CHECK-V6M-LIBSTDCXX: "-internal-isystem" "{{[^"]+}}{{[/\\]+}}include{{[/\\]+}}c++{{[/\\]+}}6.0.0"
+// CHECK-V6M-LIBSTDCXX: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-V6M-LIBSTDCXX-SAME: "-internal-isystem" "{{[^"]+}}{{[/\\]+}}include{{[/\\]+}}c++{{[/\\]+}}6.0.0"
 // CHECK-V6M-LIBSTDCXX:

[PATCH] D86877: [Clang][Driver] Support per-target runtime directories in the bare-metal toolchain

2020-09-01 Thread Raul Tambre via Phabricator via cfe-commits 
tambre added a comment.

In D86877#2248064 , @phosek wrote:

> This was discussed when `LLVM_ENABLE_PER_TARGET_RUNTIME_DIR` was introduced 
> and there was a pushback against changing the driver behavior depending on 
> the value of that option, so if we're going to reverse that decision for 
> BareMetal, I think that deserves a broader discussion.

Thanks for the background. I'd rather not go for a compile-time behaviour 
change in that case and have gone with using `getCompilerRTArgString()`. I'll 
do an internal toolchain build and do some testing before landing this.




Comment at: clang/lib/Driver/ToolChains/BareMetal.cpp:163
+  CmdArgs.push_back(
+  Args.MakeArgString("-lclang_rt.builtins-" + getTriple().getArchName()));
+#endif

phosek wrote:
> Is there a reason why BareMetal doesn't just use 
> https://github.com/llvm/llvm-project/blob/master/clang/lib/Driver/ToolChain.cpp#L462
>  like all other drivers, and instead re-implements the runtime library lookup 
> logic?
Was unaware of that function before, thanks!
I can't see a good reason not to use it, so I've switched it over.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86877

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

[PATCH] D86877: [Clang][Driver] Support per-target runtime directories in the bare-metal toolchain

2020-09-01 Thread Raul Tambre via Phabricator via cfe-commits 
tambre updated this revision to Diff 289085.
tambre added a comment.

Remove define to change driver behaviour, use 
ToolChain::getCompilerRTArgString() instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86877

Files:
  clang/lib/Driver/ToolChains/BareMetal.cpp


Index: clang/lib/Driver/ToolChains/BareMetal.cpp
===
--- clang/lib/Driver/ToolChains/BareMetal.cpp
+++ clang/lib/Driver/ToolChains/BareMetal.cpp
@@ -156,8 +156,7 @@
 
 void BareMetal::AddLinkRuntimeLib(const ArgList ,
   ArgStringList ) const {
-  CmdArgs.push_back(Args.MakeArgString("-lclang_rt.builtins-" +
-   getTriple().getArchName()));
+  CmdArgs.push_back(getCompilerRTArgString(Args, "builtins"));
 }
 
 void baremetal::Linker::ConstructJob(Compilation , const JobAction ,


Index: clang/lib/Driver/ToolChains/BareMetal.cpp
===
--- clang/lib/Driver/ToolChains/BareMetal.cpp
+++ clang/lib/Driver/ToolChains/BareMetal.cpp
@@ -156,8 +156,7 @@
 
 void BareMetal::AddLinkRuntimeLib(const ArgList ,
   ArgStringList ) const {
-  CmdArgs.push_back(Args.MakeArgString("-lclang_rt.builtins-" +
-   getTriple().getArchName()));
+  CmdArgs.push_back(getCompilerRTArgString(Args, "builtins"));
 }
 
 void baremetal::Linker::ConstructJob(Compilation , const JobAction ,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D86877: [Clang][Driver] Support per-target runtime directories in the bare-metal toolchain

2020-08-31 Thread Raul Tambre via Phabricator via cfe-commits 
tambre updated this revision to Diff 288956.
tambre added a comment.

Add missing dash.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86877

Files:
  clang/lib/Driver/CMakeLists.txt
  clang/lib/Driver/ToolChains/BareMetal.cpp


Index: clang/lib/Driver/ToolChains/BareMetal.cpp
===
--- clang/lib/Driver/ToolChains/BareMetal.cpp
+++ clang/lib/Driver/ToolChains/BareMetal.cpp
@@ -156,8 +156,12 @@
 
 void BareMetal::AddLinkRuntimeLib(const ArgList ,
   ArgStringList ) const {
-  CmdArgs.push_back(Args.MakeArgString("-lclang_rt.builtins-" +
-   getTriple().getArchName()));
+#ifdef PER_TARGET_RUNTIME_DIR
+  CmdArgs.push_back(Args.MakeArgString("-lclang_rt.builtins"));
+#else
+  CmdArgs.push_back(
+  Args.MakeArgString("-lclang_rt.builtins-" + getTriple().getArchName()));
+#endif
 }
 
 void baremetal::Linker::ConstructJob(Compilation , const JobAction ,
Index: clang/lib/Driver/CMakeLists.txt
===
--- clang/lib/Driver/CMakeLists.txt
+++ clang/lib/Driver/CMakeLists.txt
@@ -83,3 +83,7 @@
   clangBasic
   ${system_libs}
   )
+
+if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR)
+  set_source_files_properties(ToolChains/BareMetal.cpp PROPERTIES 
COMPILE_DEFINITIONS "PER_TARGET_RUNTIME_DIR")
+endif()


Index: clang/lib/Driver/ToolChains/BareMetal.cpp
===
--- clang/lib/Driver/ToolChains/BareMetal.cpp
+++ clang/lib/Driver/ToolChains/BareMetal.cpp
@@ -156,8 +156,12 @@
 
 void BareMetal::AddLinkRuntimeLib(const ArgList ,
   ArgStringList ) const {
-  CmdArgs.push_back(Args.MakeArgString("-lclang_rt.builtins-" +
-   getTriple().getArchName()));
+#ifdef PER_TARGET_RUNTIME_DIR
+  CmdArgs.push_back(Args.MakeArgString("-lclang_rt.builtins"));
+#else
+  CmdArgs.push_back(
+  Args.MakeArgString("-lclang_rt.builtins-" + getTriple().getArchName()));
+#endif
 }
 
 void baremetal::Linker::ConstructJob(Compilation , const JobAction ,
Index: clang/lib/Driver/CMakeLists.txt
===
--- clang/lib/Driver/CMakeLists.txt
+++ clang/lib/Driver/CMakeLists.txt
@@ -83,3 +83,7 @@
   clangBasic
   ${system_libs}
   )
+
+if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR)
+  set_source_files_properties(ToolChains/BareMetal.cpp PROPERTIES COMPILE_DEFINITIONS "PER_TARGET_RUNTIME_DIR")
+endif()
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D86877: [Clang][Driver] Support per-target runtime directories in the bare-metal toolchain

2020-08-31 Thread Raul Tambre via Phabricator via cfe-commits 
tambre created this revision.
tambre added a reviewer: phosek.
Herald added subscribers: cfe-commits, mgorny.
Herald added a project: clang.
tambre requested review of this revision.

When LLVM_ENABLE_PER_TARGET_RUNTIME_DIR is set to ON the clang_rt.builtins will
not have an architecture prefix and will be placed instead in an architecture
directory.
This causes issues with the bare-metal toolchain, which assumes the suffixed
variant.

Add a define dependent on LLVM_ENABLE_PER_TARGET_RUNTIME_DIR, which switches
the bare-metal toolchain to use the unsuffixed version if enabled.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86877

Files:
  clang/lib/Driver/CMakeLists.txt
  clang/lib/Driver/ToolChains/BareMetal.cpp


Index: clang/lib/Driver/ToolChains/BareMetal.cpp
===
--- clang/lib/Driver/ToolChains/BareMetal.cpp
+++ clang/lib/Driver/ToolChains/BareMetal.cpp
@@ -156,8 +156,12 @@
 
 void BareMetal::AddLinkRuntimeLib(const ArgList ,
   ArgStringList ) const {
-  CmdArgs.push_back(Args.MakeArgString("-lclang_rt.builtins-" +
-   getTriple().getArchName()));
+#ifdef PER_TARGET_RUNTIME_DIR
+  CmdArgs.push_back(Args.MakeArgString("-lclang_rt.builtins"));
+#else
+  CmdArgs.push_back(
+  Args.MakeArgString("-lclang_rt.builtins" + getTriple().getArchName()));
+#endif
 }
 
 void baremetal::Linker::ConstructJob(Compilation , const JobAction ,
Index: clang/lib/Driver/CMakeLists.txt
===
--- clang/lib/Driver/CMakeLists.txt
+++ clang/lib/Driver/CMakeLists.txt
@@ -83,3 +83,7 @@
   clangBasic
   ${system_libs}
   )
+
+if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR)
+  set_source_files_properties(ToolChains/BareMetal.cpp PROPERTIES 
COMPILE_DEFINITIONS "PER_TARGET_RUNTIME_DIR")
+endif()


Index: clang/lib/Driver/ToolChains/BareMetal.cpp
===
--- clang/lib/Driver/ToolChains/BareMetal.cpp
+++ clang/lib/Driver/ToolChains/BareMetal.cpp
@@ -156,8 +156,12 @@
 
 void BareMetal::AddLinkRuntimeLib(const ArgList ,
   ArgStringList ) const {
-  CmdArgs.push_back(Args.MakeArgString("-lclang_rt.builtins-" +
-   getTriple().getArchName()));
+#ifdef PER_TARGET_RUNTIME_DIR
+  CmdArgs.push_back(Args.MakeArgString("-lclang_rt.builtins"));
+#else
+  CmdArgs.push_back(
+  Args.MakeArgString("-lclang_rt.builtins" + getTriple().getArchName()));
+#endif
 }
 
 void baremetal::Linker::ConstructJob(Compilation , const JobAction ,
Index: clang/lib/Driver/CMakeLists.txt
===
--- clang/lib/Driver/CMakeLists.txt
+++ clang/lib/Driver/CMakeLists.txt
@@ -83,3 +83,7 @@
   clangBasic
   ${system_libs}
   )
+
+if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR)
+  set_source_files_properties(ToolChains/BareMetal.cpp PROPERTIES COMPILE_DEFINITIONS "PER_TARGET_RUNTIME_DIR")
+endif()
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

2020-08-31 Thread Raul Tambre via Phabricator via cfe-commits 
tambre added inline comments.



Comment at: clang/include/clang/Basic/IdentifierTable.h:231
 return ObjCOrBuiltinID == tok::NUM_OBJC_KEYWORDS;
   }
 

rjmccall wrote:
> Do we need to support reverting builtins anymore?
We don't. It'd be possible to set the builtin ID of identifiers to `0` as an 
optimization to avoid declaration compatibility checking if there's a 
declaration that would hide the actual builtin. But I doubt it's worth it.

I've removed code related to this, as nothing was actually checking the 
reverted status anymore.



Comment at: clang/test/CodeGen/callback_pthread_create.c:3
+// RUN: false
+// XFAIL: *
+

rjmccall wrote:
> I guess the problem with pthread_create is that the types are not really 
> reasonable to synthesize.  I wonder if we can use an approach more like what 
> we do with C++, where we don't magically synthesize a declaration but where 
> we do recognize that a particular declaration is compatible with the builtin 
> signature.
Seems like a good idea. Would also reduce code duplication between C and C++. I 
would be willing to look into that as a followup.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77491

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

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

2020-08-31 Thread Raul Tambre via Phabricator via cfe-commits 
tambre updated this revision to Diff 288911.
tambre marked 2 inline comments as done.
tambre added a comment.

Remove builtin reverting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77491

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Basic/IdentifierTable.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Decl.cpp
  clang/lib/Headers/intrin.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/AST/ast-dump-attr.cpp
  clang/test/Analysis/bstring.cpp
  clang/test/CodeGen/builtin-redeclaration.c
  clang/test/CodeGen/callback_pthread_create.c
  clang/test/Sema/implicit-builtin-decl.c
  clang/test/Sema/warn-fortify-source.c
  clang/test/SemaCXX/cxx11-compat.cpp
  clang/test/SemaCXX/warn-unused-local-typedef.cpp

Index: clang/test/SemaCXX/warn-unused-local-typedef.cpp
===
--- clang/test/SemaCXX/warn-unused-local-typedef.cpp
+++ clang/test/SemaCXX/warn-unused-local-typedef.cpp
@@ -67,10 +67,10 @@
 
 void test() {
   typedef signed long int superint; // no diag
-  printf("%f", (superint) 42);
+  printf("%ld", (superint)42);
 
   typedef signed long int superint2; // no diag
-  printf("%f", static_cast(42));
+  printf("%ld", static_cast(42));
 
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wunused-local-typedef"
Index: clang/test/SemaCXX/cxx11-compat.cpp
===
--- clang/test/SemaCXX/cxx11-compat.cpp
+++ clang/test/SemaCXX/cxx11-compat.cpp
@@ -31,7 +31,7 @@
 s = { n }, // expected-warning {{non-constant-expression cannot be narrowed from type 'int' to 'char' in initializer list in C++11}} expected-note {{explicit cast}}
 t = { 1234 }; // expected-warning {{constant expression evaluates to 1234 which cannot be narrowed to type 'char' in C++11}} expected-warning {{changes value}} expected-note {{explicit cast}}
 
-#define PRIuS "uS"
+#define PRIuS "zu"
 int printf(const char *, ...);
 typedef __typeof(sizeof(int)) size_t;
 void h(size_t foo, size_t bar) {
Index: clang/test/Sema/warn-fortify-source.c
===
--- clang/test/Sema/warn-fortify-source.c
+++ clang/test/Sema/warn-fortify-source.c
@@ -1,8 +1,6 @@
 // RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify
-// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_PASS_OBJECT_SIZE
 // RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_BUILTINS
 // RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify
-// RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_PASS_OBJECT_SIZE
 // RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_BUILTINS
 
 typedef unsigned long size_t;
@@ -13,13 +11,7 @@
 
 extern int sprintf(char *str, const char *format, ...);
 
-#if defined(USE_PASS_OBJECT_SIZE)
-void *memcpy(void *dst, const void *src, size_t c);
-static void *memcpy(void *dst __attribute__((pass_object_size(1))), const void *src, size_t c) __attribute__((overloadable)) __asm__("merp");
-static void *memcpy(void *const dst __attribute__((pass_object_size(1))), const void *src, size_t c) __attribute__((overloadable)) {
-  return 0;
-}
-#elif defined(USE_BUILTINS)
+#if defined(USE_BUILTINS)
 #define memcpy(x,y,z) __builtin_memcpy(x,y,z)
 #else
 void *memcpy(void *dst, const void *src, size_t c);
@@ -45,14 +37,7 @@
   };
   struct pair p;
   char buf[20];
-  memcpy(, buf, 20);
-#ifdef USE_PASS_OBJECT_SIZE
-  // Use the more strict checking mode on the pass_object_size attribute:
-  // expected-warning@-3 {{memcpy' will always overflow; destination buffer has size 4, but size argument is 20}}
-#else
-  // Or just fallback to type 0:
-  // expected-warning@-6 {{memcpy' will always overflow; destination buffer has size 8, but size argument is 20}}
-#endif
+  memcpy(, buf, 20); // expected-warning {{memcpy' will always overflow; destination buffer has size 8, but size argument is 20}}
 }
 
 void call_strncat() {
Index: clang/test/Sema/implicit-builtin-decl.c
===
--- clang/test/Sema/implicit-builtin-decl.c
+++ clang/test/Sema/implicit-builtin-decl.c
@@ -1,5 +1,4 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
-// RUN: not %clang_cc1 -fsyntax-only -ast-dump %s | FileCheck %s
 
 void f() {
   int *ptr = malloc(sizeof(int) * 10); // expected-warning{{implicitly declaring library function 'malloc' with type}} \
@@ -63,9 +62,5 @@
 struct __jmp_buf_tag {};
 void sigsetjmp(struct __jmp_buf_tag[1], int); // expected-warning{{declaration of built-in function 'sigsetjmp' requires the declaration of the 'jmp_buf' type, commonly provided in the header .}}

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

2020-08-30 Thread Raul Tambre via Phabricator via cfe-commits 
tambre added a comment.

FYI: PR45410, which this fixes, has been marked as a release blocker for 11.0.0.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77491

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

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

2020-08-29 Thread Raul Tambre via Phabricator via cfe-commits 
tambre added a comment.

Please review. I would like to get this in a mergeable state.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77491

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

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

2020-08-29 Thread Raul Tambre via Phabricator via cfe-commits 
tambre updated this revision to Diff 288792.
tambre added a comment.
Herald added subscribers: danielkiss, jfb.

Rebase, fix new MS builtins and other tests, XFAIL pthread_create() test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77491

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Basic/IdentifierTable.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Decl.cpp
  clang/lib/Headers/intrin.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/test/AST/ast-dump-attr.cpp
  clang/test/Analysis/bstring.cpp
  clang/test/CodeGen/builtin-redeclaration.c
  clang/test/CodeGen/callback_pthread_create.c
  clang/test/Sema/implicit-builtin-decl.c
  clang/test/Sema/warn-fortify-source.c
  clang/test/SemaCXX/cxx11-compat.cpp
  clang/test/SemaCXX/warn-unused-local-typedef.cpp

Index: clang/test/SemaCXX/warn-unused-local-typedef.cpp
===
--- clang/test/SemaCXX/warn-unused-local-typedef.cpp
+++ clang/test/SemaCXX/warn-unused-local-typedef.cpp
@@ -67,10 +67,10 @@
 
 void test() {
   typedef signed long int superint; // no diag
-  printf("%f", (superint) 42);
+  printf("%ld", (superint)42);
 
   typedef signed long int superint2; // no diag
-  printf("%f", static_cast(42));
+  printf("%ld", static_cast(42));
 
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wunused-local-typedef"
Index: clang/test/SemaCXX/cxx11-compat.cpp
===
--- clang/test/SemaCXX/cxx11-compat.cpp
+++ clang/test/SemaCXX/cxx11-compat.cpp
@@ -31,7 +31,7 @@
 s = { n }, // expected-warning {{non-constant-expression cannot be narrowed from type 'int' to 'char' in initializer list in C++11}} expected-note {{explicit cast}}
 t = { 1234 }; // expected-warning {{constant expression evaluates to 1234 which cannot be narrowed to type 'char' in C++11}} expected-warning {{changes value}} expected-note {{explicit cast}}
 
-#define PRIuS "uS"
+#define PRIuS "zu"
 int printf(const char *, ...);
 typedef __typeof(sizeof(int)) size_t;
 void h(size_t foo, size_t bar) {
Index: clang/test/Sema/warn-fortify-source.c
===
--- clang/test/Sema/warn-fortify-source.c
+++ clang/test/Sema/warn-fortify-source.c
@@ -1,8 +1,6 @@
 // RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify
-// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_PASS_OBJECT_SIZE
 // RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_BUILTINS
 // RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify
-// RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_PASS_OBJECT_SIZE
 // RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_BUILTINS
 
 typedef unsigned long size_t;
@@ -13,13 +11,7 @@
 
 extern int sprintf(char *str, const char *format, ...);
 
-#if defined(USE_PASS_OBJECT_SIZE)
-void *memcpy(void *dst, const void *src, size_t c);
-static void *memcpy(void *dst __attribute__((pass_object_size(1))), const void *src, size_t c) __attribute__((overloadable)) __asm__("merp");
-static void *memcpy(void *const dst __attribute__((pass_object_size(1))), const void *src, size_t c) __attribute__((overloadable)) {
-  return 0;
-}
-#elif defined(USE_BUILTINS)
+#if defined(USE_BUILTINS)
 #define memcpy(x,y,z) __builtin_memcpy(x,y,z)
 #else
 void *memcpy(void *dst, const void *src, size_t c);
@@ -45,14 +37,7 @@
   };
   struct pair p;
   char buf[20];
-  memcpy(, buf, 20);
-#ifdef USE_PASS_OBJECT_SIZE
-  // Use the more strict checking mode on the pass_object_size attribute:
-  // expected-warning@-3 {{memcpy' will always overflow; destination buffer has size 4, but size argument is 20}}
-#else
-  // Or just fallback to type 0:
-  // expected-warning@-6 {{memcpy' will always overflow; destination buffer has size 8, but size argument is 20}}
-#endif
+  memcpy(, buf, 20); // expected-warning {{memcpy' will always overflow; destination buffer has size 8, but size argument is 20}}
 }
 
 void call_strncat() {
Index: clang/test/Sema/implicit-builtin-decl.c
===
--- clang/test/Sema/implicit-builtin-decl.c
+++ clang/test/Sema/implicit-builtin-decl.c
@@ -1,5 +1,4 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
-// RUN: not %clang_cc1 -fsyntax-only -ast-dump %s | FileCheck %s
 
 void f() {
   int *ptr = malloc(sizeof(int) * 10); // expected-warning{{implicitly declaring library function 'malloc' with type}} \
@@ -63,9 +62,5 @@
 struct __jmp_buf_tag {};
 void sigsetjmp(struct __jmp_buf_tag[1], int); // expected-warning{{declaration of built-in function 'sigsetjmp' requires the declaration of the 'jmp_buf' type, commonly provided in the header .}}
 
-// CHECK: FunctionDecl

[PATCH] D86308: [CMake][compiler-rt][libunwind] Compile assembly files as ASM not C, unify workarounds

2020-08-27 Thread Raul Tambre via Phabricator via cfe-commits 
This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG45344cf7ac5b: [CMake][compiler-rt][libunwind] Compile 
assembly files as ASM not C, unify… (authored by tambre).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86308

Files:
  clang/runtime/CMakeLists.txt
  compiler-rt/cmake/Modules/AddCompilerRT.cmake
  libunwind/src/CMakeLists.txt


Index: libunwind/src/CMakeLists.txt
===
--- libunwind/src/CMakeLists.txt
+++ libunwind/src/CMakeLists.txt
@@ -24,14 +24,10 @@
 UnwindRegistersRestore.S
 UnwindRegistersSave.S
 )
-if (MINGW OR APPLE)
-  # CMake doesn't build assembly sources for windows/gnu targets properly
-  # (up to current CMake, 3.16), so treat them as C files.
-  # Additionally, CMake ignores OSX_ARCHITECTURE for ASM files when targeting
-  # Apple platforms.
-  set_source_files_properties(${LIBUNWIND_ASM_SOURCES}
-  PROPERTIES
-LANGUAGE C)
+
+# See add_asm_sources() in compiler-rt for explanation of this workaround.
+if((APPLE AND CMAKE_VERSION VERSION_LESS 3.19) OR (MINGW AND CMAKE_VERSION 
VERSION_LESS 3.17))
+  set_source_files_properties(${LIBUNWIND_ASM_SOURCES} PROPERTIES LANGUAGE C)
 endif()
 
 set(LIBUNWIND_HEADERS
Index: compiler-rt/cmake/Modules/AddCompilerRT.cmake
===
--- compiler-rt/cmake/Modules/AddCompilerRT.cmake
+++ compiler-rt/cmake/Modules/AddCompilerRT.cmake
@@ -109,11 +109,11 @@
 
 function(add_asm_sources output)
   set(${output} ${ARGN} PARENT_SCOPE)
-  # Xcode will try to compile asm files as C ('clang -x c'), and that will 
fail.
-  if (${CMAKE_GENERATOR} STREQUAL "Xcode")
-enable_language(ASM)
-  else()
-# Pass ASM file directly to the C++ compiler.
+  # CMake doesn't pass the correct architecture for Apple prior to CMake 3.19. 
https://gitlab.kitware.com/cmake/cmake/-/issues/20771
+  # MinGW didn't work correctly with assembly prior to CMake 3.17. 
https://gitlab.kitware.com/cmake/cmake/-/merge_requests/4287 and 
https://reviews.llvm.org/rGb780df052dd2b246a760d00e00f7de9ebdab9d09
+  # Workaround these two issues by compiling as C.
+  # Same workaround used in libunwind. Also update there if changed here.
+  if((APPLE AND CMAKE_VERSION VERSION_LESS 3.19) OR (MINGW AND CMAKE_VERSION 
VERSION_LESS 3.17))
 set_source_files_properties(${ARGN} PROPERTIES LANGUAGE C)
   endif()
 endfunction()
Index: clang/runtime/CMakeLists.txt
===
--- clang/runtime/CMakeLists.txt
+++ clang/runtime/CMakeLists.txt
@@ -75,6 +75,7 @@
 CMAKE_ARGS ${CLANG_COMPILER_RT_CMAKE_ARGS}
-DCMAKE_C_COMPILER=${LLVM_RUNTIME_OUTPUT_INTDIR}/clang
-DCMAKE_CXX_COMPILER=${LLVM_RUNTIME_OUTPUT_INTDIR}/clang++
+   -DCMAKE_ASM_COMPILER=${LLVM_RUNTIME_OUTPUT_INTDIR}/clang
-DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE}
-DCMAKE_MAKE_PROGRAM=${CMAKE_MAKE_PROGRAM}
-DLLVM_CONFIG_PATH=${LLVM_RUNTIME_OUTPUT_INTDIR}/llvm-config


Index: libunwind/src/CMakeLists.txt
===
--- libunwind/src/CMakeLists.txt
+++ libunwind/src/CMakeLists.txt
@@ -24,14 +24,10 @@
 UnwindRegistersRestore.S
 UnwindRegistersSave.S
 )
-if (MINGW OR APPLE)
-  # CMake doesn't build assembly sources for windows/gnu targets properly
-  # (up to current CMake, 3.16), so treat them as C files.
-  # Additionally, CMake ignores OSX_ARCHITECTURE for ASM files when targeting
-  # Apple platforms.
-  set_source_files_properties(${LIBUNWIND_ASM_SOURCES}
-  PROPERTIES
-LANGUAGE C)
+
+# See add_asm_sources() in compiler-rt for explanation of this workaround.
+if((APPLE AND CMAKE_VERSION VERSION_LESS 3.19) OR (MINGW AND CMAKE_VERSION VERSION_LESS 3.17))
+  set_source_files_properties(${LIBUNWIND_ASM_SOURCES} PROPERTIES LANGUAGE C)
 endif()
 
 set(LIBUNWIND_HEADERS
Index: compiler-rt/cmake/Modules/AddCompilerRT.cmake
===
--- compiler-rt/cmake/Modules/AddCompilerRT.cmake
+++ compiler-rt/cmake/Modules/AddCompilerRT.cmake
@@ -109,11 +109,11 @@
 
 function(add_asm_sources output)
   set(${output} ${ARGN} PARENT_SCOPE)
-  # Xcode will try to compile asm files as C ('clang -x c'), and that will fail.
-  if (${CMAKE_GENERATOR} STREQUAL "Xcode")
-enable_language(ASM)
-  else()
-# Pass ASM file directly to the C++ compiler.
+  # CMake doesn't pass the correct architecture for Apple prior to CMake 3.19. https://gitlab.kitware.com/cmake/cmake/-/issues/20771
+  # MinGW

[PATCH] D86308: [CMake][compiler-rt][libunwind] Compile assembly files as ASM not C, unify workarounds

2020-08-27 Thread Raul Tambre via Phabricator via cfe-commits 
tambre updated this revision to Diff 288292.
tambre added a comment.

Rebase, just in case


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86308

Files:
  clang/runtime/CMakeLists.txt
  compiler-rt/cmake/Modules/AddCompilerRT.cmake
  libunwind/src/CMakeLists.txt


Index: libunwind/src/CMakeLists.txt
===
--- libunwind/src/CMakeLists.txt
+++ libunwind/src/CMakeLists.txt
@@ -24,14 +24,10 @@
 UnwindRegistersRestore.S
 UnwindRegistersSave.S
 )
-if (MINGW OR APPLE)
-  # CMake doesn't build assembly sources for windows/gnu targets properly
-  # (up to current CMake, 3.16), so treat them as C files.
-  # Additionally, CMake ignores OSX_ARCHITECTURE for ASM files when targeting
-  # Apple platforms.
-  set_source_files_properties(${LIBUNWIND_ASM_SOURCES}
-  PROPERTIES
-LANGUAGE C)
+
+# See add_asm_sources() in compiler-rt for explanation of this workaround.
+if((APPLE AND CMAKE_VERSION VERSION_LESS 3.19) OR (MINGW AND CMAKE_VERSION 
VERSION_LESS 3.17))
+  set_source_files_properties(${LIBUNWIND_ASM_SOURCES} PROPERTIES LANGUAGE C)
 endif()
 
 set(LIBUNWIND_HEADERS
Index: compiler-rt/cmake/Modules/AddCompilerRT.cmake
===
--- compiler-rt/cmake/Modules/AddCompilerRT.cmake
+++ compiler-rt/cmake/Modules/AddCompilerRT.cmake
@@ -109,11 +109,11 @@
 
 function(add_asm_sources output)
   set(${output} ${ARGN} PARENT_SCOPE)
-  # Xcode will try to compile asm files as C ('clang -x c'), and that will 
fail.
-  if (${CMAKE_GENERATOR} STREQUAL "Xcode")
-enable_language(ASM)
-  else()
-# Pass ASM file directly to the C++ compiler.
+  # CMake doesn't pass the correct architecture for Apple prior to CMake 3.19. 
https://gitlab.kitware.com/cmake/cmake/-/issues/20771
+  # MinGW didn't work correctly with assembly prior to CMake 3.17. 
https://gitlab.kitware.com/cmake/cmake/-/merge_requests/4287 and 
https://reviews.llvm.org/rGb780df052dd2b246a760d00e00f7de9ebdab9d09
+  # Workaround these two issues by compiling as C.
+  # Same workaround used in libunwind. Also update there if changed here.
+  if((APPLE AND CMAKE_VERSION VERSION_LESS 3.19) OR (MINGW AND CMAKE_VERSION 
VERSION_LESS 3.17))
 set_source_files_properties(${ARGN} PROPERTIES LANGUAGE C)
   endif()
 endfunction()
Index: clang/runtime/CMakeLists.txt
===
--- clang/runtime/CMakeLists.txt
+++ clang/runtime/CMakeLists.txt
@@ -75,6 +75,7 @@
 CMAKE_ARGS ${CLANG_COMPILER_RT_CMAKE_ARGS}
-DCMAKE_C_COMPILER=${LLVM_RUNTIME_OUTPUT_INTDIR}/clang
-DCMAKE_CXX_COMPILER=${LLVM_RUNTIME_OUTPUT_INTDIR}/clang++
+   -DCMAKE_ASM_COMPILER=${LLVM_RUNTIME_OUTPUT_INTDIR}/clang
-DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE}
-DCMAKE_MAKE_PROGRAM=${CMAKE_MAKE_PROGRAM}
-DLLVM_CONFIG_PATH=${LLVM_RUNTIME_OUTPUT_INTDIR}/llvm-config


Index: libunwind/src/CMakeLists.txt
===
--- libunwind/src/CMakeLists.txt
+++ libunwind/src/CMakeLists.txt
@@ -24,14 +24,10 @@
 UnwindRegistersRestore.S
 UnwindRegistersSave.S
 )
-if (MINGW OR APPLE)
-  # CMake doesn't build assembly sources for windows/gnu targets properly
-  # (up to current CMake, 3.16), so treat them as C files.
-  # Additionally, CMake ignores OSX_ARCHITECTURE for ASM files when targeting
-  # Apple platforms.
-  set_source_files_properties(${LIBUNWIND_ASM_SOURCES}
-  PROPERTIES
-LANGUAGE C)
+
+# See add_asm_sources() in compiler-rt for explanation of this workaround.
+if((APPLE AND CMAKE_VERSION VERSION_LESS 3.19) OR (MINGW AND CMAKE_VERSION VERSION_LESS 3.17))
+  set_source_files_properties(${LIBUNWIND_ASM_SOURCES} PROPERTIES LANGUAGE C)
 endif()
 
 set(LIBUNWIND_HEADERS
Index: compiler-rt/cmake/Modules/AddCompilerRT.cmake
===
--- compiler-rt/cmake/Modules/AddCompilerRT.cmake
+++ compiler-rt/cmake/Modules/AddCompilerRT.cmake
@@ -109,11 +109,11 @@
 
 function(add_asm_sources output)
   set(${output} ${ARGN} PARENT_SCOPE)
-  # Xcode will try to compile asm files as C ('clang -x c'), and that will fail.
-  if (${CMAKE_GENERATOR} STREQUAL "Xcode")
-enable_language(ASM)
-  else()
-# Pass ASM file directly to the C++ compiler.
+  # CMake doesn't pass the correct architecture for Apple prior to CMake 3.19. https://gitlab.kitware.com/cmake/cmake/-/issues/20771
+  # MinGW didn't work correctly with assembly prior to CMake 3.17. https://gitlab.kitware.com/cmake/cmake/-/merge_requests/4287 and https://reviews.llvm.org/rGb780df052dd2b246a760d00e00f7de9ebdab9d09
+  # Workaround these two issues by compiling as C.
+  # Same

[PATCH] D86308: [CMake][compiler-rt][libunwind] Compile assembly files as ASM not C, unify workarounds

2020-08-24 Thread Raul Tambre via Phabricator via cfe-commits 
tambre added a comment.

In D86308#2233934 , @teemperor wrote:

> In D86308#2229936 , @tambre wrote:
>
>> In D86308#2229901 , @teemperor 
>> wrote:
>>
>>> Sorry, just got around to check this out. With the new workaround this 
>>> seems to work on macOS (the initial patch did produce the same error).
>>
>> Many thanks!
>> I've submitted an upstream CMake MR to hopefully fix this 
>> . I'd 
>> appreciate if you could test that too. There's an example testcase in CMake 
>> issue 20771 .
>
> With that CMake patch your original patch compiles just fine on macOS. Feel 
> free to add a FIXME that this workaround can be removed when the CMake 
> version you merged this is because the new minimum required. Thanks!

I've guarded the Apple workaround behind CMake 3.19 version check. I'll ensure 
that the fix on the CMake side makes it into that version.

I'll commit this tomorrow if there are no objections.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86308

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

[PATCH] D86308: [CMake][compiler-rt][libunwind] Compile assembly files as ASM not C, unify workarounds

2020-08-24 Thread Raul Tambre via Phabricator via cfe-commits 
tambre updated this revision to Diff 287421.
tambre added a comment.

Gate Apple workaround behind CMake version 3.19


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86308

Files:
  clang/runtime/CMakeLists.txt
  compiler-rt/cmake/Modules/AddCompilerRT.cmake
  libunwind/src/CMakeLists.txt


Index: libunwind/src/CMakeLists.txt
===
--- libunwind/src/CMakeLists.txt
+++ libunwind/src/CMakeLists.txt
@@ -24,14 +24,10 @@
 UnwindRegistersRestore.S
 UnwindRegistersSave.S
 )
-if (MINGW OR APPLE)
-  # CMake doesn't build assembly sources for windows/gnu targets properly
-  # (up to current CMake, 3.16), so treat them as C files.
-  # Additionally, CMake ignores OSX_ARCHITECTURE for ASM files when targeting
-  # Apple platforms.
-  set_source_files_properties(${LIBUNWIND_ASM_SOURCES}
-  PROPERTIES
-LANGUAGE C)
+
+# See add_asm_sources() in compiler-rt for explanation of this workaround.
+if((APPLE AND CMAKE_VERSION VERSION_LESS 3.19) OR (MINGW AND CMAKE_VERSION 
VERSION_LESS 3.17))
+  set_source_files_properties(${LIBUNWIND_ASM_SOURCES} PROPERTIES LANGUAGE C)
 endif()
 
 set(LIBUNWIND_HEADERS
Index: compiler-rt/cmake/Modules/AddCompilerRT.cmake
===
--- compiler-rt/cmake/Modules/AddCompilerRT.cmake
+++ compiler-rt/cmake/Modules/AddCompilerRT.cmake
@@ -109,11 +109,11 @@
 
 function(add_asm_sources output)
   set(${output} ${ARGN} PARENT_SCOPE)
-  # Xcode will try to compile asm files as C ('clang -x c'), and that will 
fail.
-  if (${CMAKE_GENERATOR} STREQUAL "Xcode")
-enable_language(ASM)
-  else()
-# Pass ASM file directly to the C++ compiler.
+  # CMake doesn't pass the correct architecture for Apple prior to CMake 3.19. 
https://gitlab.kitware.com/cmake/cmake/-/issues/20771
+  # MinGW didn't work correctly with assembly prior to CMake 3.17. 
https://gitlab.kitware.com/cmake/cmake/-/merge_requests/4287 and 
https://reviews.llvm.org/rGb780df052dd2b246a760d00e00f7de9ebdab9d09
+  # Workaround these two issues by compiling as C.
+  # Same workaround used in libunwind. Also update there if changed here.
+  if((APPLE AND CMAKE_VERSION VERSION_LESS 3.19) OR (MINGW AND CMAKE_VERSION 
VERSION_LESS 3.17))
 set_source_files_properties(${ARGN} PROPERTIES LANGUAGE C)
   endif()
 endfunction()
Index: clang/runtime/CMakeLists.txt
===
--- clang/runtime/CMakeLists.txt
+++ clang/runtime/CMakeLists.txt
@@ -75,6 +75,7 @@
 CMAKE_ARGS ${CLANG_COMPILER_RT_CMAKE_ARGS}
-DCMAKE_C_COMPILER=${LLVM_RUNTIME_OUTPUT_INTDIR}/clang
-DCMAKE_CXX_COMPILER=${LLVM_RUNTIME_OUTPUT_INTDIR}/clang++
+   -DCMAKE_ASM_COMPILER=${LLVM_RUNTIME_OUTPUT_INTDIR}/clang
-DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE}
-DCMAKE_MAKE_PROGRAM=${CMAKE_MAKE_PROGRAM}
-DLLVM_CONFIG_PATH=${LLVM_RUNTIME_OUTPUT_INTDIR}/llvm-config


Index: libunwind/src/CMakeLists.txt
===
--- libunwind/src/CMakeLists.txt
+++ libunwind/src/CMakeLists.txt
@@ -24,14 +24,10 @@
 UnwindRegistersRestore.S
 UnwindRegistersSave.S
 )
-if (MINGW OR APPLE)
-  # CMake doesn't build assembly sources for windows/gnu targets properly
-  # (up to current CMake, 3.16), so treat them as C files.
-  # Additionally, CMake ignores OSX_ARCHITECTURE for ASM files when targeting
-  # Apple platforms.
-  set_source_files_properties(${LIBUNWIND_ASM_SOURCES}
-  PROPERTIES
-LANGUAGE C)
+
+# See add_asm_sources() in compiler-rt for explanation of this workaround.
+if((APPLE AND CMAKE_VERSION VERSION_LESS 3.19) OR (MINGW AND CMAKE_VERSION VERSION_LESS 3.17))
+  set_source_files_properties(${LIBUNWIND_ASM_SOURCES} PROPERTIES LANGUAGE C)
 endif()
 
 set(LIBUNWIND_HEADERS
Index: compiler-rt/cmake/Modules/AddCompilerRT.cmake
===
--- compiler-rt/cmake/Modules/AddCompilerRT.cmake
+++ compiler-rt/cmake/Modules/AddCompilerRT.cmake
@@ -109,11 +109,11 @@
 
 function(add_asm_sources output)
   set(${output} ${ARGN} PARENT_SCOPE)
-  # Xcode will try to compile asm files as C ('clang -x c'), and that will fail.
-  if (${CMAKE_GENERATOR} STREQUAL "Xcode")
-enable_language(ASM)
-  else()
-# Pass ASM file directly to the C++ compiler.
+  # CMake doesn't pass the correct architecture for Apple prior to CMake 3.19. https://gitlab.kitware.com/cmake/cmake/-/issues/20771
+  # MinGW didn't work correctly with assembly prior to CMake 3.17. https://gitlab.kitware.com/cmake/cmake/-/merge_requests/4287 and https://reviews.llvm.org/rGb780df052dd2b246a760d00e00f7de9ebdab9d09
+  # Workaround these two issues by

[PATCH] D86308: [CMake][compiler-rt][libunwind] Compile assembly files as ASM not C, unify workarounds

2020-08-21 Thread Raul Tambre via Phabricator via cfe-commits 
tambre added a comment.

In D86308#2229901 , @teemperor wrote:

> Sorry, just got around to check this out. With the new workaround this seems 
> to work on macOS (the initial patch did produce the same error).

Many thanks!
I've submitted an upstream CMake MR to hopefully fix this 
. I'd appreciate 
if you could test that too. There's an example testcase in CMake issue 20771 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86308

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

[PATCH] D86308: [CMake][compiler-rt][libunwind] Compile assembly files as ASM not C, unify workarounds

2020-08-21 Thread Raul Tambre via Phabricator via cfe-commits 
tambre added a comment.

In D86308#2229199 , @mstorsjo wrote:

> Just FWIW, a similar change was made in libunwind earlier 
> (c48974ffd7d1676f79d39d3b1e70f07d3a5e2e44 
> ), which 
> then required workarounds for cmake issues on both mingw and macos (see 
> b780df052dd2b246a760d00e00f7de9ebdab9d09 
>  and 
> d4ded05ba851304b26a437896bc3962ef56f62cb 
> ) to 
> reintroduce the code for building the asm code as C.

Thanks for the links! That helps a ton.
I've unified the workaround code to be the same and expanded the comments in 
compiler-rt to refer to the relevant upstream issues and to each other.

In D86308#2229176 , @phosek wrote:

> In D86308#2228917 , @tambre wrote:
>
>> I'm pretty sure `add_asm_sources()` has nothing to do. The ASM language is 
>> enabled by compiler-rt anyway and CMake can recognize the files as assembly 
>> anyway.
>
> Can we remove that function altogether then?

Seems we'll need it for the workarounds.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86308

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

[PATCH] D86308: Reland [compiler-rt] Compile assembly files as ASM not C

2020-08-21 Thread Raul Tambre via Phabricator via cfe-commits 
tambre updated this revision to Diff 286957.
tambre added a comment.
Herald added a project: libunwind.
Herald added a subscriber: libcxx-commits.
Herald added a reviewer: libunwind.

Add workaround for Apple, and for MinGW prior to CMake 3.17.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86308

Files:
  clang/runtime/CMakeLists.txt
  compiler-rt/cmake/Modules/AddCompilerRT.cmake
  libunwind/src/CMakeLists.txt


Index: libunwind/src/CMakeLists.txt
===
--- libunwind/src/CMakeLists.txt
+++ libunwind/src/CMakeLists.txt
@@ -24,14 +24,10 @@
 UnwindRegistersRestore.S
 UnwindRegistersSave.S
 )
-if (MINGW OR APPLE)
-  # CMake doesn't build assembly sources for windows/gnu targets properly
-  # (up to current CMake, 3.16), so treat them as C files.
-  # Additionally, CMake ignores OSX_ARCHITECTURE for ASM files when targeting
-  # Apple platforms.
-  set_source_files_properties(${LIBUNWIND_ASM_SOURCES}
-  PROPERTIES
-LANGUAGE C)
+
+# See add_asm_sources() in compiler-rt for explanation of this workaround.
+if(APPLE OR (MINGW AND CMAKE_VERSION VERSION_LESS 3.17))
+  set_source_files_properties(${LIBUNWIND_ASM_SOURCES} PROPERTIES LANGUAGE C)
 endif()
 
 set(LIBUNWIND_HEADERS
Index: compiler-rt/cmake/Modules/AddCompilerRT.cmake
===
--- compiler-rt/cmake/Modules/AddCompilerRT.cmake
+++ compiler-rt/cmake/Modules/AddCompilerRT.cmake
@@ -109,11 +109,11 @@
 
 function(add_asm_sources output)
   set(${output} ${ARGN} PARENT_SCOPE)
-  # Xcode will try to compile asm files as C ('clang -x c'), and that will 
fail.
-  if (${CMAKE_GENERATOR} STREQUAL "Xcode")
-enable_language(ASM)
-  else()
-# Pass ASM file directly to the C++ compiler.
+  # CMake doesn't pass the correct architecture for Apple. 
https://gitlab.kitware.com/cmake/cmake/-/issues/20771
+  # MinGW didn't work correctly with assembly prior to CMake 3.17. 
https://gitlab.kitware.com/cmake/cmake/-/merge_requests/4287 and 
https://reviews.llvm.org/rGb780df052dd2b246a760d00e00f7de9ebdab9d09
+  # Workaround these two issues by compiling as C.
+  # Same workaround used in libunwind. Also update there if changed here.
+  if(APPLE OR (MINGW AND CMAKE_VERSION VERSION_LESS 3.17))
 set_source_files_properties(${ARGN} PROPERTIES LANGUAGE C)
   endif()
 endfunction()
Index: clang/runtime/CMakeLists.txt
===
--- clang/runtime/CMakeLists.txt
+++ clang/runtime/CMakeLists.txt
@@ -75,6 +75,7 @@
 CMAKE_ARGS ${CLANG_COMPILER_RT_CMAKE_ARGS}
-DCMAKE_C_COMPILER=${LLVM_RUNTIME_OUTPUT_INTDIR}/clang
-DCMAKE_CXX_COMPILER=${LLVM_RUNTIME_OUTPUT_INTDIR}/clang++
+   -DCMAKE_ASM_COMPILER=${LLVM_RUNTIME_OUTPUT_INTDIR}/clang
-DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE}
-DCMAKE_MAKE_PROGRAM=${CMAKE_MAKE_PROGRAM}
-DLLVM_CONFIG_PATH=${LLVM_RUNTIME_OUTPUT_INTDIR}/llvm-config


Index: libunwind/src/CMakeLists.txt
===
--- libunwind/src/CMakeLists.txt
+++ libunwind/src/CMakeLists.txt
@@ -24,14 +24,10 @@
 UnwindRegistersRestore.S
 UnwindRegistersSave.S
 )
-if (MINGW OR APPLE)
-  # CMake doesn't build assembly sources for windows/gnu targets properly
-  # (up to current CMake, 3.16), so treat them as C files.
-  # Additionally, CMake ignores OSX_ARCHITECTURE for ASM files when targeting
-  # Apple platforms.
-  set_source_files_properties(${LIBUNWIND_ASM_SOURCES}
-  PROPERTIES
-LANGUAGE C)
+
+# See add_asm_sources() in compiler-rt for explanation of this workaround.
+if(APPLE OR (MINGW AND CMAKE_VERSION VERSION_LESS 3.17))
+  set_source_files_properties(${LIBUNWIND_ASM_SOURCES} PROPERTIES LANGUAGE C)
 endif()
 
 set(LIBUNWIND_HEADERS
Index: compiler-rt/cmake/Modules/AddCompilerRT.cmake
===
--- compiler-rt/cmake/Modules/AddCompilerRT.cmake
+++ compiler-rt/cmake/Modules/AddCompilerRT.cmake
@@ -109,11 +109,11 @@
 
 function(add_asm_sources output)
   set(${output} ${ARGN} PARENT_SCOPE)
-  # Xcode will try to compile asm files as C ('clang -x c'), and that will fail.
-  if (${CMAKE_GENERATOR} STREQUAL "Xcode")
-enable_language(ASM)
-  else()
-# Pass ASM file directly to the C++ compiler.
+  # CMake doesn't pass the correct architecture for Apple. https://gitlab.kitware.com/cmake/cmake/-/issues/20771
+  # MinGW didn't work correctly with assembly prior to CMake 3.17. https://gitlab.kitware.com/cmake/cmake/-/merge_requests/4287 and https://reviews.llvm.org/rGb780df052dd2b246a760d00e00f7de9ebdab9d09
+  # Workaround these two issues by compiling as C.
+  # Same

[PATCH] D86308: Reland [compiler-rt] Compile assembly files as ASM not C

2020-08-20 Thread Raul Tambre via Phabricator via cfe-commits 
tambre added a comment.

I'm pretty sure `add_asm_sources()` has nothing to do. The ASM language is 
enabled by compiler-rt anyway and CMake can recognize the files as assembly 
anyway.

@teemperor Could you give this a try? If this doesn't work, could you upload 
the Ninja file for me to investigate? I've never used macOS so the simulators 
and many architectures at once is quite new to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86308

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

[PATCH] D86308: Reland [compiler-rt] Compile assembly files as ASM not C

2020-08-20 Thread Raul Tambre via Phabricator via cfe-commits 
tambre created this revision.
tambre added reviewers: phosek, teemperor.
Herald added subscribers: Sanitizers, cfe-commits, dexonsmith, kristof.beyls, 
mgorny, dberris.
Herald added projects: clang, Sanitizers.
tambre requested review of this revision.

It isn't very wise to pass an assembly file to the compiler and tell it to 
compile as a C file and hope that the compiler recognizes it as assembly 
instead.
Simply don't mark the file as C and CMake will recognize the rest.

[525/634] Building C object 
lib/tsan/CMakeFiles/clang_rt.tsan-aarch64.dir/rtl/tsan_rtl_aarch64.S.o
FAILED: lib/tsan/CMakeFiles/clang_rt.tsan-aarch64.dir/rtl/tsan_rtl_aarch64.S.o
/opt/tooling/drive/host/bin/clang --target=aarch64-linux-gnu 
-I/opt/tooling/drive/llvm/compiler-rt/lib/tsan/.. -isystem 
/opt/tooling/drive/toolchain/opt/drive/toolchain/include -x c -Wall 
-Wno-unused-parameter -fno-lto -fPIC -fno-builtin -fno-exceptions 
-fomit-frame-pointer -funwind-tables -fno-stack-protector 
-fno-sanitize=safe-stack -fvisibility=hidden -fno-lto -O3 -gline-tables-only 
-Wno-gnu -Wno-variadic-macros -Wno-c99-extensions -Wno-non-virtual-dtor -fPIE 
-fno-rtti -Wframe-larger-than=530 -Wglobal-constructors --sysroot=. -MD -MT 
lib/tsan/CMakeFiles/clang_rt.tsan-aarch64.dir/rtl/tsan_rtl_aarch64.S.o -MF 
lib/tsan/CMakeFiles/clang_rt.tsan-aarch64.dir/rtl/tsan_rtl_aarch64.S.o.d -o 
lib/tsan/CMakeFiles/clang_rt.tsan-aarch64.dir/rtl/tsan_rtl_aarch64.S.o -c 
/opt/tooling/drive/llvm/compiler-rt/lib/tsan/rtl/tsan_rtl_aarch64.S
/opt/tooling/drive/llvm/compiler-rt/lib/tsan/rtl/tsan_rtl_aarch64.S:29:1: 
error: expected identifier or '('
.section .text
^
1 error generated.

Additionally fixed Clang not being passed as the assembly compiler for 
compiler-rt runtime build.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86308

Files:
  clang/runtime/CMakeLists.txt
  compiler-rt/cmake/Modules/AddCompilerRT.cmake


Index: compiler-rt/cmake/Modules/AddCompilerRT.cmake
===
--- compiler-rt/cmake/Modules/AddCompilerRT.cmake
+++ compiler-rt/cmake/Modules/AddCompilerRT.cmake
@@ -109,13 +109,6 @@
 
 function(add_asm_sources output)
   set(${output} ${ARGN} PARENT_SCOPE)
-  # Xcode will try to compile asm files as C ('clang -x c'), and that will 
fail.
-  if (${CMAKE_GENERATOR} STREQUAL "Xcode")
-enable_language(ASM)
-  else()
-# Pass ASM file directly to the C++ compiler.
-set_source_files_properties(${ARGN} PROPERTIES LANGUAGE C)
-  endif()
 endfunction()
 
 macro(set_output_name output name arch)
Index: clang/runtime/CMakeLists.txt
===
--- clang/runtime/CMakeLists.txt
+++ clang/runtime/CMakeLists.txt
@@ -75,6 +75,7 @@
 CMAKE_ARGS ${CLANG_COMPILER_RT_CMAKE_ARGS}
-DCMAKE_C_COMPILER=${LLVM_RUNTIME_OUTPUT_INTDIR}/clang
-DCMAKE_CXX_COMPILER=${LLVM_RUNTIME_OUTPUT_INTDIR}/clang++
+   -DCMAKE_ASM_COMPILER=${LLVM_RUNTIME_OUTPUT_INTDIR}/clang
-DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE}
-DCMAKE_MAKE_PROGRAM=${CMAKE_MAKE_PROGRAM}
-DLLVM_CONFIG_PATH=${LLVM_RUNTIME_OUTPUT_INTDIR}/llvm-config


Index: compiler-rt/cmake/Modules/AddCompilerRT.cmake
===
--- compiler-rt/cmake/Modules/AddCompilerRT.cmake
+++ compiler-rt/cmake/Modules/AddCompilerRT.cmake
@@ -109,13 +109,6 @@
 
 function(add_asm_sources output)
   set(${output} ${ARGN} PARENT_SCOPE)
-  # Xcode will try to compile asm files as C ('clang -x c'), and that will fail.
-  if (${CMAKE_GENERATOR} STREQUAL "Xcode")
-enable_language(ASM)
-  else()
-# Pass ASM file directly to the C++ compiler.
-set_source_files_properties(${ARGN} PROPERTIES LANGUAGE C)
-  endif()
 endfunction()
 
 macro(set_output_name output name arch)
Index: clang/runtime/CMakeLists.txt
===
--- clang/runtime/CMakeLists.txt
+++ clang/runtime/CMakeLists.txt
@@ -75,6 +75,7 @@
 CMAKE_ARGS ${CLANG_COMPILER_RT_CMAKE_ARGS}
-DCMAKE_C_COMPILER=${LLVM_RUNTIME_OUTPUT_INTDIR}/clang
-DCMAKE_CXX_COMPILER=${LLVM_RUNTIME_OUTPUT_INTDIR}/clang++
+   -DCMAKE_ASM_COMPILER=${LLVM_RUNTIME_OUTPUT_INTDIR}/clang
-DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE}
-DCMAKE_MAKE_PROGRAM=${CMAKE_MAKE_PROGRAM}
-DLLVM_CONFIG_PATH=${LLVM_RUNTIME_OUTPUT_INTDIR}/llvm-config
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85706: [compiler-rt] Compile assembly files as ASM not C

2020-08-20 Thread Raul Tambre via Phabricator via cfe-commits 
tambre added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85706

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

[PATCH] D76323: [AST] Fix handling of long double and bool in __builtin_bit_cast

2020-08-18 Thread Raul Tambre via Phabricator via cfe-commits 
tambre added a comment.

For reference, I've been using this + D75960  
in my employer's Clang build for almost two months. We have been using 
`std::bit_cast` in our codebase quite a bit and haven't encountered any issues 
with these two changes. It would be nice too see this merged.


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

https://reviews.llvm.org/D76323

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

[PATCH] D85706: [compiler-rt] Compile assembly files as ASM not C

2020-08-14 Thread Raul Tambre via Phabricator via cfe-commits 
tambre added a comment.

In D85706#2214009 , @phosek wrote:

> One concern I have with this change is that we may not always consistently 
> update CMAKE_ASM_FLAGS or set CMAKE_ASM_COMPILER. This wouldn't make a 
> difference before, but it'll with this change. Can you check if these 
> variables are being updated as needed?

I searched and was able to find only one instance. I've fixed it in this 
change.  
I lack commit privileges, so whoever commits this should be ready to revert it, 
as there's a chance for breakage.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85706

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

[PATCH] D85706: [compiler-rt] Compile assembly files as ASM not C

2020-08-14 Thread Raul Tambre via Phabricator via cfe-commits 
tambre updated this revision to Diff 285692.
tambre added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Pass CMAKE_ASM_COMPILER to compiler-rt in runtime build


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85706

Files:
  clang/runtime/CMakeLists.txt
  compiler-rt/cmake/Modules/AddCompilerRT.cmake


Index: compiler-rt/cmake/Modules/AddCompilerRT.cmake
===
--- compiler-rt/cmake/Modules/AddCompilerRT.cmake
+++ compiler-rt/cmake/Modules/AddCompilerRT.cmake
@@ -109,13 +109,11 @@
 
 function(add_asm_sources output)
   set(${output} ${ARGN} PARENT_SCOPE)
-  # Xcode will try to compile asm files as C ('clang -x c'), and that will 
fail.
-  if (${CMAKE_GENERATOR} STREQUAL "Xcode")
-enable_language(ASM)
-  else()
-# Pass ASM file directly to the C++ compiler.
-set_source_files_properties(${ARGN} PROPERTIES LANGUAGE C)
-  endif()
+  # Make sure ASM language is available.
+  # We explicitly mark the source files as ASM, so they don't get passed to the
+  # C/CXX compiler and hopes that it recognizes them as assembly.
+  enable_language(ASM)
+  set_source_files_properties(${ARGN} PROPERTIES LANGUAGE ASM)
 endfunction()
 
 macro(set_output_name output name arch)
Index: clang/runtime/CMakeLists.txt
===
--- clang/runtime/CMakeLists.txt
+++ clang/runtime/CMakeLists.txt
@@ -75,6 +75,7 @@
 CMAKE_ARGS ${CLANG_COMPILER_RT_CMAKE_ARGS}
-DCMAKE_C_COMPILER=${LLVM_RUNTIME_OUTPUT_INTDIR}/clang
-DCMAKE_CXX_COMPILER=${LLVM_RUNTIME_OUTPUT_INTDIR}/clang++
+   -DCMAKE_ASM_COMPILER=${LLVM_RUNTIME_OUTPUT_INTDIR}/clang
-DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE}
-DCMAKE_MAKE_PROGRAM=${CMAKE_MAKE_PROGRAM}
-DLLVM_CONFIG_PATH=${LLVM_RUNTIME_OUTPUT_INTDIR}/llvm-config


Index: compiler-rt/cmake/Modules/AddCompilerRT.cmake
===
--- compiler-rt/cmake/Modules/AddCompilerRT.cmake
+++ compiler-rt/cmake/Modules/AddCompilerRT.cmake
@@ -109,13 +109,11 @@
 
 function(add_asm_sources output)
   set(${output} ${ARGN} PARENT_SCOPE)
-  # Xcode will try to compile asm files as C ('clang -x c'), and that will fail.
-  if (${CMAKE_GENERATOR} STREQUAL "Xcode")
-enable_language(ASM)
-  else()
-# Pass ASM file directly to the C++ compiler.
-set_source_files_properties(${ARGN} PROPERTIES LANGUAGE C)
-  endif()
+  # Make sure ASM language is available.
+  # We explicitly mark the source files as ASM, so they don't get passed to the
+  # C/CXX compiler and hopes that it recognizes them as assembly.
+  enable_language(ASM)
+  set_source_files_properties(${ARGN} PROPERTIES LANGUAGE ASM)
 endfunction()
 
 macro(set_output_name output name arch)
Index: clang/runtime/CMakeLists.txt
===
--- clang/runtime/CMakeLists.txt
+++ clang/runtime/CMakeLists.txt
@@ -75,6 +75,7 @@
 CMAKE_ARGS ${CLANG_COMPILER_RT_CMAKE_ARGS}
-DCMAKE_C_COMPILER=${LLVM_RUNTIME_OUTPUT_INTDIR}/clang
-DCMAKE_CXX_COMPILER=${LLVM_RUNTIME_OUTPUT_INTDIR}/clang++
+   -DCMAKE_ASM_COMPILER=${LLVM_RUNTIME_OUTPUT_INTDIR}/clang
-DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE}
-DCMAKE_MAKE_PROGRAM=${CMAKE_MAKE_PROGRAM}
-DLLVM_CONFIG_PATH=${LLVM_RUNTIME_OUTPUT_INTDIR}/llvm-config
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

2020-05-30 Thread Raul Tambre via Phabricator via cfe-commits 
tambre updated this revision to Diff 267450.
tambre marked 4 inline comments as done.
tambre added a comment.

Fix some intrinsics not being marked as builtin due to being static in the 
headers.
Make some code easier to read, fix test for sigsetjmp in 
Sema/implicit-builtin-decl.c to reflect the original intent.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77491

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/IdentifierTable.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Decl.cpp
  clang/lib/Headers/intrin.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/test/AST/ast-dump-attr.cpp
  clang/test/CodeGen/builtin-redeclaration.c
  clang/test/CodeGen/callback_pthread_create.c
  clang/test/Sema/implicit-builtin-decl.c
  clang/test/Sema/warn-fortify-source.c
  clang/test/SemaCXX/cxx11-compat.cpp
  clang/test/SemaCXX/warn-unused-local-typedef.cpp

Index: clang/test/SemaCXX/warn-unused-local-typedef.cpp
===
--- clang/test/SemaCXX/warn-unused-local-typedef.cpp
+++ clang/test/SemaCXX/warn-unused-local-typedef.cpp
@@ -67,10 +67,10 @@
 
 void test() {
   typedef signed long int superint; // no diag
-  printf("%f", (superint) 42);
+  printf("%ld", (superint)42);
 
   typedef signed long int superint2; // no diag
-  printf("%f", static_cast(42));
+  printf("%ld", static_cast(42));
 
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wunused-local-typedef"
Index: clang/test/SemaCXX/cxx11-compat.cpp
===
--- clang/test/SemaCXX/cxx11-compat.cpp
+++ clang/test/SemaCXX/cxx11-compat.cpp
@@ -31,7 +31,7 @@
 s = { n }, // expected-warning {{non-constant-expression cannot be narrowed from type 'int' to 'char' in initializer list in C++11}} expected-note {{explicit cast}}
 t = { 1234 }; // expected-warning {{constant expression evaluates to 1234 which cannot be narrowed to type 'char' in C++11}} expected-warning {{changes value}} expected-note {{explicit cast}}
 
-#define PRIuS "uS"
+#define PRIuS "zu"
 int printf(const char *, ...);
 typedef __typeof(sizeof(int)) size_t;
 void h(size_t foo, size_t bar) {
Index: clang/test/Sema/warn-fortify-source.c
===
--- clang/test/Sema/warn-fortify-source.c
+++ clang/test/Sema/warn-fortify-source.c
@@ -1,8 +1,6 @@
 // RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify
-// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_PASS_OBJECT_SIZE
 // RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_BUILTINS
 // RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify
-// RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_PASS_OBJECT_SIZE
 // RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_BUILTINS
 
 typedef unsigned long size_t;
@@ -13,13 +11,7 @@
 
 extern int sprintf(char *str, const char *format, ...);
 
-#if defined(USE_PASS_OBJECT_SIZE)
-void *memcpy(void *dst, const void *src, size_t c);
-static void *memcpy(void *dst __attribute__((pass_object_size(1))), const void *src, size_t c) __attribute__((overloadable)) __asm__("merp");
-static void *memcpy(void *const dst __attribute__((pass_object_size(1))), const void *src, size_t c) __attribute__((overloadable)) {
-  return 0;
-}
-#elif defined(USE_BUILTINS)
+#if defined(USE_BUILTINS)
 #define memcpy(x,y,z) __builtin_memcpy(x,y,z)
 #else
 void *memcpy(void *dst, const void *src, size_t c);
@@ -45,14 +37,7 @@
   };
   struct pair p;
   char buf[20];
-  memcpy(, buf, 20);
-#ifdef USE_PASS_OBJECT_SIZE
-  // Use the more strict checking mode on the pass_object_size attribute:
-  // expected-warning@-3 {{memcpy' will always overflow; destination buffer has size 4, but size argument is 20}}
-#else
-  // Or just fallback to type 0:
-  // expected-warning@-6 {{memcpy' will always overflow; destination buffer has size 8, but size argument is 20}}
-#endif
+  memcpy(, buf, 20); // expected-warning {{memcpy' will always overflow; destination buffer has size 8, but size argument is 20}}
 }
 
 void call_strncat() {
Index: clang/test/Sema/implicit-builtin-decl.c
===
--- clang/test/Sema/implicit-builtin-decl.c
+++ clang/test/Sema/implicit-builtin-decl.c
@@ -1,5 +1,4 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
-// RUN: not %clang_cc1 -fsyntax-only -ast-dump %s | FileCheck %s
 
 void f() {
   int *ptr = malloc(sizeof(int) * 10); // expected-warning{{implicitly declaring library function 'malloc' with type}} \
@@ -63,9 +62,5 @@
 struct __jmp_buf_tag {};
 void sigsetjmp(struct __jmp_buf_tag[1], int); // expected-warning{{declaration of built-in function 'sigsetjmp' requires the declaration of the 'jmp_buf' type, commonly

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

2020-05-30 Thread Raul Tambre via Phabricator via cfe-commits 
tambre marked an inline comment as done.
tambre added inline comments.



Comment at: clang/lib/AST/Decl.cpp:3169-3175
   } else {
-if (!getIdentifier())
+const auto *Attr = getAttr();
+
+if (!Attr)
   return 0;
 
+BuiltinID = Attr->getID();

aaron.ballman wrote:
> I think this is a bit more clear:
> ```
> } else if (const auto *A = getAttr()) {
>   BuiltinID = A->getID();
> }
> ```
> and initialize `BuiltinID` to zero above.
Done.



Comment at: clang/test/CodeGen/callback_pthread_create.c:17
 
+// FIXME: How to do builtin handling for this?
 int pthread_create(pthread_t *, const pthread_attr_t *,

As many others prior to this patch, `pthread_create` was recognized as a 
builtin due to its name and thus had attributes applied.
Unlike others however, `pthread_create` is the only builtin in `Builtins.def` 
that doesn't have its arguments specified. Doing that would require 
implementing support for function pointers in the builtin database and adding 
yet another special case for `pthread_t` and `pthread_attr_t`.
That'd be quite a bit of work, which I'm not interested in doing.

How about simply removing the hack that is the `pthread_create` builtin entry 
and disabling/removing this test?



Comment at: clang/test/Sema/implicit-builtin-decl.c:64
-struct __jmp_buf_tag {};
-void sigsetjmp(struct __jmp_buf_tag[1], int); // expected-warning{{declaration 
of built-in function 'sigsetjmp' requires the declaration of the 'jmp_buf' 
type, commonly provided in the header .}}
 

aaron.ballman wrote:
> It looks like we're losing test coverage with this change?
Indeed. I've reverted this change and changed the test to instead not test for 
it being recognized as a builtin, since it shouldn't be.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77491



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

[PATCH] D79800: [Sema] Implement DR2233

2020-05-30 Thread Raul Tambre via Phabricator via cfe-commits 
tambre abandoned this revision.
tambre added a comment.

In D79800#2058985 , @rsmith wrote:

> Unfortunately, I think this approach basically can't work, because we need to 
> consider inheriting default arguments from a previous (or subsequent!) 
> declaration before dropping default arguments preceding a pack. I think we 
> will instead need to detect this situation when issuing the diagnostic for a 
> parameter with a default argument followed by a parameter without one, and 
> will need to make sure that all the parts of Clang that look at default 
> arguments can cope with them being discontiguous.


Thanks for reviewing!
Abandoning, as I lack the motivation to implement the proper solution you laid 
out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79800



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

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

2020-05-25 Thread Raul Tambre via Phabricator via cfe-commits 
tambre added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:8880
+  }
+}
+

rjmccall wrote:
> tambre wrote:
> > rjmccall wrote:
> > > Hmm.  I'm concerned about not doing any sort of semantic compatibility 
> > > check here before we assign the function special semantics.  Have we 
> > > really never done those in C++?
> > > 
> > > If not, I wonder if we can reasonably make an implicit declaration but 
> > > just make it hidden from lookup.
> > Currently there's no semantic compatibility checking for builtin 
> > redeclarations. There is for initial declarations though.
> > 
> > I've added this checking by splitting the actual builtin declaration 
> > creation off from `LazilyCreateBuiltin` into `CreateBuiltin` and checking 
> > if the current declaration is compatible with what the builtin's would be.
> > This results in stronger typechecking than before for builtin declarations, 
> > so some incompatible declarations are no longer marked as builtin. See 
> > `cxx1z-noexcept-function-type.cpp` for an example.
> That makes sense to me in principle.  I'm definitely concerned about 
> `noexcept` differences causing C library functions to not be treated as 
> builtins, though; that seems stricter than we want.  How reasonable is it to 
> weaken this?
I agree having `noexcept` weakened is reasonable.
I've changed it to create an identical type to the NewFD with the exception 
spec removed for the comparison. This fixes it.



Comment at: clang/test/CodeGen/ms-intrinsics.c:23
 void test__stosb(unsigned char *Dest, unsigned char Data, size_t Count) {
   return __stosb(Dest, Data, Count);
 }

`__stosb` and friends aren't marked as builtin because they're declared as 
`static`.
I don't think there's a good reason to have builtins as `static` and we should 
simply remove the `static` specifier from those intrinsics in headers.
Alternatively, we could weaken compatibility checking similar to `noexcept`.
Thoughts?



Comment at: clang/test/Sema/warn-fortify-source.c:20
 void *memcpy(void *dst, const void *src, size_t c);
 static void *memcpy(void *dst __attribute__((pass_object_size(1))), const void 
*src, size_t c) __attribute__((overloadable)) __asm__("merp");
 static void *memcpy(void *const dst __attribute__((pass_object_size(1))), 
const void *src, size_t c) __attribute__((overloadable)) {

rjmccall wrote:
> tambre wrote:
> > Not quite sure what to do here. These were previously recognized as 
> > builtins due to their name despite being incompatible and thus had fortify 
> > checking similar to the real `memcpy`.
> > 
> > Maybe:
> > 1. Introduce a generic version of `ArmBuiltinAliasAttr`.
> > 2. Something like `FormatAttr`.
> That's interesting.  It definitely seems wrong to apply builtin logic to a 
> function that doesn't have a compatible low-level signature.  My inclination 
> is to disable builtin checking here, but to notify the contributors so that 
> they can figure out an appropriate response.
Agreed.
I've removed this test, as there doesn't seem to be an easy way to replicate 
this behaviour.



Comment at: clang/test/SemaCXX/cxx1z-noexcept-function-type.cpp:120
   extern "C" int strcmp(const char *, const char *);
-  extern "C" int strncmp(const char *, const char *, decltype(sizeof(0))) 
noexcept;
+  extern "C" int strncmp(const char *, const char *, decltype(sizeof(0)));
 

tambre wrote:
> This doesn't work anymore since we now properly check builtin declaration 
> compatibility and since C++17 noexcept is part of the function type but 
> builtins aren't noexcept.
> Thoughts?
Fixed by removing `noexcept` for the declaration compatibility comparison.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77491



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

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

2020-05-25 Thread Raul Tambre via Phabricator via cfe-commits 
tambre updated this revision to Diff 265871.
tambre marked 6 inline comments as done.
tambre added a comment.

Weakened noexcept checking.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77491

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/IdentifierTable.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Decl.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/test/AST/ast-dump-attr.cpp
  clang/test/CodeGen/builtin-redeclaration.c
  clang/test/CodeGen/callback_pthread_create.c
  clang/test/CodeGen/ms-intrinsics.c
  clang/test/Sema/implicit-builtin-decl.c
  clang/test/Sema/warn-fortify-source.c
  clang/test/SemaCXX/cxx11-compat.cpp
  clang/test/SemaCXX/warn-unused-local-typedef.cpp

Index: clang/test/SemaCXX/warn-unused-local-typedef.cpp
===
--- clang/test/SemaCXX/warn-unused-local-typedef.cpp
+++ clang/test/SemaCXX/warn-unused-local-typedef.cpp
@@ -67,10 +67,10 @@
 
 void test() {
   typedef signed long int superint; // no diag
-  printf("%f", (superint) 42);
+  printf("%ld", (superint)42);
 
   typedef signed long int superint2; // no diag
-  printf("%f", static_cast(42));
+  printf("%ld", static_cast(42));
 
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wunused-local-typedef"
Index: clang/test/SemaCXX/cxx11-compat.cpp
===
--- clang/test/SemaCXX/cxx11-compat.cpp
+++ clang/test/SemaCXX/cxx11-compat.cpp
@@ -31,7 +31,7 @@
 s = { n }, // expected-warning {{non-constant-expression cannot be narrowed from type 'int' to 'char' in initializer list in C++11}} expected-note {{explicit cast}}
 t = { 1234 }; // expected-warning {{constant expression evaluates to 1234 which cannot be narrowed to type 'char' in C++11}} expected-warning {{changes value}} expected-note {{explicit cast}}
 
-#define PRIuS "uS"
+#define PRIuS "zu"
 int printf(const char *, ...);
 typedef __typeof(sizeof(int)) size_t;
 void h(size_t foo, size_t bar) {
Index: clang/test/Sema/warn-fortify-source.c
===
--- clang/test/Sema/warn-fortify-source.c
+++ clang/test/Sema/warn-fortify-source.c
@@ -1,10 +1,12 @@
 // RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify
-// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_PASS_OBJECT_SIZE
 // RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_BUILTINS
 // RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify
-// RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_PASS_OBJECT_SIZE
 // RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_BUILTINS
 
+// FIXME: Incompatible builtin redeclarations aren't considered builtins and thus don't call the builtin nor inherit their attributes.
+// %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_PASS_OBJECT_SIZE
+// %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_PASS_OBJECT_SIZE
+
 typedef unsigned long size_t;
 
 #ifdef __cplusplus
Index: clang/test/Sema/implicit-builtin-decl.c
===
--- clang/test/Sema/implicit-builtin-decl.c
+++ clang/test/Sema/implicit-builtin-decl.c
@@ -60,12 +60,15 @@
 
 extern float fmaxf(float, float);
 
-struct __jmp_buf_tag {};
-void sigsetjmp(struct __jmp_buf_tag[1], int); // expected-warning{{declaration of built-in function 'sigsetjmp' requires the declaration of the 'jmp_buf' type, commonly provided in the header .}}
+typedef struct __jmp_buf_tag {
+} sigjmp_buf[1];
 
-// CHECK: FunctionDecl {{.*}}  col:6 sigsetjmp '
+int sigsetjmp(struct __jmp_buf_tag[1], int);
+
+// CHECK: FunctionDecl {{.*}}  col:5 implicit sigsetjmp '
+// CHECK: FunctionDecl {{.*}}  col:5 sigsetjmp '
 // CHECK-NOT: FunctionDecl
-// CHECK: ReturnsTwiceAttr {{.*}} <{{.*}}> Implicit
+// CHECK: ReturnsTwiceAttr {{.*}} <{{.*}}> Inherited Implicit
 
 // PR40692
 void pthread_create(); // no warning expected
Index: clang/test/CodeGen/ms-intrinsics.c
===
--- clang/test/CodeGen/ms-intrinsics.c
+++ clang/test/CodeGen/ms-intrinsics.c
@@ -17,6 +17,7 @@
 
 #include 
 
+// FIXME: Why aren't these builtins recognized as builtins?
 #if defined(__i386__) || defined(__x86_64__)
 void test__stosb(unsigned char *Dest, unsigned char Data, size_t Count) {
   return __stosb(Dest, Data, Count);
Index: clang/test/CodeGen/callback_pthread_create.c
===
--- clang/test/CodeGen/callback_pthread_create.c
+++ clang/test/CodeGen/callback_pthread_create.c
@@ -14,6 +14,7 @@
 typedef __darwin_pthread_t pthread_t;
 typedef __darwin_pthread_attr_t pthread_attr_t;
 
+// FIXME: Why isn't this

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

2020-05-25 Thread Raul Tambre via Phabricator via cfe-commits 
tambre updated this revision to Diff 265872.
tambre marked 4 inline comments as done.
tambre added a comment.

Remove memcpy overload tests from warn-fortify-source.c, which relied on 
identifier-based builtin identification.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77491

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/IdentifierTable.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Decl.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/test/AST/ast-dump-attr.cpp
  clang/test/CodeGen/builtin-redeclaration.c
  clang/test/CodeGen/callback_pthread_create.c
  clang/test/CodeGen/ms-intrinsics.c
  clang/test/Sema/implicit-builtin-decl.c
  clang/test/Sema/warn-fortify-source.c
  clang/test/SemaCXX/cxx11-compat.cpp
  clang/test/SemaCXX/warn-unused-local-typedef.cpp

Index: clang/test/SemaCXX/warn-unused-local-typedef.cpp
===
--- clang/test/SemaCXX/warn-unused-local-typedef.cpp
+++ clang/test/SemaCXX/warn-unused-local-typedef.cpp
@@ -67,10 +67,10 @@
 
 void test() {
   typedef signed long int superint; // no diag
-  printf("%f", (superint) 42);
+  printf("%ld", (superint)42);
 
   typedef signed long int superint2; // no diag
-  printf("%f", static_cast(42));
+  printf("%ld", static_cast(42));
 
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wunused-local-typedef"
Index: clang/test/SemaCXX/cxx11-compat.cpp
===
--- clang/test/SemaCXX/cxx11-compat.cpp
+++ clang/test/SemaCXX/cxx11-compat.cpp
@@ -31,7 +31,7 @@
 s = { n }, // expected-warning {{non-constant-expression cannot be narrowed from type 'int' to 'char' in initializer list in C++11}} expected-note {{explicit cast}}
 t = { 1234 }; // expected-warning {{constant expression evaluates to 1234 which cannot be narrowed to type 'char' in C++11}} expected-warning {{changes value}} expected-note {{explicit cast}}
 
-#define PRIuS "uS"
+#define PRIuS "zu"
 int printf(const char *, ...);
 typedef __typeof(sizeof(int)) size_t;
 void h(size_t foo, size_t bar) {
Index: clang/test/Sema/warn-fortify-source.c
===
--- clang/test/Sema/warn-fortify-source.c
+++ clang/test/Sema/warn-fortify-source.c
@@ -1,8 +1,6 @@
 // RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify
-// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_PASS_OBJECT_SIZE
 // RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_BUILTINS
 // RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify
-// RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_PASS_OBJECT_SIZE
 // RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_BUILTINS
 
 typedef unsigned long size_t;
@@ -13,13 +11,7 @@
 
 extern int sprintf(char *str, const char *format, ...);
 
-#if defined(USE_PASS_OBJECT_SIZE)
-void *memcpy(void *dst, const void *src, size_t c);
-static void *memcpy(void *dst __attribute__((pass_object_size(1))), const void *src, size_t c) __attribute__((overloadable)) __asm__("merp");
-static void *memcpy(void *const dst __attribute__((pass_object_size(1))), const void *src, size_t c) __attribute__((overloadable)) {
-  return 0;
-}
-#elif defined(USE_BUILTINS)
+#if defined(USE_BUILTINS)
 #define memcpy(x,y,z) __builtin_memcpy(x,y,z)
 #else
 void *memcpy(void *dst, const void *src, size_t c);
@@ -45,14 +37,7 @@
   };
   struct pair p;
   char buf[20];
-  memcpy(, buf, 20);
-#ifdef USE_PASS_OBJECT_SIZE
-  // Use the more strict checking mode on the pass_object_size attribute:
-  // expected-warning@-3 {{memcpy' will always overflow; destination buffer has size 4, but size argument is 20}}
-#else
-  // Or just fallback to type 0:
-  // expected-warning@-6 {{memcpy' will always overflow; destination buffer has size 8, but size argument is 20}}
-#endif
+  memcpy(, buf, 20); // expected-warning {{memcpy' will always overflow; destination buffer has size 8, but size argument is 20}}
 }
 
 void call_strncat() {
Index: clang/test/Sema/implicit-builtin-decl.c
===
--- clang/test/Sema/implicit-builtin-decl.c
+++ clang/test/Sema/implicit-builtin-decl.c
@@ -60,12 +60,15 @@
 
 extern float fmaxf(float, float);
 
-struct __jmp_buf_tag {};
-void sigsetjmp(struct __jmp_buf_tag[1], int); // expected-warning{{declaration of built-in function 'sigsetjmp' requires the declaration of the 'jmp_buf' type, commonly provided in the header .}}
+typedef struct __jmp_buf_tag {
+} sigjmp_buf[1];
 
-// CHECK: FunctionDecl {{.*}}  col:6 sigsetjmp '
+int sigsetjmp(struct __jmp_buf_tag[1], int);
+
+// CHECK: FunctionDecl {{.*}}  col:5 implicit sigsetjmp '
+// CHECK: FunctionDecl {{.*}}  col:5 sigsetjmp '
 //

[PATCH] D79800: [Sema] Remove default values for arguments prior to a parameter pack if the pack is used

2020-05-25 Thread Raul Tambre via Phabricator via cfe-commits 
tambre updated this revision to Diff 265850.
tambre marked 5 inline comments as done.
tambre added a comment.

Handle multiple parameter packs interleaved with default values.
Mark DR777 as superseded by DR2233. Mark DR2233 as resolved.
Moved tests from dr7xx.cpp to dr22xx.cpp. Added note in dr7xx.cpp about DR777 
being superseded.
Add more tests that cover bugs observed in other implementations and the 
deficiency in my first implementation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79800

Files:
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/CXX/drs/dr22xx.cpp
  clang/test/CXX/drs/dr7xx.cpp
  clang/www/cxx_dr_status.html

Index: clang/www/cxx_dr_status.html
===
--- clang/www/cxx_dr_status.html
+++ clang/www/cxx_dr_status.html
@@ -4681,7 +4681,7 @@
 https://wg21.link/cwg777;>777
 CD2
 Default arguments and parameter packs
-Clang 3.7
+Superseded by 2233
   
   
 https://wg21.link/cwg778;>778
@@ -13213,7 +13213,7 @@
 https://wg21.link/cwg2233;>2233
 DRWP
 Function parameter packs following default arguments
-Unknown
+Clang 11
   
   
 https://wg21.link/cwg2234;>2234
Index: clang/test/CXX/drs/dr7xx.cpp
===
--- clang/test/CXX/drs/dr7xx.cpp
+++ clang/test/CXX/drs/dr7xx.cpp
@@ -219,16 +219,4 @@
   Collision c; // expected-note {{in instantiation of}}
 }
 
-namespace dr777 { // dr777: 3.7
-#if __cplusplus >= 201103L
-template 
-void f(int i = 0, T ...args) {}
-void ff() { f(); }
-
-template 
-void g(int i = 0, T ...args, T ...args2) {}
-
-template 
-void h(int i = 0, T ...args, int j = 1) {}
-#endif
-}
+// dr777 superseded by dr2233
Index: clang/test/CXX/drs/dr22xx.cpp
===
--- clang/test/CXX/drs/dr22xx.cpp
+++ clang/test/CXX/drs/dr22xx.cpp
@@ -35,3 +35,44 @@
   }
 #endif
 }
+
+namespace dr2233 { // dr2233: 11
+#if __cplusplus >= 201103L
+template 
+void f(int i = 0, T... args) {}
+
+template 
+void g(int i = 0, T... args, T... args2) {}
+
+template 
+void h(int i = 0, T... args, int j = 1) {}
+
+template 
+void i(int i = 0, T... args, int j = 1, U... args2) {}
+
+template 
+void j(int i = 0, Ts... ts) {}
+
+template <>
+void j(int i, int j) {}
+
+// PR23029
+// Ensure instantiating the templates works.
+void use() {
+  f();
+  f(0, 1);
+  f(1, 2);
+  g(1, 2, 3);
+  h(0, 1);
+  i();
+  i(3);
+  i(3, 2);
+  i(3, 2, 1);
+  i(1, 2, 3, 4, 5);
+  j();
+  j(1);
+  j(1, 2);
+  j(1, 2);
+}
+#endif
+} // namespace dr2233
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -1974,6 +1974,47 @@
 TemplateArgumentList::CreateCopy(SemaRef.Context,
  Innermost),
 /*InsertPos=*/nullptr);
+
+// DR777, DR2233.
+// Parameter packs are allowed after and inbetween parameters with default
+// values. We need to remove default arguments for parameters before the
+// first expanded parameter pack to prevent prevent it being diagnosed as
+// invalid code due to the expanded parameters lacking default values. This
+// is safe to do because if a parameter pack is expanded the user must've
+// provided arguments for all parameters before it.
+FunctionDecl *TemplatedDecl = FunctionTemplate->getTemplatedDecl();
+unsigned FirstPack = Function->getNumParams();
+bool RemoveDefaults = false;
+
+// Go backwards through the template declaration parameters and find the
+// first parameter pack, which has non-zero number of arguments.
+for (unsigned p = TemplatedDecl->getNumParams(); p-- > 0;) {
+  ParmVarDecl *Param = TemplatedDecl->getParamDecl(p);
+
+  if (Param->isParameterPack()) {
+llvm::Optional Args =
+SemaRef.getNumArgumentsInExpansion(Param->getType(), TemplateArgs);
+assert(Args != None && "Unknown number of pack expansion arguments.");
+
+if (Args.getValue() == 0)
+  continue;
+
+FirstPack -= Args.getValue();
+RemoveDefaults = true;
+break;
+  } else {
+FirstPack--;
+  }
+}
+
+// If we found such a parameter pack, then remove default arguments for all
+// parameters before it.
+if (RemoveDefaults) {
+  for (unsigned p = 0; p < FirstPack; p++) {
+ParmVarDecl *Param = Function->getParamDecl(p);
+Param->setDefaultArg(nullptr);
+  }
+}
   } else if (isFriend && D->isThisDeclarationADefinition()) {
 // Do not connect the friend to the template unless it's actually a
 // definition. We don't want

[PATCH] D79800: [Sema] Implement DR2233

2020-05-25 Thread Raul Tambre via Phabricator via cfe-commits 
tambre updated this revision to Diff 265855.
tambre marked an inline comment as done.
tambre added a comment.

Improve comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79800

Files:
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/CXX/drs/dr22xx.cpp
  clang/test/CXX/drs/dr7xx.cpp
  clang/www/cxx_dr_status.html

Index: clang/www/cxx_dr_status.html
===
--- clang/www/cxx_dr_status.html
+++ clang/www/cxx_dr_status.html
@@ -4681,7 +4681,7 @@
 https://wg21.link/cwg777;>777
 CD2
 Default arguments and parameter packs
-Clang 3.7
+Superseded by 2233
   
   
 https://wg21.link/cwg778;>778
@@ -13213,7 +13213,7 @@
 https://wg21.link/cwg2233;>2233
 DRWP
 Function parameter packs following default arguments
-Unknown
+Clang 11
   
   
 https://wg21.link/cwg2234;>2234
Index: clang/test/CXX/drs/dr7xx.cpp
===
--- clang/test/CXX/drs/dr7xx.cpp
+++ clang/test/CXX/drs/dr7xx.cpp
@@ -219,16 +219,4 @@
   Collision c; // expected-note {{in instantiation of}}
 }
 
-namespace dr777 { // dr777: 3.7
-#if __cplusplus >= 201103L
-template 
-void f(int i = 0, T ...args) {}
-void ff() { f(); }
-
-template 
-void g(int i = 0, T ...args, T ...args2) {}
-
-template 
-void h(int i = 0, T ...args, int j = 1) {}
-#endif
-}
+// dr777 superseded by dr2233
Index: clang/test/CXX/drs/dr22xx.cpp
===
--- clang/test/CXX/drs/dr22xx.cpp
+++ clang/test/CXX/drs/dr22xx.cpp
@@ -35,3 +35,44 @@
   }
 #endif
 }
+
+namespace dr2233 { // dr2233: 11
+#if __cplusplus >= 201103L
+template 
+void f(int i = 0, T... args) {}
+
+template 
+void g(int i = 0, T... args, T... args2) {}
+
+template 
+void h(int i = 0, T... args, int j = 1) {}
+
+template 
+void i(int i = 0, T... args, int j = 1, U... args2) {}
+
+template 
+void j(int i = 0, Ts... ts) {}
+
+template <>
+void j(int i, int j) {}
+
+// PR23029
+// Ensure instantiating the templates works.
+void use() {
+  f();
+  f(0, 1);
+  f(1, 2);
+  g(1, 2, 3);
+  h(0, 1);
+  i();
+  i(3);
+  i(3, 2);
+  i(3, 2, 1);
+  i(1, 2, 3, 4, 5);
+  j();
+  j(1);
+  j(1, 2);
+  j(1, 2);
+}
+#endif
+} // namespace dr2233
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -1974,6 +1974,48 @@
 TemplateArgumentList::CreateCopy(SemaRef.Context,
  Innermost),
 /*InsertPos=*/nullptr);
+
+// DR777, DR2233.
+// Parameter packs are allowed after and inbetween parameters with default
+// values. We need to remove default arguments for parameters before the
+// first expanded parameter pack to prevent the declaration being diagnosed
+// as invalid due to the expanded parameters after parameters with default
+// values lacking default values. This is safe to do because if a parameter
+// pack is expanded the user must've provided arguments for all parameters
+// before it.
+FunctionDecl *TemplatedDecl = FunctionTemplate->getTemplatedDecl();
+unsigned FirstPack = Function->getNumParams();
+bool RemoveDefaults = false;
+
+// Go backwards through the template declaration parameters and find the
+// first parameter pack, which has non-zero number of arguments.
+for (unsigned p = TemplatedDecl->getNumParams(); p-- > 0;) {
+  ParmVarDecl *Param = TemplatedDecl->getParamDecl(p);
+
+  if (Param->isParameterPack()) {
+llvm::Optional Args =
+SemaRef.getNumArgumentsInExpansion(Param->getType(), TemplateArgs);
+assert(Args != None && "Unknown number of pack expansion arguments.");
+
+if (Args.getValue() == 0)
+  continue;
+
+FirstPack -= Args.getValue();
+RemoveDefaults = true;
+break;
+  } else {
+FirstPack--;
+  }
+}
+
+// If we found such a parameter pack, then remove default arguments for all
+// parameters before it.
+if (RemoveDefaults) {
+  for (unsigned p = 0; p < FirstPack; p++) {
+ParmVarDecl *Param = Function->getParamDecl(p);
+Param->setDefaultArg(nullptr);
+  }
+}
   } else if (isFriend && D->isThisDeclarationADefinition()) {
 // Do not connect the friend to the template unless it's actually a
 // definition. We don't want non-template functions to be marked as being
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79800: [Sema] Implement DR2233

2020-05-25 Thread Raul Tambre via Phabricator via cfe-commits 
tambre added a comment.

Thanks for the reviews!
I believe this now handles all cases and with this we're standards-conforming 
in regard to DR777 and DR2233.




Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:1987
+
+if (Function->getNumParams() >= NumTemplatedParams) {
+  unsigned FirstDefault = 0;

rjmccall wrote:
> I don't think this simple comparison works; the relationship can be 
> complicated because there might be multiple packs in play.  By the same 
> token, I think it's possible that there can be multiple interleavings of 
> defaultable parameters with packs.  I think you need to scan backwards 
> looking for a parameter without a default argument that was instantiated from 
> a pack expansion and then remove any earlier parameters with defaults.
> 
> That is, assuming this is a reasonable implementation approach at all 
> compared to just teaching other parts of the compiler about this case, which 
> I think Richard needs to weigh in on.
Good point, my current change doesn't work in the following case:


```
template
void g(int i = 1, Ts... ts, int j = 3, As... as)
{
}

void testg()
{
g(2, 3, 4, 5);
}
```

Fixed and added that as a test case.

I think this approach is fine, as keeping info for each parameter for how they 
were expanded doesn't seem reasonable. Cases such as this ought to be checked 
and handled early during the actual template instantiation not during later 
checking of the instantiated function declaration.



Comment at: clang/test/CXX/drs/dr7xx.cpp:225
 template 
 void f(int i = 0, T ...args) {}
 void ff() { f(); }

Quuxplusone wrote:
> rjmccall wrote:
> > Quuxplusone wrote:
> > > Is this even supposed to compile? The only valid specializations of `f` 
> > > require `T...` to be an empty pack, which violates 
> > > [temp.res/8.3](https://timsong-cpp.github.io/cppwp/temp.res#8.3).
> > > 
> > > The comment mentions 
> > > [DR777](http://cwg-issue-browser.herokuapp.com/cwg777), but DR777 doesn't 
> > > explain the circumstances under which its wording change matters. It 
> > > //seems// only to apply to templates that are already ill-formed by 
> > > temp.res/8.3.
> > Yeah, Richard made this point in 
> > [[http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#2233|DR2233]],
> >  and the wording was weakened to allow it, in a way that essentially makes 
> > the earlier default arguments dead.
> Huh. Ick. Indeed the other vendors seem to be implementing DR777/DR2233, so I 
> guess Clang ought to catch up even if it's a silly direction to go in. :( I 
> do see a small bit of implementation divergence in 
> https://godbolt.org/z/ZMCvAX —
> ```
> template
> int f(int i=1, Ts... ts) { return (i + ... + ts); }
> 
> template<>
> int f(int i, int j) { return 42; }
> ```
> GCC rejects as ill-formed. MSVC makes the specialization callable with 2 
> arguments only. EDG (ICC) makes the specialization callable with 0 or 2 
> arguments (and does [crazy things](https://godbolt.org/z/QTrVeh) when you 
> call it with 0 arguments).
MSVC [[ https://godbolt.org/z/FCtrcs | seems to be the most standard-conforming 
]] in this regard. And now with the revised patch Clang is on par. :)

[[ https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95287 | GCC bug ]]
[[ 
https://developercommunity.visualstudio.com/content/problem/1046639/incorrect-intellisense-for-functions-with-a-defaul.html
 | IntelliSense bug ]]

I also submitted bug reports to ICC, but those seem to need moderator approval 
before they're public.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79800



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

[PATCH] D79800: [Sema] Remove default values for arguments prior to a parameter pack if the pack is used

2020-05-21 Thread Raul Tambre via Phabricator via cfe-commits 
tambre added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79800



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

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

2020-05-17 Thread Raul Tambre via Phabricator via cfe-commits 
tambre marked 4 inline comments as done.
tambre added a comment.

Thanks for the reviews and design pointers, John!

There are still a few tests failing, but I'd rather not dive in before the 
general approach is considered acceptable.




Comment at: clang/lib/Sema/SemaDecl.cpp:8880
+  }
+}
+

rjmccall wrote:
> Hmm.  I'm concerned about not doing any sort of semantic compatibility check 
> here before we assign the function special semantics.  Have we really never 
> done those in C++?
> 
> If not, I wonder if we can reasonably make an implicit declaration but just 
> make it hidden from lookup.
Currently there's no semantic compatibility checking for builtin 
redeclarations. There is for initial declarations though.

I've added this checking by splitting the actual builtin declaration creation 
off from `LazilyCreateBuiltin` into `CreateBuiltin` and checking if the current 
declaration is compatible with what the builtin's would be.
This results in stronger typechecking than before for builtin declarations, so 
some incompatible declarations are no longer marked as builtin. See 
`cxx1z-noexcept-function-type.cpp` for an example.



Comment at: clang/lib/Sema/SemaExpr.cpp:6047
   OverloadDecl->setParams(Params);
+  Sema->mergeDeclAttributes(OverloadDecl, FDecl);
   return OverloadDecl;

`rewriteBuiltinFunctionDecl()` creates a new function declaration and happened 
to disregard the builtin's attributes. This caused `BuiltinAttr` to go missing 
in a bunch of OpenCL tests, resulting in failures.



Comment at: clang/test/Sema/warn-fortify-source.c:20
 void *memcpy(void *dst, const void *src, size_t c);
 static void *memcpy(void *dst __attribute__((pass_object_size(1))), const void 
*src, size_t c) __attribute__((overloadable)) __asm__("merp");
 static void *memcpy(void *const dst __attribute__((pass_object_size(1))), 
const void *src, size_t c) __attribute__((overloadable)) {

Not quite sure what to do here. These were previously recognized as builtins 
due to their name despite being incompatible and thus had fortify checking 
similar to the real `memcpy`.

Maybe:
1. Introduce a generic version of `ArmBuiltinAliasAttr`.
2. Something like `FormatAttr`.



Comment at: clang/test/SemaCXX/cxx11-compat.cpp:35
+#define PRIuS "zu"
 int printf(const char *, ...);
 typedef __typeof(sizeof(int)) size_t;

For some reason this `printf` didn't get format checking before. Same for 
`SemaCXX/warn-unused-local-typedef.cpp`.



Comment at: clang/test/SemaCXX/cxx1z-noexcept-function-type.cpp:120
   extern "C" int strcmp(const char *, const char *);
-  extern "C" int strncmp(const char *, const char *, decltype(sizeof(0))) 
noexcept;
+  extern "C" int strncmp(const char *, const char *, decltype(sizeof(0)));
 

This doesn't work anymore since we now properly check builtin declaration 
compatibility and since C++17 noexcept is part of the function type but 
builtins aren't noexcept.
Thoughts?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77491



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

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

2020-05-17 Thread Raul Tambre via Phabricator via cfe-commits 
tambre updated this revision to Diff 264497.
tambre marked an inline comment as not done.
tambre added a comment.

Semantic compatibility checking for C++ builtin redeclarations.
Remove some now unnecessary logic from getBuiltinID().
Update more tests. 4 tests still failing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77491

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/IdentifierTable.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Decl.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/test/AST/ast-dump-attr.cpp
  clang/test/CodeGen/builtin-redeclaration.c
  clang/test/CodeGen/callback_pthread_create.c
  clang/test/CodeGen/ms-intrinsics.c
  clang/test/Sema/implicit-builtin-decl.c
  clang/test/Sema/warn-fortify-source.c
  clang/test/SemaCXX/cxx11-compat.cpp
  clang/test/SemaCXX/cxx1z-noexcept-function-type.cpp
  clang/test/SemaCXX/warn-unused-local-typedef.cpp

Index: clang/test/SemaCXX/warn-unused-local-typedef.cpp
===
--- clang/test/SemaCXX/warn-unused-local-typedef.cpp
+++ clang/test/SemaCXX/warn-unused-local-typedef.cpp
@@ -67,10 +67,10 @@
 
 void test() {
   typedef signed long int superint; // no diag
-  printf("%f", (superint) 42);
+  printf("%ld", (superint)42);
 
   typedef signed long int superint2; // no diag
-  printf("%f", static_cast(42));
+  printf("%ld", static_cast(42));
 
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wunused-local-typedef"
Index: clang/test/SemaCXX/cxx1z-noexcept-function-type.cpp
===
--- clang/test/SemaCXX/cxx1z-noexcept-function-type.cpp
+++ clang/test/SemaCXX/cxx1z-noexcept-function-type.cpp
@@ -117,7 +117,7 @@
 namespace Builtins {
   // Pick two functions that ought to have the same noexceptness.
   extern "C" int strcmp(const char *, const char *);
-  extern "C" int strncmp(const char *, const char *, decltype(sizeof(0))) noexcept;
+  extern "C" int strncmp(const char *, const char *, decltype(sizeof(0)));
 
   // Check we recognized both as builtins.
   typedef int arr[strcmp("bar", "foo") + 4 * strncmp("foo", "bar", 4)];
Index: clang/test/SemaCXX/cxx11-compat.cpp
===
--- clang/test/SemaCXX/cxx11-compat.cpp
+++ clang/test/SemaCXX/cxx11-compat.cpp
@@ -31,7 +31,7 @@
 s = { n }, // expected-warning {{non-constant-expression cannot be narrowed from type 'int' to 'char' in initializer list in C++11}} expected-note {{explicit cast}}
 t = { 1234 }; // expected-warning {{constant expression evaluates to 1234 which cannot be narrowed to type 'char' in C++11}} expected-warning {{changes value}} expected-note {{explicit cast}}
 
-#define PRIuS "uS"
+#define PRIuS "zu"
 int printf(const char *, ...);
 typedef __typeof(sizeof(int)) size_t;
 void h(size_t foo, size_t bar) {
Index: clang/test/Sema/warn-fortify-source.c
===
--- clang/test/Sema/warn-fortify-source.c
+++ clang/test/Sema/warn-fortify-source.c
@@ -1,10 +1,12 @@
 // RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify
-// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_PASS_OBJECT_SIZE
 // RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_BUILTINS
 // RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify
-// RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_PASS_OBJECT_SIZE
 // RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_BUILTINS
 
+// FIXME: Incompatible builtin redeclarations aren't considered builtins and thus don't call the builtin nor inherit their attributes.
+// %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_PASS_OBJECT_SIZE
+// %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_PASS_OBJECT_SIZE
+
 typedef unsigned long size_t;
 
 #ifdef __cplusplus
Index: clang/test/Sema/implicit-builtin-decl.c
===
--- clang/test/Sema/implicit-builtin-decl.c
+++ clang/test/Sema/implicit-builtin-decl.c
@@ -60,12 +60,15 @@
 
 extern float fmaxf(float, float);
 
-struct __jmp_buf_tag {};
-void sigsetjmp(struct __jmp_buf_tag[1], int); // expected-warning{{declaration of built-in function 'sigsetjmp' requires the declaration of the 'jmp_buf' type, commonly provided in the header .}}
+typedef struct __jmp_buf_tag {
+} sigjmp_buf[1];
 
-// CHECK: FunctionDecl {{.*}}  col:6 sigsetjmp '
+int sigsetjmp(struct __jmp_buf_tag[1], int);
+
+// CHECK: FunctionDecl {{.*}}  col:5 implicit sigsetjmp '
+// CHECK: FunctionDecl {{.*}}  col:5 sigsetjmp '
 // CHECK-NOT: FunctionDecl
-// CHECK: ReturnsTwiceAttr {{.*}} <{{.*}}> Implicit
+// CHECK: ReturnsTwiceAttr {{.*}} <{{.*}}>

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

2020-05-16 Thread Raul Tambre via Phabricator via cfe-commits 
tambre updated this revision to Diff 264447.
tambre marked 3 inline comments as done.
tambre added a comment.

Fix adding BuiltinAttr in C++ mode. Update one test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77491

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/IdentifierTable.h
  clang/lib/AST/Decl.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/test/AST/ast-dump-attr.cpp
  clang/test/CodeGen/builtin-redeclaration.c
  clang/test/Sema/implicit-builtin-decl.c

Index: clang/test/Sema/implicit-builtin-decl.c
===
--- clang/test/Sema/implicit-builtin-decl.c
+++ clang/test/Sema/implicit-builtin-decl.c
@@ -60,12 +60,15 @@
 
 extern float fmaxf(float, float);
 
-struct __jmp_buf_tag {};
-void sigsetjmp(struct __jmp_buf_tag[1], int); // expected-warning{{declaration of built-in function 'sigsetjmp' requires the declaration of the 'jmp_buf' type, commonly provided in the header .}}
+typedef struct __jmp_buf_tag {
+} sigjmp_buf[1];
 
-// CHECK: FunctionDecl {{.*}}  col:6 sigsetjmp '
+int sigsetjmp(struct __jmp_buf_tag[1], int);
+
+// CHECK: FunctionDecl {{.*}}  col:5 implicit sigsetjmp '
+// CHECK: FunctionDecl {{.*}}  col:5 sigsetjmp '
 // CHECK-NOT: FunctionDecl
-// CHECK: ReturnsTwiceAttr {{.*}} <{{.*}}> Implicit
+// CHECK: ReturnsTwiceAttr {{.*}} <{{.*}}> Inherited Implicit
 
 // PR40692
 void pthread_create(); // no warning expected
Index: clang/test/CodeGen/builtin-redeclaration.c
===
--- /dev/null
+++ clang/test/CodeGen/builtin-redeclaration.c
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -emit-llvm-only %s
+
+// PR45410
+// Ensure we mark local extern redeclarations with a different type as non-builtin.
+void non_builtin() {
+  extern float exp();
+  exp(); // Will crash due to wrong number of arguments if this calls the builtin.
+}
+
+// PR45410
+// We mark exp() builtin as const with -fno-math-errno (default).
+// We mustn't do that for extern redeclarations of builtins where the type differs.
+float attribute() {
+  extern float exp();
+  return exp(1);
+}
Index: clang/test/AST/ast-dump-attr.cpp
===
--- clang/test/AST/ast-dump-attr.cpp
+++ clang/test/AST/ast-dump-attr.cpp
@@ -109,6 +109,7 @@
 extern "C" int printf(const char *format, ...);
 // CHECK: FunctionDecl{{.*}}printf
 // CHECK-NEXT: ParmVarDecl{{.*}}format{{.*}}'const char *'
+// CHECK-NEXT: BuiltinAttr{{.*}}Implicit
 // CHECK-NEXT: FormatAttr{{.*}}Implicit printf 1 2
 
 alignas(8) extern int x;
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -2108,6 +2108,7 @@
false,
R->isFunctionProtoType());
   New->setImplicit();
+  New->addAttr(BuiltinAttr::CreateImplicit(Context, ID));
 
   // Create Decl objects for each parameter, adding them to the
   // FunctionDecl.
@@ -3330,7 +3331,11 @@
   // there but not here.
   NewTypeInfo = NewTypeInfo.withCallingConv(OldTypeInfo.getCC());
   RequiresAdjustment = true;
-} else if (New->getBuiltinID()) {
+} else if (Old->getBuiltinID()) {
+  // Builtin attribute isn't propagated to the new one yet at this point.
+  // Check if the old is a builtin.
+  // TODO: Maybe we should only warn if the redeclaration is compatible?
+
   // Calling Conventions on a Builtin aren't really useful and setting a
   // default calling convention and cdecl'ing some builtin redeclarations is
   // common, so warn and ignore the calling convention on the redeclaration.
@@ -3756,6 +3761,7 @@
   // If the previous declaration was an implicitly-generated builtin
   // declaration, then at the very least we should use a specialized note.
   unsigned BuiltinID;
+  // TODO: Remove isImplicit check?
   if (Old->isImplicit() && (BuiltinID = Old->getBuiltinID())) {
 // If it's actually a library-defined builtin function like 'malloc'
 // or 'printf', just warn about the incompatible redeclaration.
@@ -8859,6 +8865,20 @@
 if (D.isInvalidType())
   NewFD->setInvalidDecl();
 
+// In C builtins get merged with implicitly lazily created declarations.
+// In C++ we need to check if it's a builtin and add the BuiltinAttr here.
+if (IdentifierInfo *II = Previous.getLookupName().getAsIdentifierInfo()) {
+  if (II->getBuiltinID()) {
+// A builtin needs to have C linkage.
+LinkageSpecDecl *linkage =
+dyn_cast(NewFD->getFirstDecl()->getDeclContext());
+if (linkage && linkage->getLanguage() == LinkageSpecDecl::lang_c) {
+  NewFD->addAttr(
+  BuiltinAttr::CreateImplicit(Context, II->getBuiltinID()));
+}
+

  1   2   >