- User Since
- Apr 18 2013, 6:48 AM (291 w, 6 d)
Tue, Nov 20
Thu, Nov 15
Tue, Nov 13
Thanks for the review! I made adjustments in r346748.
Mon, Nov 12
Fri, Nov 9
Thu, Nov 8
Wed, Nov 7
Thanks for fixing!
Tue, Nov 6
Okay, looks good to me (with a small nit).
Mon, Nov 5
Fri, Nov 2
Assuming the test still passes when you put the EXPORTINLINE filecheck prefix back, looks good to me!
Thanks! I don't think the new isThisDeclarationADefinition() and isImplicitlyInstantiable() checks are right. Besides that, I only have a few more comments about the test.
Thu, Nov 1
Thank you Takuto! I think the functionality looks great now: it's not too complicated :-)
Wed, Oct 31
r345709 should do it.
Bah, I should have known something was going to break if I touched this :-)
Tue, Oct 30
$ g++ -fvisibility=hidden -fvisibility-inlines-hidden -fpic -shared -o lib.so lib.cc $ g++ main.cc lib.so /tmp/cc557J3i.o: In function `S::bar()': main.cc:(.text._ZN1S3barEv[_ZN1S3barEv]+0xd): undefined reference to `foo()'
So this is a known issue with -fvisibility-inlines-hidden. This doesn't come up often in practice, but when it does the developer needs to deal with it.
Yeah, that is the reason of few chromium code changes.
Mon, Oct 29
I've been thinking more about your example with static locals in lambda and how this works with regular dllexport.
Fri, Oct 26
One note about flag ordering: the /clang: flags are concatenated to the end of the argument list, so in cases where the last flag wins, the /clang: flags will be chosen regardless of their order relative to other flags on the driver command line.
Thu, Oct 25
I didn't have time to look at the script yet, but I read through the instructions doc. For most users I think maybe that's also the most important part, because I think many folks build quite differently. I will try to get to reading the script tomorrow.
Tue, Oct 23
Looks good to me.
Oct 19 2018
Looks good to me, thanks!
I haven't started looking at the code yet.
Oct 15 2018
Oct 13 2018
Oct 11 2018
Sorry for the breakage.
Looks good to me. Craig should probably sign off too though.
Looking good, just a few minor comments.
This is looking pretty good, so I'd suggest start writing tests. For testing, take a look at test/CodeGen/X86/switch.ll
Oct 10 2018
Thanks for doing this! I have some comments, and as was pointed out, this needs tests.
Sorry for the slow response.
Thanks! I think this is getting close now.
Here's a version now using cc1 flags, but printing directly from the driver. Please take a look.
Most codebases probably only use either -fvisibility=hidden or
-fvisibility-inlines-hidden, since marking inlines hidden should be a
no-op if everything was already going to be hidden, but it looks like
Chromium uses both.
Oct 9 2018
Oct 5 2018
Oct 4 2018
Oct 3 2018
Thanks for fixing! Please also update the test in test/Driver/cl-options.c and include me on clang-cl code reviews.
Oct 2 2018
Oct 1 2018
Sep 28 2018
This appears to have caused the thread exit code to get clobbered, as shown by some Chromium test failures: https://bugs.chromium.org/p/chromium/issues/detail?id=890310
Hmm, it's strange that we haven't hit this assert before, then. What is causing you to hit it?