This revision was automatically updated to reflect the committed changes.
Closed by commit rL346237: [benchmark] Disable exceptions in Microsoft STL
(authored by eandrews, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
eandrews added a comment.
Thanks!
https://reviews.llvm.org/D52998
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
kbobyrev accepted this revision.
kbobyrev added a comment.
This revision is now accepted and ready to land.
The patch is good to go!
Please add the commit reference to `utils/benchmark/README.LLVM`
(https://github.com/google/benchmark/commit/a9b31c51b1ee7ec7b31438c647123c2cbac5d956)
and the
kbobyrev added a comment.
In https://reviews.llvm.org/D52998#1284312, @eandrews wrote:
> > Upstreaming it first might be better, especially since the change seems to
> > be trivial. Is this line addition the only change proposed for the
> > benchmark library?
>
> Yes this line is the only
eandrews added a comment.
> Upstreaming it first might be better, especially since the change seems to be
> trivial. Is this line addition the only change proposed for the benchmark
> library?
Yes this line is the only change.
> Do you want me to submit the patch to the benchmark library? It
kbobyrev requested changes to this revision.
kbobyrev added a comment.
This revision now requires changes to proceed.
In https://reviews.llvm.org/D52998#1284146, @eandrews wrote:
> @kbobyrev I apologize if I was unclear in the comments. I was asking if the
> changes proposed in the comments
eandrews updated this revision to Diff 172171.
eandrews added a comment.
@kbobyrev I apologize if I was unclear in the comments. I was asking if the
changes proposed in the comments are alright with you since they would involve
modifying `benchmark/CMakelists.txt` (instead of
kbobyrev accepted this revision.
kbobyrev added a comment.
This revision is now accepted and ready to land.
In https://reviews.llvm.org/D52998#1258962, @eandrews wrote:
> Yes. I understand. At the moment, exception handling flags are generated
> based on `BENCHMARK_ENABLE_EXCEPTIONS` in
eandrews added a comment.
In https://reviews.llvm.org/D52998#1259602, @lebedev.ri wrote:
> In https://reviews.llvm.org/D52998#1258962, @eandrews wrote:
>
> > Yes. I understand. At the moment, exception handling flags are generated
> > based on `BENCHMARK_ENABLE_EXCEPTIONS` in
lebedev.ri added a comment.
In https://reviews.llvm.org/D52998#1258962, @eandrews wrote:
> Yes. I understand. At the moment, exception handling flags are generated
> based on `BENCHMARK_ENABLE_EXCEPTIONS` in `utils/benchmark/CMakeLists.txt` .
> However, `_HAS_EXCEPTIONS` is not defined based
eandrews added a comment.
Yes. I understand. At the moment, exception handling flags are generated based
on `BENCHMARK_ENABLE_EXCEPTIONS` in `utils/benchmark/CMakeLists.txt` .
However, `_HAS_EXCEPTIONS` is not defined based on this (code below). The
warnings are a result of this mismatch.
zturner added a comment.
It's not enough to just set `_HAS_EXCEPTIONS=1`, it has to match whatever the
value of `/EH` is passed to the compiler. So if we need to be able to throw
catch exceptions, then you should pass `/EHsc` and not touch `_HAS_EXCEPTIONS`
(technically you could set it to 1
eandrews added a comment.
Thanks for the information Zachary.
@lebedev.ri I do not know how benchmark library developers need/want to handle
exceptions in MSVC STL. If these need to be enabled when
BENCHMARK_ENABLE_EXCEPTIONS is true, I think we can just modify
_HAS_EXCEPTIONS in
zturner added subscribers: eandrews, zturner.
zturner added a comment.
`_HAS_EXCEPTIONS=0` is an undocumented STL specific thing that the library
implementation uses to mean "don't write code that does `throw X;`, do
something else instead".
https://reviews.llvm.org/D52998
`_HAS_EXCEPTIONS=0` is an undocumented STL specific thing that the library
implementation uses to mean "don't write code that does `throw X;`, do
something else instead".
On Mon, Oct 8, 2018 at 2:27 PM Elizabeth Andrews via Phabricator <
revi...@reviews.llvm.org> wrote:
> eandrews added a
eandrews added a comment.
I do not think defining _HAS_EXCEPTIONS=0 affects C++ exceptions in the
application. I thought it only affected the STL. I will verify this and update
you.
https://reviews.llvm.org/D52998
___
cfe-commits mailing list
lebedev.ri added a comment.
Are those warnings about C++ exceptions, or some windows-specific exception
stuff?
It seems the C++ exceptions are used in tests e.g.
https://github.com/google/benchmark/search?q=throw_q=throw
https://github.com/google/benchmark/search?q=catch_q=catch
eandrews created this revision.
eandrews added reviewers: rnk, kbobyrev, omtcyfz, lebedev.ri, erichkeane.
Herald added a subscriber: mgorny.
Define _HAS_EXCEPTIONS=0, when compiling benchmark files, to disable exceptions
in Microsoft STL.
Windows builds were failing due to C4530 warnings (C++
18 matches
Mail list logo
