Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dictionary generation of some classes fails on gcc12 (at least in part due to the use of modules) #11329

Closed
pcanal opened this issue Sep 7, 2022 · 19 comments · Fixed by #11468

Comments

@pcanal
Copy link
Member

pcanal commented Sep 7, 2022

The Failure is visible on the GCC 12 / Fedora 36 build: https://lcgapp-services.cern.ch/root-jenkins/job/root-nightly-master/3285/LABEL=ROOT-fedora36,SPEC=default,V=master/testReport/junit/projectroot.roottest.root.tree/branches/roottest_root_tree_branches_make/
for example:

+Error in <TTree::Branch>: The class requested (vector<int>) for the branch "vec" is an instance of an stl collection and does not have a compiled CollectionProxy. Please generate the dictionary for this collection (vector<int>) to avoid to write corrupted data.

This indicates that the generation of the dictionary for std::vector<int> failed even-though it is explicitly requested as part of the G__Core.cxx dictionary.

And indeed when inspecting the build log we see for G__Core.cxx:

Warning: Unused class rule: vector<Int_t> 

I narrowed down the problem to the presence of:

-cxxmodule -s /home/sftnight/build/manual/build/lib/libCore.so  -m _Builtin_intrinsics -mByproduct _Builtin_intrinsics -mByproduct ROOT_Foundation_Stage1_NoRTTI -mByproduct ROOT_Foundation_C -mByproduct ROOT_Rtypes 

on the command line.

And it can easily be reproduce with the files:

// a.h
#include <vector>

and

// aLinkDef.h 
#pragma link C++ class vector<Int_t>+;

and with

/home/sftnight/build/manual/build/core/rootcling_stage1/src/rootcling_stage1 -v2 -f G__Core.cxx  a.h aLinkDef.h

we get a proper dictionary for the vector. While with

/home/sftnight/build/manual/build/core/rootcling_stage1/src/rootcling_stage1 -v2 -f G__Core.cxx -cxxmodule -s /home/sftnight/build/manual/build/lib/libCore.so  -m _Builtin_intrinsics -mByproduct _Builtin_intrinsics -mByproduct ROOT_Foundation_Stage1_NoRTTI -mByproduct ROOT_Foundation_C -mByproduct ROOT_Rtypes  a.h aLinkDef.h 

we get an empty dictionary and the warning:

Warning: Unused class rule: vector<Int_t> 
@pcanal pcanal added the bug label Sep 7, 2022
@pcanal
Copy link
Member Author

pcanal commented Sep 7, 2022

A few observation:

  • it works without -cxxmodule
  • it fails with -cxxmodule (and the other argument to suppress warning about automatically generating modules)
  • without -cxxmodule, the code that traverse the the translation unit to try to match decls with the selections patterns sees
    • vector
    • vectorROOT::Interna::TSchemaHelper
    • both are seen twice.
  • with -cxxmodule, the code that traverse the the translation unit to try to match decls with the selections patterns sees
    • vector
    • vectorROOT::Interna::TSchemaHelper
    • vector
    • vector<std::__cxx11::basic_string>
    • each are seen twice
    • there is no vector

If I dump the translation unit I see:

grep push_back | grep 1276
| | | |-CXXMethodDecl 0xa59e3a8 <line:1275:7, line:1288:7> line:1276:7 push_back 'void (const std::vector::value_type &)'
| | | |-CXXMethodDecl 0xa675e98 <line:1275:7, line:1288:7> line:1276:7 push_back 'void (const std::vector<ROOT::Internal::TSchemaHelper, std::allocator<ROOT::Internal::TSchemaHelper> >::value_type &)'
| |   |-CXXMethodDecl 0xa7bde58 <line:1275:7, line:1288:7> line:1276:7 push_back 'void (const std::vector<int, std::allocator<int> >::value_type &)'
|-CXXMethodDecl 0xa675e98 <line:1275:7, line:1288:7> line:1276:7 push_back 'void (const std::vector<ROOT::Internal::TSchemaHelper, std::allocator<ROOT::Internal::TSchemaHelper> >::value_type &)'
|-CXXMethodDecl 0xa7bde58 <line:1275:7, line:1288:7> line:1276:7 push_back 'void (const std::vector<int, std::allocator<int> >::value_type &)'
|-CXXMethodDecl 0xa675e98 <line:1275:7, line:1288:7> line:1276:7 push_back 'void (const std::vector<ROOT::Internal::TSchemaHelper, std::allocator<ROOT::Internal::TSchemaHelper> >::value_type &)'
|-CXXMethodDecl 0xa7bde58 <line:1275:7, line:1288:7> line:1276:7 push_back 'void (const std::vector<int, std::allocator<int> >::value_type &)'

in the successful case.

In the failing case I see:

| | | |-CXXMethodDecl 0xed02b08 <line:1275:7, line:1288:7> line:1276:7 imported in std.vector hidden push_back 'void (const std::vector::value_type &)'
| | | |-CXXMethodDecl 0xa1b9830 <line:1275:7, line:1288:7> line:1276:7 in std.vector push_back 'void (const std::vector<ROOT::Internal::TSchemaHelper, std::allocator<ROOT::Internal::TSchemaHelper> >::value_type &)'
| | | |-CXXMethodDecl 0xb33a8c8 <line:1275:7, line:1288:7> line:1276:7 imported in std.regex hidden push_back 'void (const std::vector<int, std::allocator<int> >::value_type &)'
| | | |-CXXMethodDecl 0xee92b98 <line:1275:7, line:1288:7> line:1276:7 imported in std.vector hidden push_back 'void (const std::vector<double, std::allocator<double> >::value_type &)'
| | | |-CXXMethodDecl 0xeea7768 <line:1275:7, line:1288:7> line:1276:7 imported in std.vector hidden push_back 'void (const std::vector<unsigned int, std::allocator<unsigned int> >::value_type &)'
| | | |-CXXMethodDecl 0xeebc838 <line:1275:7, line:1288:7> line:1276:7 imported in std.regex hidden push_back 'void (const std::vector<unsigned long, std::allocator<unsigned long> >::value_type &)'
| |   |-CXXMethodDecl 0xc995488 <line:1275:7, line:1288:7> line:1276:7 imported in std.vector hidden push_back 'void (const std::vector<std::__cxx11::basic_string<char>, std::allocator<std::__cxx11::basic_string<char> > >::value_type &)'
|-CXXMethodDecl 0xa1b9830 <line:1275:7, line:1288:7> line:1276:7 in std.vector push_back 'void (const std::vector<ROOT::Internal::TSchemaHelper, std::allocator<ROOT::Internal::TSchemaHelper> >::value_type &)'
|-CXXMethodDecl 0xee92b98 <line:1275:7, line:1288:7> line:1276:7 imported in std.vector hidden push_back 'void (const std::vector<double, std::allocator<double> >::value_type &)'
|-CXXMethodDecl 0xeea7768 <line:1275:7, line:1288:7> line:1276:7 imported in std.vector hidden push_back 'void (const std::vector<unsigned int, std::allocator<unsigned int> >::value_type &)'
|-CXXMethodDecl 0xc995488 <line:1275:7, line:1288:7> line:1276:7 imported in std.vector hidden push_back 'void (const std::vector<std::__cxx11::basic_string<char>, std::allocator<std::__cxx11::basic_string<char> > >::value_type &)'
|-CXXMethodDecl 0xa1b9830 <line:1275:7, line:1288:7> line:1276:7 in std.vector push_back 'void (const std::vector<ROOT::Internal::TSchemaHelper, std::allocator<ROOT::Internal::TSchemaHelper> >::value_type &)'
|-CXXMethodDecl 0xee92b98 <line:1275:7, line:1288:7> line:1276:7 imported in std.vector hidden push_back 'void (const std::vector<double, std::allocator<double> >::value_type &)'
|-CXXMethodDecl 0xeea7768 <line:1275:7, line:1288:7> line:1276:7 imported in std.vector hidden push_back 'void (const std::vector<unsigned int, std::allocator<unsigned int> >::value_type &)'
|-CXXMethodDecl 0xc995488 <line:1275:7, line:1288:7> line:1276:7 imported in std.vector hidden push_back 'void (const std::vector<std::__cxx11::basic_string<char>, std::allocator<std::

several observations:

a. They are all imported from a module except for std::vectorROOT::Internal::TSchemaHelper
b. we see the string, unsigned int and double as expected
c. we also see vector ... but it does not get to the selector code
d. we also see vector that is not seen by the selector code.
e. the one seen by the selector are marked with imported in std.vector hidden
f. the one NOT seen by the selector are marked with imported in std.regex hidden
g. the one NOT seen by the selector are mentioned only once in the output instead of 3 times for the others.

@pcanal pcanal changed the title Dictionary generation of some classes fails on gcc12 Dictionary generation of some classes fails on gcc12 (at least in part due to the use of modules) Sep 7, 2022
@pcanal
Copy link
Member Author

pcanal commented Sep 7, 2022

The backtrace of the traversal code that does not see the decl for std::vector<int> is :

#0  RScanner::TreatRecordDeclOrTypedefNameDecl (this=0x7fffffffb5c0, typeDecl=0xa1d5918) at /home/sftnight/build/manual/root/core/dictgen/src/Scanner.cxx:677
#1  0x00000000005d7f16 in RScanner::VisitRecordDecl (this=0x7fffffffb5c0, D=0xa1d5918) at /home/sftnight/build/manual/root/core/dictgen/src/Scanner.cxx:527
#2  0x00000000005efaf4 in clang::RecursiveASTVisitor<RScanner>::WalkUpFromRecordDecl (this=0x7fffffffb5c0, D=0xa1d5918)
    at /home/sftnight/build/manual/build/interpreter/llvm/src/tools/clang/include/clang/AST/DeclNodes.inc:253
#3  0x00000000005efbc6 in clang::RecursiveASTVisitor<RScanner>::WalkUpFromCXXRecordDecl (this=0x7fffffffb5c0, D=0xa1d5918)
    at /home/sftnight/build/manual/build/interpreter/llvm/src/tools/clang/include/clang/AST/DeclNodes.inc:257
#4  0x00000000005efd06 in clang::RecursiveASTVisitor<RScanner>::WalkUpFromClassTemplateSpecializationDecl (this=0x7fffffffb5c0, D=0xa1d5918)
    at /home/sftnight/build/manual/build/interpreter/llvm/src/tools/clang/include/clang/AST/DeclNodes.inc:261
#5  0x00000000005e38f2 in clang::RecursiveASTVisitor<RScanner>::TraverseClassTemplateSpecializationDecl (this=0x7fffffffb5c0, D=0xa1d5918)
    at /home/sftnight/build/manual/root/interpreter/llvm/src/tools/clang/include/clang/AST/RecursiveASTVisitor.h:1891
#6  0x00000000005dd442 in clang::RecursiveASTVisitor<RScanner>::TraverseDecl (this=0x7fffffffb5c0, D=0xa1d5918)
    at /home/sftnight/build/manual/build/interpreter/llvm/src/tools/clang/include/clang/AST/DeclNodes.inc:261
#7  0x00000000005ef1f7 in clang::RecursiveASTVisitor<RScanner>::TraverseTemplateInstantiations (this=0x7fffffffb5c0, D=0x9828b70)
    at /home/sftnight/build/manual/root/interpreter/llvm/src/tools/clang/include/clang/AST/RecursiveASTVisitor.h:1670
#8  0x00000000005e2c6c in clang::RecursiveASTVisitor<RScanner>::TraverseClassTemplateDecl (this=0x7fffffffb5c0, D=0x9828b70)
    at /home/sftnight/build/manual/root/interpreter/llvm/src/tools/clang/include/clang/AST/RecursiveASTVisitor.h:1762
#9  0x00000000005dd2aa in clang::RecursiveASTVisitor<RScanner>::TraverseDecl (this=0x7fffffffb5c0, D=0x9828b70)
    at /home/sftnight/build/manual/build/interpreter/llvm/src/tools/clang/include/clang/AST/DeclNodes.inc:201
#10 0x00000000005d9991 in RScanner::TraverseDeclContextHelper (this=0x7fffffffb5c0, DC=0x9dd5c50) at /home/sftnight/build/manual/root/core/dictgen/src/Scanner.cxx:970
#11 0x00000000005e1b29 in clang::RecursiveASTVisitor<RScanner>::TraverseNamespaceDecl (this=0x7fffffffb5c0, D=0x9dd5c20)
    at /home/sftnight/build/manual/root/interpreter/llvm/src/tools/clang/include/clang/AST/RecursiveASTVisitor.h:1513
#12 0x00000000005dd046 in clang::RecursiveASTVisitor<RScanner>::TraverseDecl (this=0x7fffffffb5c0, D=0x9dd5c20)
    at /home/sftnight/build/manual/build/interpreter/llvm/src/tools/clang/include/clang/AST/DeclNodes.inc:105
#13 0x00000000005d9991 in RScanner::TraverseDeclContextHelper (this=0x7fffffffb5c0, DC=0x863dcd0) at /home/sftnight/build/manual/root/core/dictgen/src/Scanner.cxx:970
#14 0x00000000005e73f3 in clang::RecursiveASTVisitor<RScanner>::TraverseTranslationUnitDecl (this=0x7fffffffb5c0, D=0x863dca8)
    at /home/sftnight/build/manual/root/interpreter/llvm/src/tools/clang/include/clang/AST/RecursiveASTVisitor.h:1489
#15 0x00000000005ddccb in clang::RecursiveASTVisitor<RScanner>::TraverseDecl (this=0x7fffffffb5c0, D=0x863dca8)
    at /home/sftnight/build/manual/build/interpreter/llvm/src/tools/clang/include/clang/AST/DeclNodes.inc:577
#16 0x00000000005d9f5c in RScanner::Scan (this=0x7fffffffb5c0, C=...) at /home/sftnight/build/manual/root/core/dictgen/src/Scanner.cxx:1058
#17 0x00000000005a99a0 in RootClingMain (argc=19, argv=0x7fffffffe028, isGenreflex=false) at /home/sftnight/build/manual/root/core/dictgen/src/rootcling_impl.cxx:4806
#18 0x00000000005afd6a in ROOT_rootcling_Driver (argc=19, argv=0x7fffffffe028, config=...) at /home/sftnight/build/manual/root/core/dictgen/src/rootcling_impl.cxx:6147
#19 0x00000000004a4f75 in main (argc=19, argv=0x7fffffffe028) at /home/sftnight/build/manual/root/core/rootcling_stage1/src/rootcling_stage1.cxx:46

@pcanal
Copy link
Member Author

pcanal commented Sep 7, 2022

The problem reproduces on root-fedora36-2 in /home/sftnight/build/manual/build with

/home/sftnight/build/manual/build/core/rootcling_stage1/src/rootcling_stage1 -v2 -f G__Core.cxx  a.h aLinkDef.h # good
/home/sftnight/build/manual/build/core/rootcling_stage1/src/rootcling_stage1 -v2 -f G__Core.cxx -cxxmodule -s /home/sftnight/build/manual/build/lib/libCore.so  -m _Builtin_intrinsics -mByproduct _Builtin_intrinsics -mByproduct ROOT_Foundation_Stage1_NoRTTI -mByproduct ROOT_Foundation_C -mByproduct ROOT_Rtypes  a.h aLinkDef.h # bad

I commented out the printing of the translation unit decl at the moment; the printed translation unit decl are in : rs.bad.04.log rs.good.04.log

@vgvassilev
Copy link
Member

The backtrace of the traversal code that does not see the decl for std::vector<int> is :

#0  RScanner::TreatRecordDeclOrTypedefNameDecl (this=0x7fffffffb5c0, typeDecl=0xa1d5918) at /home/sftnight/build/manual/root/core/dictgen/src/Scanner.cxx:677

Does RScanner::shouldVisitDecl return false for std::vector<int>? I guess we need to understand what makes this declaration not visible for gcc12...

@vgvassilev
Copy link
Member

I think I understand what happens. gcc 12 has a template instantiation coming from the regex header file. That means we do not reinstantiate but deserialize the instantiation upon use but it is still hidden. However, RScanner::shouldVisitDecl ignores declarations from unimported modules which are marked hidden. If I add #include <regex> to a.h the printout is still unhappy but the warning goes away.

I am not sure what'd be the right fix, however, when we use a hidden declaration in a cling::Transaction we should probably make it visible even if we did not have a corresponding include. The alternative is to remove the shouldVisit as I am not sure if I understand well the intent of a785402 however removing it would probably be a headache.

@pcanal
Copy link
Member Author

pcanal commented Sep 13, 2022

One thing I don't understand is what does "invisible" mean in this context and/or why is the invisible decl hidding the visible decl (i.e. The header for vector is explicitly included in my example) and the header contains 2 different way to 'explicitly' instantiate the template).

@vgvassilev
Copy link
Member

I think I added a fix in the build that you used and it might work around this issue. The warning is gone. Can you check if that fixes the issue? I don't think my fix is reliable in the general case but might be good enough...

@pcanal
Copy link
Member Author

pcanal commented Sep 14, 2022

Yes, this seems to solve the immediate problem (but of course, not the underlying issue)

vgvassilev added a commit to vgvassilev/root that referenced this issue Sep 30, 2022
…rt it.

The C++ modules marks the std::vector<int> instantiation as not visible because
it came from the `regex` header file which we did not explicitly include.

root-project/root@a785402 introduces checks if certain declaration is visible
in dictionary generation time which was intending to semantically improve the
coherence by what the user "allowed" (or requested) rootcling to see vs what
it can see globally. While this model works well it seems to not work for
template instantiations as they won't be re-instantiated with visible modifier.

This patch works around the current issue seen with libstdc++ 12 but a better
solution would be to implement a finer grained control over the implicit template
instatiations when generating a dictionary.

Fixes root-project#11329
vgvassilev added a commit that referenced this issue Oct 4, 2022
…rt it.

The C++ modules marks the std::vector<int> instantiation as not visible because
it came from the `regex` header file which we did not explicitly include.

a785402 introduces checks if certain declaration is visible
in dictionary generation time which was intending to semantically improve the
coherence by what the user "allowed" (or requested) rootcling to see vs what
it can see globally. While this model works well it seems to not work for
template instantiations as they won't be re-instantiated with visible modifier.

This patch works around the current issue seen with libstdc++ 12 but a better
solution would be to implement a finer grained control over the implicit template
instatiations when generating a dictionary.

Fixes #11329
@pcanal
Copy link
Member Author

pcanal commented Oct 4, 2022

Did we open a new issue to keep track of the more general issue?

FonsRademakers pushed a commit to root-project/cling that referenced this issue Oct 4, 2022
…rt it.

The C++ modules marks the std::vector<int> instantiation as not visible because
it came from the `regex` header file which we did not explicitly include.

root-project/root@a785402 introduces checks if certain declaration is visible
in dictionary generation time which was intending to semantically improve the
coherence by what the user "allowed" (or requested) rootcling to see vs what
it can see globally. While this model works well it seems to not work for
template instantiations as they won't be re-instantiated with visible modifier.

This patch works around the current issue seen with libstdc++ 12 but a better
solution would be to implement a finer grained control over the implicit template
instatiations when generating a dictionary.

Fixes root-project/root#11329
@vgvassilev
Copy link
Member

Did we open a new issue to keep track of the more general issue?

Please go ahead an open one it might be indeed necessary.

@github-actions
Copy link

github-actions bot commented Oct 5, 2022

Hi @vgvassilev,

It appears this issue is closed, but wasn't yet added to a project. Please add upcoming versions that will include the fix, or 'not applicable' otherwise.

Sincerely,
🤖

5 similar comments
@github-actions
Copy link

github-actions bot commented Oct 6, 2022

Hi @vgvassilev,

It appears this issue is closed, but wasn't yet added to a project. Please add upcoming versions that will include the fix, or 'not applicable' otherwise.

Sincerely,
🤖

@github-actions
Copy link

github-actions bot commented Oct 7, 2022

Hi @vgvassilev,

It appears this issue is closed, but wasn't yet added to a project. Please add upcoming versions that will include the fix, or 'not applicable' otherwise.

Sincerely,
🤖

@github-actions
Copy link

Hi @vgvassilev,

It appears this issue is closed, but wasn't yet added to a project. Please add upcoming versions that will include the fix, or 'not applicable' otherwise.

Sincerely,
🤖

@github-actions
Copy link

Hi @vgvassilev,

It appears this issue is closed, but wasn't yet added to a project. Please add upcoming versions that will include the fix, or 'not applicable' otherwise.

Sincerely,
🤖

@github-actions
Copy link

Hi @vgvassilev,

It appears this issue is closed, but wasn't yet added to a project. Please add upcoming versions that will include the fix, or 'not applicable' otherwise.

Sincerely,
🤖

vgvassilev added a commit to vgvassilev/root that referenced this issue Oct 12, 2022
…rt it.

The C++ modules marks the std::vector<int> instantiation as not visible because
it came from the `regex` header file which we did not explicitly include.

root-project/root@a785402 introduces checks if certain declaration is visible
in dictionary generation time which was intending to semantically improve the
coherence by what the user "allowed" (or requested) rootcling to see vs what
it can see globally. While this model works well it seems to not work for
template instantiations as they won't be re-instantiated with visible modifier.

This patch works around the current issue seen with libstdc++ 12 but a better
solution would be to implement a finer grained control over the implicit template
instatiations when generating a dictionary.

Fixes root-project#11329
@github-actions
Copy link

Hi @vgvassilev,

It appears this issue is closed, but wasn't yet added to a project. Please add upcoming versions that will include the fix, or 'not applicable' otherwise.

Sincerely,
🤖

@github-actions
Copy link

Hi @vgvassilev,

It appears this issue is closed, but wasn't yet added to a project. Please add upcoming versions that will include the fix, or 'not applicable' otherwise.

Sincerely,
🤖

vgvassilev added a commit to vgvassilev/root that referenced this issue Oct 15, 2022
…rt it.

The C++ modules marks the std::vector<int> instantiation as not visible because
it came from the `regex` header file which we did not explicitly include.

root-project/root@a785402 introduces checks if certain declaration is visible
in dictionary generation time which was intending to semantically improve the
coherence by what the user "allowed" (or requested) rootcling to see vs what
it can see globally. While this model works well it seems to not work for
template instantiations as they won't be re-instantiated with visible modifier.

This patch works around the current issue seen with libstdc++ 12 but a better
solution would be to implement a finer grained control over the implicit template
instatiations when generating a dictionary.

Fixes root-project#11329
vgvassilev added a commit to vgvassilev/root that referenced this issue Oct 15, 2022
…rt it.

The C++ modules marks the std::vector<int> instantiation as not visible because
it came from the `regex` header file which we did not explicitly include.

root-project/root@a785402 introduces checks if certain declaration is visible
in dictionary generation time which was intending to semantically improve the
coherence by what the user "allowed" (or requested) rootcling to see vs what
it can see globally. While this model works well it seems to not work for
template instantiations as they won't be re-instantiated with visible modifier.

This patch works around the current issue seen with libstdc++ 12 but a better
solution would be to implement a finer grained control over the implicit template
instatiations when generating a dictionary.

Fixes root-project#11329
Axel-Naumann pushed a commit that referenced this issue Oct 15, 2022
…rt it.

The C++ modules marks the std::vector<int> instantiation as not visible because
it came from the `regex` header file which we did not explicitly include.

a785402 introduces checks if certain declaration is visible
in dictionary generation time which was intending to semantically improve the
coherence by what the user "allowed" (or requested) rootcling to see vs what
it can see globally. While this model works well it seems to not work for
template instantiations as they won't be re-instantiated with visible modifier.

This patch works around the current issue seen with libstdc++ 12 but a better
solution would be to implement a finer grained control over the implicit template
instatiations when generating a dictionary.

Fixes #11329
@github-actions
Copy link

Hi @vgvassilev,

It appears this issue is closed, but wasn't yet added to a project. Please add upcoming versions that will include the fix, or 'not applicable' otherwise.

Sincerely,
🤖

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants