[Impala-ASF-CR] IMPALA-3909 (follow-up): Properly qualify min() and max() in header
Impala Public Jenkins has submitted this change and it was merged. Change subject: IMPALA-3909 (follow-up): Properly qualify min() and max() in header .. IMPALA-3909 (follow-up): Properly qualify min() and max() in header parquet-column-stats.h was using unqualified references to std::max and std::min, which we disallow in header files. The reason this compiled is that some headers included "names.h". This commit flushes those out as well, and fixes the resulting compilation errors. I tried to make a static_assert method that complained if names.h was included in a header file, but couldn't do so without needing an extra macro, e.g.: USE_IMPORTED_NAMES(); Which seemed too verbose for now. Change-Id: If8381d5b1fa072e89ac1f7f54959c02bf8e43b4b Reviewed-on: http://gerrit.cloudera.org:8080/5916 Reviewed-by: Jim AppleTested-by: Impala Public Jenkins --- M be/src/exec/parquet-column-stats.h M be/src/exec/write-stream.inline.h M be/src/exprs/anyval-util.h M be/src/runtime/fragment-instance-state.cc M be/src/runtime/query-exec-mgr.cc M be/src/runtime/query-state.cc M be/src/service/impala-internal-service.cc M be/src/util/process-state-info.h M be/src/util/uid-util.h 9 files changed, 22 insertions(+), 20 deletions(-) Approvals: Impala Public Jenkins: Verified Jim Apple: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/5916 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: If8381d5b1fa072e89ac1f7f54959c02bf8e43b4b Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Henry Robinson Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple
[Impala-ASF-CR] IMPALA-3909 (follow-up): Properly qualify min() and max() in header
Henry Robinson has posted comments on this change. Change subject: IMPALA-3909 (follow-up): Properly qualify min() and max() in header .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/5916/2//COMMIT_MSG Commit Message: Line 14: I tried to make a static_assert method that complained if names.h was > clang-tidy can check this, IIRC: Can it check if a using statement is transitively included? It wasn't clear from those links. -- To view, visit http://gerrit.cloudera.org:8080/5916 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If8381d5b1fa072e89ac1f7f54959c02bf8e43b4b Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Henry RobinsonGerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Jim Apple Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3909 (follow-up): Properly qualify min() and max() in header
Jim Apple has posted comments on this change. Change subject: IMPALA-3909 (follow-up): Properly qualify min() and max() in header .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/5916/2//COMMIT_MSG Commit Message: Line 14: I tried to make a static_assert method that complained if names.h was > Can it check if a using statement is transitively included? It wasn't clear That I am not sure about. It would require some experimentation, partially because those links were for the HEAD version of documentation, but the toolchain is on version 3.8.1. -- To view, visit http://gerrit.cloudera.org:8080/5916 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If8381d5b1fa072e89ac1f7f54959c02bf8e43b4b Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Henry RobinsonGerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3909 (follow-up): Properly qualify min() and max() in header
Impala Public Jenkins has posted comments on this change. Change subject: IMPALA-3909 (follow-up): Properly qualify min() and max() in header .. Patch Set 2: Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/244/ -- To view, visit http://gerrit.cloudera.org:8080/5916 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If8381d5b1fa072e89ac1f7f54959c02bf8e43b4b Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Henry RobinsonGerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3909 (follow-up): Properly qualify min() and max() in header
Jim Apple has posted comments on this change. Change subject: IMPALA-3909 (follow-up): Properly qualify min() and max() in header .. Patch Set 2: Code-Review+2 (1 comment) Thank you for keeping the code tidy, Henry! http://gerrit.cloudera.org:8080/#/c/5916/2//COMMIT_MSG Commit Message: Line 14: I tried to make a static_assert method that complained if names.h was clang-tidy can check this, IIRC: http://clang.llvm.org/extra/clang-tidy/checks/google-global-names-in-headers.html http://clang.llvm.org/extra/clang-tidy/checks/misc-definitions-in-headers.html -- To view, visit http://gerrit.cloudera.org:8080/5916 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If8381d5b1fa072e89ac1f7f54959c02bf8e43b4b Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Henry RobinsonGerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Jim Apple Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3909 (follow-up): Properly qualify min() and max() in header
Henry Robinson has posted comments on this change. Change subject: IMPALA-3909 (follow-up): Properly qualify min() and max() in header .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/5916/1//COMMIT_MSG Commit Message: Line 7: IMPALA-3909 (follow-up): Properly qualify min() and max() in header > Why? because we require names to be fully qualified in headers. http://gerrit.cloudera.org:8080/#/c/5916/1/be/src/exec/parquet-column-stats.h File be/src/exec/parquet-column-stats.h: Line 106: min_value_ = std::min(min_value_, v); > Also need #include Done -- To view, visit http://gerrit.cloudera.org:8080/5916 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If8381d5b1fa072e89ac1f7f54959c02bf8e43b4b Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Henry RobinsonGerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Jim Apple Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3909 (follow-up): Properly qualify min() and max() in header
Henry Robinson has uploaded a new patch set (#2). Change subject: IMPALA-3909 (follow-up): Properly qualify min() and max() in header .. IMPALA-3909 (follow-up): Properly qualify min() and max() in header parquet-column-stats.h was using unqualified references to std::max and std::min, which we disallow in header files. The reason this compiled is that some headers included "names.h". This commit flushes those out as well, and fixes the resulting compilation errors. I tried to make a static_assert method that complained if names.h was included in a header file, but couldn't do so without needing an extra macro, e.g.: USE_IMPORTED_NAMES(); Which seemed too verbose for now. Change-Id: If8381d5b1fa072e89ac1f7f54959c02bf8e43b4b --- M be/src/exec/parquet-column-stats.h M be/src/exec/write-stream.inline.h M be/src/exprs/anyval-util.h M be/src/runtime/fragment-instance-state.cc M be/src/runtime/query-exec-mgr.cc M be/src/runtime/query-state.cc M be/src/service/impala-internal-service.cc M be/src/util/process-state-info.h M be/src/util/uid-util.h 9 files changed, 22 insertions(+), 20 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/16/5916/2 -- To view, visit http://gerrit.cloudera.org:8080/5916 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: If8381d5b1fa072e89ac1f7f54959c02bf8e43b4b Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Henry RobinsonGerrit-Reviewer: Jim Apple
[Impala-ASF-CR] IMPALA-3909 (follow-up): Properly qualify min() and max() in header
Henry Robinson has uploaded a new change for review. http://gerrit.cloudera.org:8080/5916 Change subject: IMPALA-3909 (follow-up): Properly qualify min() and max() in header .. IMPALA-3909 (follow-up): Properly qualify min() and max() in header Change-Id: If8381d5b1fa072e89ac1f7f54959c02bf8e43b4b --- M be/src/exec/parquet-column-stats.h 1 file changed, 4 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/16/5916/1 -- To view, visit http://gerrit.cloudera.org:8080/5916 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: If8381d5b1fa072e89ac1f7f54959c02bf8e43b4b Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Henry Robinson
[Impala-ASF-CR] IMPALA-3909 (follow-up): Properly qualify min() and max() in header
Jim Apple has posted comments on this change. Change subject: IMPALA-3909 (follow-up): Properly qualify min() and max() in header .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/5916/1//COMMIT_MSG Commit Message: Line 7: IMPALA-3909 (follow-up): Properly qualify min() and max() in header Why? -- To view, visit http://gerrit.cloudera.org:8080/5916 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If8381d5b1fa072e89ac1f7f54959c02bf8e43b4b Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Henry RobinsonGerrit-Reviewer: Jim Apple Gerrit-HasComments: Yes