Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(505)

Issue 1137973002: clang/win plugin: Do not warn on dllexport inline move constructors. (Closed)

Created:
5 years, 7 months ago by Nico
Modified:
5 years, 7 months ago
Reviewers:
hans, dcheng
CC:
chromium-reviews
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

clang/win plugin: Do not warn on dllexport inline move constructors. This is like https://codereview.chromium.org/1129093002 , but for move constructors instead of copy constructors. Also add a test for https://codereview.chromium.org/1129093002 (and this). Since testing dllexport on non-win requires a -target and can't depend on system headers (windows system headers aren't installed where we run tests), convert the test the new test was based on to be standalone (we should do this for all tests, http://crbug.com/486559) BUG=483986, 486559 Committed: https://crrev.com/130ce4bc6a38d2b4b904499c5e14db1841835c2a Cr-Commit-Position: refs/heads/master@{#329208}

Patch Set 1 #

Patch Set 2 : . #

Patch Set 3 : . #

Unified diffs Side-by-side diffs Delta from patch set Stats (+121 lines, -29 lines) Patch
M tools/clang/plugins/FindBadConstructsConsumer.cpp View 1 chunk +9 lines, -2 lines 0 comments Download
M tools/clang/plugins/tests/missing_ctor.h View 4 chunks +21 lines, -9 lines 0 comments Download
M tools/clang/plugins/tests/missing_ctor.cpp View 1 chunk +2 lines, -5 lines 0 comments Download
M tools/clang/plugins/tests/missing_ctor.txt View 1 chunk +5 lines, -5 lines 0 comments Download
A tools/clang/plugins/tests/missing_ctor_dllexport.h View 1 chunk +60 lines, -0 lines 0 comments Download
A + tools/clang/plugins/tests/missing_ctor_dllexport.cpp View 1 chunk +3 lines, -6 lines 0 comments Download
A tools/clang/plugins/tests/missing_ctor_dllexport.flags View 1 chunk +1 line, -0 lines 0 comments Download
A tools/clang/plugins/tests/missing_ctor_dllexport.txt View 1 chunk +14 lines, -0 lines 0 comments Download
M tools/clang/plugins/tests/test.sh View 1 2 1 chunk +6 lines, -2 lines 0 comments Download

Messages

Total messages: 13 (5 generated)
Nico
5 years, 7 months ago (2015-05-10 22:52:00 UTC) #2
Nico
s/me/hans/ in reviewers (how did that happen??)
5 years, 7 months ago (2015-05-10 22:54:01 UTC) #4
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1137973002/40001
5 years, 7 months ago (2015-05-11 01:19:48 UTC) #6
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
5 years, 7 months ago (2015-05-11 01:30:47 UTC) #8
hans
lgtm
5 years, 7 months ago (2015-05-11 18:04:45 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1137973002/40001
5 years, 7 months ago (2015-05-11 18:11:29 UTC) #11
commit-bot: I haz the power
Committed patchset #3 (id:40001)
5 years, 7 months ago (2015-05-11 18:47:37 UTC) #12
commit-bot: I haz the power
5 years, 7 months ago (2015-05-11 18:49:26 UTC) #13
Message was sent while issue was closed.
Patchset 3 (id:??) landed as
https://crrev.com/130ce4bc6a38d2b4b904499c5e14db1841835c2a
Cr-Commit-Position: refs/heads/master@{#329208}

Powered by Google App Engine
This is Rietveld 408576698