[READY] Hide C++ symbols by default

[bstaletic]

I accidentally closed #826 (force push gone wrong and confused github).
This PR implements the same change but also #827 at the same time.

The boost downgrade is for this change to work properly because of [a bug in the new boost that disallows hidden C++ symbols (i.e. Boost doesn't force the ycm_core to be visible).

---

This change is 

[micbou]

I think it's overkill to create a header file for one definition. This is what I did for YCM_DLL_EXPORT and it was a mistake. It would be more reasonable to define YCM_EXPORT in Utils.h.

---

Reviewed 14 of 14 files at r1.
Review status: 13 of 369 files reviewed at latest revision, 1 unresolved discussion.

---

cpp/CMakeLists.txt, line 266 at r1 (raw file):

      -DYCM_EXPORT=__attribute__\(\(visibility\(\"default\"\)\)\) )
  endif()
endif()

This block should be with the one that sets the -fvisibility=hidden flag.

---

Comments from Reviewable

[codecov-io]

Codecov Report

Merging #848 into master will decrease coverage by 0.17%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #848      +/-   ##
==========================================
- Coverage   94.82%   94.64%   -0.18%     
==========================================
  Files          41       41              
  Lines        4018     4018              
==========================================
- Hits         3810     3803       -7     
- Misses        208      215       +7

[bstaletic]

I moved the definition to Utils.h.

---

Review status: 2 of 370 files reviewed at latest revision, 1 unresolved discussion.

---

cpp/CMakeLists.txt, line 266 at r1 (raw file):

Previously, micbou wrote…

This block should be with the one that sets the -fvisibility=hidden flag.

Done.

---

Comments from Reviewable

[bstaletic]

The latest commit updates cpp/ycm/.ycm_extra_conf.py to reflect this PR's change.

Is it just me or is codecov reporting nonsense?

[Valloric]

Thanks!

---

Review status: 6 of 370 files reviewed at latest revision, all discussions resolved, some commit checks failed.

---

Comments from Reviewable

[zzbot]

☔ The latest upstream changes (presumably #887) made this pull request unmergeable. Please resolve the merge conflicts.

[bstaletic]

I have just force-pushed the update that should make this work with the latest boost.
But I'd advise to wait for the bots to say all the tests have passed.

[micbou]

Some minor comments but .

---

Reviewed 1 of 14 files at r1, 8 of 8 files at r2, 1 of 356 files at r4.
Review status: all files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.

---

cpp/CMakeLists.txt, line 148 at r3 (raw file):

else()
  add_definitions( -DYCM_EXPORT= )
endif()

Missing blank line.

---

cpp/ycm/exceptions.h, line 30 at r4 (raw file):

 * Thrown when a file does not exist.
 */
struct YCM_EXPORT ClangParseError : virtual std::exception {};

Is there a reason to put YCM_EXPORT after struct and not before like we do for classes?

---

Comments from Reviewable

[bstaletic]

Review status: 13 of 14 files reviewed at latest revision, 2 unresolved discussions.

---

cpp/CMakeLists.txt, line 148 at r3 (raw file):

Previously, micbou wrote…

Missing blank line.

Done.

---

cpp/ycm/exceptions.h, line 30 at r4 (raw file):

Previously, micbou wrote…

Is there a reason to put YCM_EXPORT after struct and not before like we do for classes?

There actually is, and you've found the reason last time.

If we do it the logical way compilers* will ignore it for whatever reason. And if it gets ignored, the exception is simply not thrown when calling throw.

* I'm certain GCC will ignore it, not sure about clang.

---

Comments from Reviewable

[micbou]

Let's merge.

@zzbot r+

---

Reviewed 1 of 1 files at r5.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

cpp/ycm/exceptions.h, line 30 at r4 (raw file):

Previously, bstaletic (Boris Staletic) wrote…

There actually is, and you've found the reason last time.

If we do it the logical way compilers* will ignore it for whatever reason. And if it gets ignored, the exception is simply not thrown when calling throw.

* I'm certain GCC will ignore it, not sure about clang.

Right. I vaguely remember now.

---

Comments from Reviewable

[zzbot]

📌 Commit 7aeb56f has been approved by micbou

[zzbot]

⌛ Testing commit 7aeb56f with merge f787e1a...

[zzbot]

☀️ Test successful - status-appveyor, status-travis
Approved by: micbou
Pushing f787e1a to master...