Re: [PATCH] D41316: [libcxx] Allow random_device to be built optionally

[Zhao, Weiming via cfe-commits]

We can wrap the random_device as a minstd_rand, a linear congruential 
enginer that a lot of C lib uses for rand().
However based on documentation, we should just provides dummy 
implementation which throws an exception in the constructor of 
random_device [1,2]
But again, compared with run-time exception, a link time error is better 
if we simply skip the implementation.


[1]
"explicit random_device(const string& token = implementation-defined );
...
Throws: A value of an implementation-defined type derived from exception 
if the random_device could not be initialized."
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2017/n4659.pdf 
section 29.6.6  Class random_device (page 1082)


[2] "Notice that random devices may not always be available to produce 
random numbers (and in some systems, they may even never be available). 
This is signaled by throwing an exception derived from the standard 
exception  on construction 
 or when a number 
is requested with operator() 
. "   
http://www.cplusplus.com/reference/random/random_device/


Weiming


On 1/2/2018 11:12 AM, Arthur O'Dwyer via Phabricator wrote:

Quuxplusone added a comment.

@weimingz: Since your platform supports `srand(0)`, is it possible to look at how your 
platform implements `srand(0)` and "inline" that implementation into 
`random_device`? That seems like it would be more in keeping with the other ifdefs in 
this file.

I'm confident that constructing an instance of `random_device` MUST NOT 
actually call `srand`. (I'd like to say that it shouldn't even call `rand`.) 
Either of those calls would be observable by the programmer. But there is a 
precedent for e.g. `random_shuffle` making calls to `rand`.


Repository:
   rCXX libc++

https://reviews.llvm.org/D41316





--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by 
The Linux FoundationWe

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

Re: [PATCH] D16171: Warning on redeclaring with a conflicting asm label

[Zhao, Weiming via cfe-commits]

Sorry, I was distracted by other issues after I uploaded the patch.

I will take another look of the implementation.

Thanks,

Weiming


On 9/12/2016 1:31 PM, Sam Shepperd wrote:

phabricss added a comment.

On 09/12/2016 01:26 PM, Nick Lewycky wrote:


Firstly, I thought glibc had applied a patch to fix this bug? As in, the error 
is correct and glibc fixed their bug?


I can confirm that the bug still exists in glibc 2.24 and HEAD from glibc git.  
Also it appears that the issue on the llvm mailing list was just dropped 
without any resolution:

r255371 - Error on redeclaring with a conflicting asm label 


   ../include/sys/stat.h:16:15: error: cannot apply asm label to function after 
its first use
   hidden_proto (__fxstat)
   ~~^
   ./../include/libc-symbols.h:420:19: note: expanded from macro 'hidden_proto'
 __hidden_proto (name, , __GI_##name, ##attrs)
 ^
   ./../include/libc-symbols.h:424:33: note: expanded from macro 
'__hidden_proto'
 extern thread __typeof (name) name __asm__ (__hidden_asmname (#internal)) \

As far as your review of the patch by weimingz, that is beyond my skill level 
so I will let him reply.  Thanks!


https://reviews.llvm.org/D16171





--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by 
The Linux Foundation

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

Re: [PATCH] D21673: [libcxx] guard throw with exception enabling check

[Zhao, Weiming via cfe-commits]

It's a good idea. Currently, there are about 600+  "throws" being 
guarded by _LIBCPP_NO_EXCEPTIONS macro.


How about let's merge the patch now and I can do the conversion of 
existing code to the wrapper in background?


Weiming

On 6/24/2016 1:25 AM, Noel Grandin wrote:

grandinj added a subscriber: grandinj.


Comment at: include/experimental/optional:524
@@ -521,1 +523,3 @@
+assert(!"bad optional access");
+#endif
  return this->__val_;

If this kind of code is going to show up in lots of places, then maybe wrap it 
up in a macro:

_LIBCPP_THROW_OR_ASSERT(bad_optional_access(), "bad optional access")

?

With some extra preprocessor magic, and if LLVM didn't care about the string in 
the assert, could even leave out the second parameter


http://reviews.llvm.org/D21673





--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by 
The Linux Foundation

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

Re: [PATCH] D17741: adds __FILE_BASENAME__ builtin macro

[Zhao, Weiming via cfe-commits]

Sounds good. Will do.

Thanks for reviewing

On 4/8/2016 11:46 AM, Alex Rosenberg wrote:

alexr added a subscriber: alexr.
alexr added a comment.

There are multiple features in this patch that should be considered separately. 
Please split the patch.

That said, I don't think we want to add a fundamental new preprocessor feature 
like __FILE_BASENAME__ without at least getting some early buy-in from the WG14 
and WG21 committees. I suspect they’d not want to add to the preprocessor at 
all.

The flag concept seems much more tractable. Please split it into two flags, one 
to force __FILE__ to only be the basename, and one that handles the prefix 
stripping. That is, a magic value of __ALL_DIR__ seems like the wrong choice 
here.


http://reviews.llvm.org/D17741





--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by 
The Linux Foundation

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

Re: [PATCH] D16171: Warning on redeclaring with a conflicting asm label

[Zhao, Weiming via cfe-commits]

I agree what you said about different code generated with clang and GCC 
generates. In this case, we should throw an error (err_late_asm_label).


But in this example, there is no use of the function. They are just 
redundant declarations and there is no actual code generated.
So I suggest we just give warnings for this case. Otherwise, it will 
break existing code like some SPEC benchmarks.


Please review my 2nd patch.

Weiming

On 1/14/2016 2:28 PM, Nick Lewycky wrote:
On 14 January 2016 at 10:38, Weiming Zhao via cfe-commits 
> wrote:


weimingz added a comment.

Hi Nick,

Below is a reduced code:
t.c:

  static long double acoshl (long double __x) __asm__ ("" "acosh")
;  // this is from
/arm-linux-gnueabi/libc/usr/include/bits/mathcalls.h
  extern long double acoshl (long double) __asm__ (""
"__acoshl_finite") ; // this is from existing code

GCC gives warning like:
/tmp/t.c:2:1: warning: asm declaration ignored due to conflict
with previous rename [-Wpragmas]
 extern long double acoshl (long double) __asm__ (""
"__acoshl_finite") ;


That's the same case as in this testcase:

  void foo() __asm__("one");
  void foo() __asm__("two");
  void test() { foo(); }

GCC emits a call to 'one' while Clang emits a call to 'two'. This is a 
real bug. Please don't downgrade this to a warning.


As an alternative, I would accept a patch which changes how clang 
generates code so that it also produces a call to 'one', with a 
warning. It looks like what we need to do is drop subsequent asm label 
declarations on functions that already have one.


Nick


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