[cling][AST] Strip elaborated keyword exposed by partial desugaring - #23060
[cling][AST] Strip elaborated keyword exposed by partial desugaring#23060guitargeek wants to merge 1 commit into
Conversation
GetPartiallyDesugaredTypeImpl removes the elaborated type keyword
('typename', 'struct', ...) of its input type: the keyword is pure
syntax and must never end up in a normalized type name. Since the
LLVM 22 migration removed ElaboratedType, the keyword lives on the
type node itself, and desugaring can expose a *new* one that nothing
strips. With
```c++
template <typename T> struct Container {
typedef typename std::vector<T*> seq_type;
seq_type m_sequential;
};
```
the keyword sits on the TemplateSpecializationType that seq_type
resolves to, so the normalized name of m_sequential came out as
'typename std::vector<Track*>'. TStreamerInfo::BuildOld then fails to
match it against the 'vector<Track*>' stored in the file, sets the new
type to -2 and skips the element, i.e. the member is silently not read.
The leak became visible with 081af22, which attaches the
reconstructed prefix to the TemplateSpecializationType; TypePrinter
prints the keyword whenever it is not None.
Only TemplateSpecializationType is affected; struct/enum elaborated
typedefs already normalize correctly.
Fixes root-project#23055
🤖 Done with the help of AI
hahnjo
left a comment
hahnjo
left a comment
There was a problem hiding this comment.
At first, I debated if I want to spend time trying to understand what the AI did. In the end, I did and my conclusion would be that Cling is not the right place to commit changes based on half-understood problems...
| // a normalized type name. The keyword of the *input* type is taken care of | ||
| // at the top of this function, but desugaring can expose a new one, as in | ||
| // typedef typename std::vector<T*> seq_type; | ||
| // where the keyword sits on the TemplateSpecializationType that the typedef | ||
| // resolves to. |
There was a problem hiding this comment.
What is *input*, where is something like this already "taken care of at the top of this function" (I don't see it), and what happens if there is a typedef pointing to another alias?
|
Can you ask claude to reproduce the behavior through a clang diagnostic (through the type printer) and if that's reproducible that could be a good reason to submit an upstream fix. |
Test Results 23 files 23 suites 5d 0h 9m 13s ⏱️ For more details on these failures, see this check. Results for commit bf98c57. |
3 participants
GetPartiallyDesugaredTypeImpl removes the elaborated type keyword ('typename', 'struct', ...) of its input type: the keyword is pure syntax and must never end up in a normalized type name. Since the LLVM 22 migration removed ElaboratedType, the keyword lives on the type node itself, and desugaring can expose a new one that nothing strips. With
the keyword sits on the TemplateSpecializationType that seq_type resolves to, so the normalized name of m_sequential came out as 'typename std::vector<Track*>'. TStreamerInfo::BuildOld then fails to match it against the 'vector<Track*>' stored in the file, sets the new type to -2 and skips the element, i.e. the member is silently not read.
The leak became visible with 081af22, which attaches the reconstructed prefix to the TemplateSpecializationType; TypePrinter prints the keyword whenever it is not None.
Only TemplateSpecializationType is affected; struct/enum elaborated typedefs already normalize correctly.
Fixes #23055
🤖 Done with the help of AI