You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@orc.apache.org by GitBox <gi...@apache.org> on 2020/08/04 21:07:06 UTC
[GitHub] [orc] nikitamikhaylov opened a new pull request #517: Fix build with clang-10
nikitamikhaylov opened a new pull request #517:
URL: https://github.com/apache/orc/pull/517
You have clang-6 in your CI, but that is too old.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] nikitamikhaylov commented on pull request #517: Fix build with clang-10
Posted by GitBox <gi...@apache.org>.
nikitamikhaylov commented on pull request #517:
URL: https://github.com/apache/orc/pull/517#issuecomment-669054366
:(
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] nikitamikhaylov commented on pull request #517: Fix build with clang-10
Posted by GitBox <gi...@apache.org>.
nikitamikhaylov commented on pull request #517:
URL: https://github.com/apache/orc/pull/517#issuecomment-657784463
> Hi, @nikitamikhaylov . Thank you for your first contribution.
>
> Does this PR include all fixes for clang-10?
After that fixes I've successfully compiled it with clang-10 without any warnings. I hope that is all you need.
You can try to include clang-10 in your CI :)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] nikitamikhaylov edited a comment on pull request #517: Fix build with clang-10
Posted by GitBox <gi...@apache.org>.
nikitamikhaylov edited a comment on pull request #517:
URL: https://github.com/apache/orc/pull/517#issuecomment-668881610
I'm sorry, I didn't mean to offend you with a comment about C++. 😔
For some strange reason I had no notification for comments from this PR. Will do everything tomorrow.
I don't understand what you mean under `**BEFORE**` and `**AFTER**` sections and why I have to include log as `**AFTER**` section.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] nikitamikhaylov closed pull request #517: Fix build with clang-10
Posted by GitBox <gi...@apache.org>.
nikitamikhaylov closed pull request #517:
URL: https://github.com/apache/orc/pull/517
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun commented on pull request #517: Fix build with clang-10
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #517:
URL: https://github.com/apache/orc/pull/517#issuecomment-659831415
For this PR, ORC-647 added macOS 10.15 test to Travis CI with `clang12`. Could you rebase this PR to the master?
```
$ clang --version
90Apple clang version 12.0.0 (clang-1200.0.22.8)
91Target: x86_64-apple-darwin19.5.0
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun commented on pull request #517: Fix build with clang-10
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #517:
URL: https://github.com/apache/orc/pull/517#issuecomment-657783235
Hi, @nikitamikhaylov . Thank you for your first contribution.
Does this PR include all fixes for clang-10?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun edited a comment on pull request #517: Fix build with clang-10
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun edited a comment on pull request #517:
URL: https://github.com/apache/orc/pull/517#issuecomment-668825060
It's unnecessarily insulting to me.
> Do you know C++?
As you said, the followings are just esthetic changes.
> Every statement is separated by ; and ; after { means empty statement, which has no effect.
Thanks for the info.
> P.S. I am using Ubuntu 20 with Clang 10
I closed this PR because I want to reply from you. You didn't reply to the question for a long time.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun commented on pull request #517: Fix build with clang-10
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #517:
URL: https://github.com/apache/orc/pull/517#issuecomment-667712507
I closed this PR because there is no evidence that build is broken in clang-10. Please free free to reopen this with the evidence.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun commented on a change in pull request #517: Fix build with clang-10
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #517:
URL: https://github.com/apache/orc/pull/517#discussion_r454476361
##########
File path: c++/test/TestBloomFilter.cc
##########
@@ -152,9 +152,9 @@ namespace orc {
// test strings
bloomFilter.reset();
- const char * emptyStr = u8"";
- const char * enStr = u8"english";
- const char * cnStr = u8"ä¸å›½å—";
+ const char * emptyStr = "";
+ const char * enStr = "english";
+ const char * cnStr = "ä¸å›½å—";
Review comment:
As you pointed out, CI doesn't run clang-10.
> You have clang-6 in your CI, but that is too old.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] nikitamikhaylov commented on a change in pull request #517: Fix build with clang-10
Posted by GitBox <gi...@apache.org>.
nikitamikhaylov commented on a change in pull request #517:
URL: https://github.com/apache/orc/pull/517#discussion_r454279531
##########
File path: c++/test/TestBloomFilter.cc
##########
@@ -152,9 +152,9 @@ namespace orc {
// test strings
bloomFilter.reset();
- const char * emptyStr = u8"";
- const char * enStr = u8"english";
- const char * cnStr = u8"ä¸å›½å—";
+ const char * emptyStr = "";
+ const char * enStr = "english";
+ const char * cnStr = "ä¸å›½å—";
Review comment:
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p0482r3.html
https://docs.microsoft.com/en-us/cpp/cpp/string-and-character-literals-cpp?view=vs-2019
There was funny warning from clang `type of UTF-8 string literal will change from array of const char to array of const char8_t in C++20`
And your CI said that everything is Ok.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] nikitamikhaylov edited a comment on pull request #517: Fix build with clang-10
Posted by GitBox <gi...@apache.org>.
nikitamikhaylov edited a comment on pull request #517:
URL: https://github.com/apache/orc/pull/517#issuecomment-669054366
:(
This PR was created from my master branch. So lets close it and create new #527
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun edited a comment on pull request #517: Fix build with clang-10
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun edited a comment on pull request #517:
URL: https://github.com/apache/orc/pull/517#issuecomment-658275200
@nikitamikhaylov . Apache ORC has a dockerized testing environment like the following. When I ran this PR on clang9 on Ubuntu 20.04, this didn't pass the test. So, I'm wondering if this is only working on clang10.
- https://github.com/apache/orc/commit/0f33183e116789e6b37e38039d2746f6b59a4f0e
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun closed pull request #517: Fix build with clang-10
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun closed pull request #517:
URL: https://github.com/apache/orc/pull/517
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun commented on pull request #517: Fix build with clang-10
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #517:
URL: https://github.com/apache/orc/pull/517#issuecomment-668825060
It's unnecessarily insulting to me.
> Do you know C++?
As you said, the followings are just esthetic changes.
> Every statement is separated by ; and ; after { means empty statement, which has no effect.
Could you show me your command line log? What was your failure in your environment?
> P.S. I am using Ubuntu 20 with Clang 10
I closed this PR because I want to reply from you. You didn't reply to the question for a long time.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] nikitamikhaylov commented on pull request #517: Fix build with clang-10
Posted by GitBox <gi...@apache.org>.
nikitamikhaylov commented on pull request #517:
URL: https://github.com/apache/orc/pull/517#issuecomment-668709736
> I closed this PR because there is no evidence that build is broken in clang-10. Please free free to reopen this with the evidence.
@dongjoon-hyun Here you are: https://gist.github.com/nikitamikhaylov/18e5f22e81a89f701520558a4c39f30d
Do you know C++? :) Every statement is separated by `;` and `;` after `{` means empty statement, which has no effect.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun commented on pull request #517: Fix build with clang-10
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #517:
URL: https://github.com/apache/orc/pull/517#issuecomment-668826414
Please include your comment on the PR description as `**BEFORE**` section.
- https://github.com/apache/orc/pull/517#issuecomment-668709736
Also, please include the log after this PR on the PR description as `**AFTER**` section.
Thanks for the reply.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun commented on a change in pull request #517: Fix build with clang-10
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #517:
URL: https://github.com/apache/orc/pull/517#discussion_r453929349
##########
File path: c++/test/TestBloomFilter.cc
##########
@@ -152,9 +152,9 @@ namespace orc {
// test strings
bloomFilter.reset();
- const char * emptyStr = u8"";
- const char * enStr = u8"english";
- const char * cnStr = u8"ä¸å›½å—";
+ const char * emptyStr = "";
+ const char * enStr = "english";
+ const char * cnStr = "ä¸å›½å—";
Review comment:
Did you run all tests too?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] nikitamikhaylov edited a comment on pull request #517: Fix build with clang-10
Posted by GitBox <gi...@apache.org>.
nikitamikhaylov edited a comment on pull request #517:
URL: https://github.com/apache/orc/pull/517#issuecomment-668709736
> I closed this PR because there is no evidence that build is broken in clang-10. Please free free to reopen this with the evidence.
@dongjoon-hyun Here you are: https://gist.github.com/nikitamikhaylov/18e5f22e81a89f701520558a4c39f30d
Do you know C++? :) Every statement is separated by `;` and `;` after `{` means empty statement, which has no effect.
And you are using `Apple Clang` - not pure `Clang`. You can install the latter with `brew install llvm`.
P.S. I am using Ubuntu 20 with Clang 10
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] nikitamikhaylov commented on pull request #517: Fix build with clang-10
Posted by GitBox <gi...@apache.org>.
nikitamikhaylov commented on pull request #517:
URL: https://github.com/apache/orc/pull/517#issuecomment-668881610
I'm sorry, I didn't mean to offend you with a comment about C++. 😔
For some strange reason I had no notification for comments from this PR. Will do everything tomorrow.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun commented on pull request #517: Fix build with clang-10
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #517:
URL: https://github.com/apache/orc/pull/517#issuecomment-658275200
@nikitamikhaylov . Apache ORC has a dockerized testing environment like the following. When I run this this PR on clang9 on Ubuntu 20.04, this didn't pass the test. So, I'm wondering if this is only working on clang10.
- https://github.com/apache/orc/commit/0f33183e116789e6b37e38039d2746f6b59a4f0e
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun edited a comment on pull request #517: Fix build with clang-10
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun edited a comment on pull request #517:
URL: https://github.com/apache/orc/pull/517#issuecomment-668825060
It's unnecessarily insulting to me.
> Do you know C++?
As you said, the followings are just esthetic changes.
> Every statement is separated by ; and ; after { means empty statement, which has no effect.
Thanks for the info and the log.
> P.S. I am using Ubuntu 20 with Clang 10
I closed this PR because I want to reply from you. You didn't reply to the question for a long time.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org