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

Issue 964203002: tools/gn: Fix errors found by Facebook's flint tool. (Closed)

Created:
5 years, 9 months ago by tfarina
Modified:
5 years, 9 months ago
Reviewers:
brettw, scottmg
CC:
chromium-reviews, tfarina, rginda+watch_chromium.org, brettw
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

tools/gn: Fix errors found by Facebook's flint tool. Example of warnings/errors/advices: tools/gn/exec_process.h:5: Missing include guard. If you are ABSOLUTELY sure that you don't want an include guard, then include a comment containing override-include-guard in lieu of an include guard. tools/gn/ninja_binary_target_writer.cc:50: Single-argument constructor 'IncludeWriter(PathOutput& path_output)' may inadvertently be used as a type conversion constructor. Prefix the function with the 'explicit' keyword to avoid this, or add an /* implicit */ comment to suppress this warning. tools/gn/standard_out.cc:98: Advice: Prefer `nullptr' to `NULL' in new C++ code. Unlike `NULL', `nullptr' can't accidentally be used in arithmetic or as an integer. See http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2431.pdf for details. tools/gn/tutorial/hello.cc:7: The associated header file of .cpp files should be included before any other includes. tools/gn/target_generator.h:16: Warning: Base class TargetGenerator has virtual functions but a public non-virtual destructor. The errors were found with the following command line: $ flint --recusive tools/gn And cleaned up until silenced with: $ gn gen out-gn $ ninja -C out-gn gn gn_unittests BUG=None TEST=see above R=scottmg@chromium.org,brettw@chromium.org Committed: https://crrev.com/dc6279e30a3549ecb2fcab0596e05dc59d355d52 Cr-Commit-Position: refs/heads/master@{#318639}

Patch Set 1 #

Patch Set 2 : fix chromium-style plugin errors #

Unified diffs Side-by-side diffs Delta from patch set Stats (+46 lines, -35 lines) Patch
M tools/gn/action_target_generator.h View 1 1 chunk +1 line, -1 line 0 comments Download
M tools/gn/binary_target_generator.h View 1 1 chunk +1 line, -1 line 0 comments Download
M tools/gn/builder.h View 1 chunk +1 line, -1 line 0 comments Download
M tools/gn/c_include_iterator.h View 1 chunk +1 line, -1 line 0 comments Download
M tools/gn/commands.cc View 1 chunk +2 lines, -1 line 0 comments Download
M tools/gn/config_values_extractors.cc View 1 chunk +1 line, -1 line 0 comments Download
M tools/gn/copy_target_generator.h View 1 1 chunk +1 line, -1 line 0 comments Download
M tools/gn/exec_process.h View 2 chunks +7 lines, -0 lines 0 comments Download
M tools/gn/group_target_generator.h View 1 1 chunk +1 line, -1 line 0 comments Download
M tools/gn/input_file.h View 1 chunk +1 line, -1 line 0 comments Download
M tools/gn/input_file_manager.h View 1 chunk +1 line, -1 line 0 comments Download
M tools/gn/loader.h View 1 chunk +1 line, -1 line 0 comments Download
M tools/gn/ninja_binary_target_writer.cc View 1 chunk +1 line, -1 line 0 comments Download
M tools/gn/parse_tree.h View 3 chunks +3 lines, -3 lines 0 comments Download
M tools/gn/pattern.h View 2 chunks +2 lines, -2 lines 0 comments Download
M tools/gn/scope.h View 1 3 chunks +6 lines, -8 lines 0 comments Download
M tools/gn/scope.cc View 1 1 chunk +5 lines, -0 lines 0 comments Download
M tools/gn/scope_per_file_provider.h View 1 1 chunk +1 line, -1 line 0 comments Download
M tools/gn/setup.h View 1 chunk +2 lines, -2 lines 0 comments Download
M tools/gn/standard_out.cc View 1 chunk +1 line, -1 line 0 comments Download
M tools/gn/substitution_pattern.h View 1 chunk +1 line, -1 line 0 comments Download
M tools/gn/target_generator.h View 1 chunk +1 line, -1 line 0 comments Download
M tools/gn/test_with_scope.h View 1 chunk +1 line, -1 line 0 comments Download
M tools/gn/tutorial/hello.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M tools/gn/unique_vector.h View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 21 (9 generated)
tfarina
5 years, 9 months ago (2015-03-01 15:27:46 UTC) #1
brettw
Super! LGTM. It's nice the Facebook tool matches Google-style so closely.
5 years, 9 months ago (2015-03-01 18:47:44 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/964203002/20001
5 years, 9 months ago (2015-03-01 19:47:43 UTC) #6
commit-bot: I haz the power
Try jobs failed on following builders: ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator_ninja/builds/990)
5 years, 9 months ago (2015-03-01 19:52:59 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/964203002/20001
5 years, 9 months ago (2015-03-01 22:07:33 UTC) #10
commit-bot: I haz the power
Try jobs failed on following builders: ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator_ninja/builds/1000)
5 years, 9 months ago (2015-03-01 22:12:41 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/964203002/20001
5 years, 9 months ago (2015-03-01 23:37:57 UTC) #14
commit-bot: I haz the power
Try jobs failed on following builders: ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator_ninja/builds/1014)
5 years, 9 months ago (2015-03-01 23:43:40 UTC) #16
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/964203002/20001
5 years, 9 months ago (2015-03-02 02:05:43 UTC) #18
commit-bot: I haz the power
Committed patchset #2 (id:20001)
5 years, 9 months ago (2015-03-02 03:07:23 UTC) #19
commit-bot: I haz the power
Patchset 2 (id:??) landed as https://crrev.com/dc6279e30a3549ecb2fcab0596e05dc59d355d52 Cr-Commit-Position: refs/heads/master@{#318639}
5 years, 9 months ago (2015-03-02 03:08:13 UTC) #20
scottmg
5 years, 9 months ago (2015-03-02 03:11:28 UTC) #21
Message was sent while issue was closed.
Cool! I'll have to try that tool.

Powered by Google App Engine
This is Rietveld 408576698