Ariel-Burton added inline comments.
================
Comment at: clang/lib/Sema/SemaType.cpp:7116
+ for (;;) {
+ if (const TypedefType *TT = dyn_cast<TypedefType>(Desugared)) {
+ Desugared = TT->desugar();
----------------
rnk wrote:
> Ariel-Burton wrote:
> > rnk wrote:
> > > This seems like a good place to use getSingleStepDesugaredType to look
> > > through all type sugar (parens, typedefs, template substitutions, etc).
> > > This seems like a good place to use getSingleStepDesugaredType to look
> > > through all type sugar (parens, typedefs, template substitutions, etc).
> >
> > I'm not sure what you mean. Could you expand a little, please?
> Clang's AST has lots of "type sugar nodes". These are types which usually
> don't have any semantic meaning, they just carry source location information,
> like whether there was a typedef or extra parens in the type. AttributedType
> is also a type sugar node, so we cannot do a full desugaring here, we have to
> step through each node one at a time to accumulate the attributes.
>
> Your code looks through one kind of type sugar, but this loop should probably
> be generalized to handle all kinds of type sugar. I think
> getSingleStepDesugaredType will do that.
Thanks for the clarification. Do you mean something like:
```
for (;;) {
const AttributedType *AT = dyn_cast<AttributedType>(Desugared);
if (AT) {
Attrs[AT->getAttrKind()] = true;
Desugared = AT->getModifiedType();
} else {
QualType QT = Desugared.getSingleStepDesugaredType(S.Context);
if (Desugared != QT) {
Desugared = QT;
} else {
break;
}
}
}
```
I don't think that we can use getSingleStepDesugaredType indiscriminately. Not
all sugar should be peeled away, for example, in the case of of parentheses:
```
int *(__ptr32 wrong);
```
is accepted when it shouldn't.
To check for the cases where we don't want to desugar has the same sort of
complexity as checking for the cases where we do. I suggest going with the
original proposal.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D130123/new/
https://reviews.llvm.org/D130123
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
