[cxxmodules] Introduce fCxxModulesEnabled private member #2700

yamaguchi1024 · 2018-09-27T07:55:13Z

This is a better implementation over doing #ifdef R__USE__CXXMODULES
everywhere.

phsft-bot · 2018-09-27T07:55:24Z

Starting build on slc6/gcc48, slc6-i686/gcc49, centos7/clang39, centos7/gcc62, centos7/gcc7, fedora28/native, ubuntu16/native, mac1013/native, windows10/vc15 with flags -Dccache=ON
How to customize builds

vgvassilev · 2018-09-27T08:07:11Z

core/metacling/src/TCling.h

   TObjArray*      fRootmapFiles;     // Loaded rootmap files.
   Bool_t          fLockProcessLine;  // True if ProcessLine should lock gInterpreterMutex.
   Bool_t          fAllowLibLoad;     // True if library load is allowed (i.e. not in rootcling)
+   Bool_t          fHasCxxModule;     // True if C++ modules was enabled


Could you rename to fCxxModulesEnabled = false and just flip the flag if R__USE_CXXMODULES is set or getenv("ROOT_MODULES") != nullptr?

What's the problem with fHasCxxModule ?

It has no meaning to me. What is has cxxmodule? Does it mean it was built or ROOT is intending to use it? Also the name is inconsistent to the comment next to it ;)

PS: We have cases where we have R__USE_CXXMODULES set but we still have no pcm file which can be misleading if you read the variable name.

yamaguchi1024 · 2018-09-27T08:22:21Z

@phsft-bot build just on slc6/gcc62 with flags -Druntime_cxxmodules=On -Dctest_test_exclude_none=on

phsft-bot · 2018-09-27T08:22:29Z

Starting build on slc6/gcc62 with flags -Dccache=ON -Druntime_cxxmodules=On -Dctest_test_exclude_none=on
How to customize builds

phsft-bot · 2018-09-27T09:42:10Z

Build failed on slc6/gcc62.
See console output.

Errors:

981/1650 Test [cxxmodules] Added missing PushTransactionRAII to rootcling. #912: tutorial-vecops-vo004_SortAndSelect-py ............................................................***Failed Error regular expression found in output. Regex=[: error:] 17.28 sec
983/1650 Test [cxxmodules] Added missing PushTransactionRAII to GetMangledName #913: tutorial-vecops-vo005_Combinations-py .............................................................***Failed Error regular expression found in output. Regex=[: error:] 17.62 sec

Failing tests:

phsft-bot · 2018-09-27T13:09:50Z

Starting build on slc6/gcc62 with flags -Dccache=ON -Druntime_cxxmodules=On -Dctest_test_exclude_none=on
How to customize builds

yamaguchi1024 · 2018-09-27T13:10:09Z

@phsft-bot build!

phsft-bot · 2018-09-27T13:10:17Z

Starting build on slc6/gcc48, slc6-i686/gcc49, centos7/clang39, centos7/gcc62, centos7/gcc7, fedora28/native, ubuntu16/native, mac1013/native, windows10/vc15 with flags -Dccache=ON
How to customize builds

vgvassilev

LGTM!

phsft-bot · 2018-09-27T13:22:54Z

Build failed on fedora28/native.
See console output.

Warnings:

/mnt/build/workspace/root-pullrequests-build/root/interpreter/llvm/src/include/llvm/ADT/SmallVector.h:309:11: warning: ‘void* memcpy(void*, const void*, size_t)’ writing to an object of type ‘struct std::pair<void*, long unsigned int>’ with no trivial copy-assignment; use copy-assignment or copy-initialization instead [-Wclass-memaccess]
/mnt/build/workspace/root-pullrequests-build/root/interpreter/llvm/src/include/llvm/ADT/SmallVector.h:296:13: warning: ‘void* memcpy(void*, const void*, size_t)’ writing to an object of type ‘struct std::pair<clang::CXXRecordDecl*, clang::SourceLocation>’ with no trivial copy-assignment; use copy-assignment or copy-initialization instead [-Wclass-memaccess]
/mnt/build/workspace/root-pullrequests-build/root/interpreter/llvm/src/include/llvm/ADT/SmallVector.h:296:13: warning: ‘void* memcpy(void*, const void*, size_t)’ writing to an object of type ‘struct std::pair<const clang::CXXMethodDecl*, const clang::CXXMethodDecl*>’ with no trivial copy-assignment; use copy-assignment or copy-initialization instead [-Wclass-memaccess]
/mnt/build/workspace/root-pullrequests-build/root/interpreter/llvm/src/include/llvm/ADT/SmallVector.h:296:13: warning: ‘void* memcpy(void*, const void*, size_t)’ writing to an object of type ‘struct std::pair<clang::CXXMethodDecl*, const clang::FunctionProtoType*>’ with no trivial copy-assignment; use copy-assignment or copy-initialization instead [-Wclass-memaccess]
/mnt/build/workspace/root-pullrequests-build/root/interpreter/llvm/src/include/llvm/ADT/SmallVector.h:309:11: warning: ‘void* memcpy(void*, const void*, size_t)’ writing to an object of type ‘struct std::pair<void*, long unsigned int>’ with no trivial copy-assignment; use copy-assignment or copy-initialization instead [-Wclass-memaccess]
/mnt/build/workspace/root-pullrequests-build/root/interpreter/llvm/src/include/llvm/ADT/SmallVector.h:309:11: warning: ‘void* memcpy(void*, const void*, size_t)’ writing to an object of type ‘struct std::pair<void*, long unsigned int>’ with no trivial copy-assignment; use copy-assignment or copy-initialization instead [-Wclass-memaccess]
/mnt/build/workspace/root-pullrequests-build/root/interpreter/llvm/src/include/llvm/ADT/SmallVector.h:309:11: warning: ‘void* memcpy(void*, const void*, size_t)’ writing to an object of type ‘struct std::pair<void*, long unsigned int>’ with no trivial copy-assignment; use copy-assignment or copy-initialization instead [-Wclass-memaccess]
/mnt/build/workspace/root-pullrequests-build/root/interpreter/llvm/src/include/llvm/ADT/SmallVector.h:309:11: warning: ‘void* memcpy(void*, const void*, size_t)’ writing to an object of type ‘struct std::pair<void*, long unsigned int>’ with no trivial copy-assignment; use copy-assignment or copy-initialization instead [-Wclass-memaccess]
/mnt/build/workspace/root-pullrequests-build/root/interpreter/llvm/src/include/llvm/ADT/SmallVector.h:309:11: warning: ‘void* memcpy(void*, const void*, size_t)’ writing to an object of type ‘struct std::pair<void*, long unsigned int>’ with no trivial copy-assignment; use copy-assignment or copy-initialization instead [-Wclass-memaccess]
/mnt/build/workspace/root-pullrequests-build/root/interpreter/llvm/src/include/llvm/ADT/SmallVector.h:296:13: warning: ‘void* memcpy(void*, const void*, size_t)’ writing to an object of type ‘struct std::pair<clang::CXXRecordDecl*, clang::SourceLocation>’ with no trivial copy-assignment; use copy-assignment or copy-initialization instead [-Wclass-memaccess]

And 25 more

yamaguchi1024 · 2018-09-27T13:44:29Z

What!? So it was actually this change which was upsetting fedora...

phsft-bot · 2018-09-27T13:59:31Z

Starting build on slc6/gcc48, slc6-i686/gcc49, centos7/clang39, centos7/gcc62, centos7/gcc7, fedora28/native, ubuntu16/native, mac1013/native, windows10/vc15 with flags -Dccache=ON
How to customize builds

This is a better implementation over doing #ifdef R__USE__CXXMODULES everywhere.

phsft-bot · 2018-09-27T14:11:02Z

Build failed on mac1013/native.
See console output.

yamaguchi1024 requested a review from vgvassilev September 27, 2018 07:55

yamaguchi1024 requested a review from pcanal as a code owner September 27, 2018 07:55

vgvassilev reviewed Sep 27, 2018

View reviewed changes

yamaguchi1024 force-pushed the fhascxx branch from ccfcd55 to a2b280a Compare September 27, 2018 13:09

yamaguchi1024 changed the title ~~[cxxmodules] Introduce fHasCxxModule private member~~ [cxxmodules] Introduce fCxxModulesEnabled private member Sep 27, 2018

vgvassilev approved these changes Sep 27, 2018

View reviewed changes

yamaguchi1024 force-pushed the fhascxx branch from a2b280a to 69f78e2 Compare September 27, 2018 13:59

vgvassilev merged commit 80b1a73 into root-project:master Sep 27, 2018

[cxxmodules] Introduce fCxxModulesEnabled private member

69f78e2

This is a better implementation over doing #ifdef R__USE__CXXMODULES everywhere.

yamaguchi1024 deleted the fhascxx branch September 27, 2018 22:17

[cxxmodules] Introduce fCxxModulesEnabled private member #2700

[cxxmodules] Introduce fCxxModulesEnabled private member #2700

Uh oh!

Conversation

yamaguchi1024 commented Sep 27, 2018

Uh oh!

phsft-bot commented Sep 27, 2018

Uh oh!

vgvassilev Sep 27, 2018

Choose a reason for hiding this comment

Uh oh!

yamaguchi1024 Sep 27, 2018

Choose a reason for hiding this comment

Uh oh!

vgvassilev Sep 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yamaguchi1024 commented Sep 27, 2018

Uh oh!

phsft-bot commented Sep 27, 2018

Uh oh!

phsft-bot commented Sep 27, 2018

Errors:

Failing tests:

Uh oh!

phsft-bot commented Sep 27, 2018

Uh oh!

yamaguchi1024 commented Sep 27, 2018

Uh oh!

phsft-bot commented Sep 27, 2018

Uh oh!

vgvassilev left a comment

Choose a reason for hiding this comment

Uh oh!

phsft-bot commented Sep 27, 2018

Warnings:

Uh oh!

yamaguchi1024 commented Sep 27, 2018

Uh oh!

phsft-bot commented Sep 27, 2018

Uh oh!

phsft-bot commented Sep 27, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

vgvassilev Sep 27, 2018 •

edited

Loading