Skip to content

[Clang][RFC] Resugar attributed type alias #143143

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.

Sign up for GitHub

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

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

[Clang][RFC] Resugar attributed type alias #143143

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7a418dd
[Clang] Retain type sugar on attributed type declarations
zyn0217 Jun 6, 2025
081acf9
Test?
zyn0217 Jun 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions clang/lib/Sema/SemaType.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6856,6 +6856,8 @@ namespace {
Reference,
MemberPointer,
MacroQualified,
Elaborated,
TypeAlias,
};

QualType Original;
Expand Down Expand Up @@ -6893,6 +6895,12 @@ namespace {
} else if (isa<MacroQualifiedType>(Ty)) {
T = cast<MacroQualifiedType>(Ty)->getUnderlyingType();
Stack.push_back(MacroQualified);
} else if (isa<ElaboratedType>(Ty)) {
T = cast<ElaboratedType>(Ty)->desugar();
Stack.push_back(Elaborated);
} else if (isa<TypedefType>(Ty)) {
T = cast<TypedefType>(Ty)->desugar();
Stack.push_back(TypeAlias);
} else {
const Type *DTy = Ty->getUnqualifiedDesugaredType();
if (Ty == DTy) {
Expand Down Expand Up @@ -6939,6 +6947,7 @@ namespace {
case Desugar:
// This is the point at which we potentially lose source
// information.
// FIXME: Preserve more sugar
return wrap(C, Old->getUnqualifiedDesugaredType(), I);

case Attributed:
Expand Down Expand Up @@ -6998,6 +7007,31 @@ namespace {
else
return C.getRValueReferenceType(New);
}
case Elaborated: {
auto *ET = cast<ElaboratedType>(Old);
return C.getElaboratedType(ET->getKeyword(), ET->getQualifier(),
wrap(C, ET->getNamedType(), I));
}
case TypeAlias: {
auto *ET = cast<TypedefType>(Old);
QualType Underlying = wrap(C, ET->desugar(), I);
TypedefNameDecl *Typedef;
if (auto *TD = dyn_cast<TypedefDecl>(ET->getDecl())) {
Typedef = TypedefDecl::Create(C, TD->getDeclContext(),
TD->getBeginLoc(), TD->getLocation(),
TD->getIdentifier(), nullptr);
Typedef->setModedTypeSourceInfo(TD->getTypeSourceInfo(), Underlying);
} else {
auto *Alias = cast<TypeAliasDecl>(ET->getDecl());
Typedef = TypedefDecl::Create(
C, Alias->getDeclContext(), Alias->getBeginLoc(),
Alias->getLocation(), Alias->getIdentifier(), nullptr);
Typedef->setModedTypeSourceInfo(Alias->getTypeSourceInfo(),
Underlying);
}
Typedef->setPreviousDecl(ET->getDecl());
return C.getTypedefType(Typedef, Underlying);
Comment on lines +7016 to +7033
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
auto *ET = cast<TypedefType>(Old);
QualType Underlying = wrap(C, ET->desugar(), I);
TypedefNameDecl *Typedef;
if (auto *TD = dyn_cast<TypedefDecl>(ET->getDecl())) {
Typedef = TypedefDecl::Create(C, TD->getDeclContext(),
TD->getBeginLoc(), TD->getLocation(),
TD->getIdentifier(), nullptr);
Typedef->setModedTypeSourceInfo(TD->getTypeSourceInfo(), Underlying);
} else {
auto *Alias = cast<TypeAliasDecl>(ET->getDecl());
Typedef = TypedefDecl::Create(
C, Alias->getDeclContext(), Alias->getBeginLoc(),
Alias->getLocation(), Alias->getIdentifier(), nullptr);
Typedef->setModedTypeSourceInfo(Alias->getTypeSourceInfo(),
Underlying);
}
Typedef->setPreviousDecl(ET->getDecl());
return C.getTypedefType(Typedef, Underlying);
return C.getTypedefType(cast<TypedefType>(Old)->getDecl(), Underlying);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise, passing that second 'Underlying' argument would be unnecessary.

The reason we have that second parameter is to support a feature called 'divergent typedefs', which was added a couple of years ago in order to make resugaring more efficient.

This is currently used by the getCommonSugaredType infrastructure as well.

But yeah, this allows you to invent a different underlying type for a TypedefType, without having to create a fake TypedefDecl for it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But yeah, this allows you to invent a different underlying type for a TypedefType, without having to create a fake TypedefDecl for it.

No? There's an assert in that function:

assert(hasSameType(Decl->getUnderlyingType(), Underlying));

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's has same type, not identical types. Are we not just modifying the type sugar here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The context of this resugar is to find the FunctionType and add an attribute (by creating a new function type) on it.

The resugarer tries to add back anything it gets rid of, like the TypedefType in this case.

Or do you know if there's other way around to avoid copying the function type?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, but the new function type is still the same as the original one, it's just not identical (different sugar).

If that's the case, that's still under contract with getTypedefType.

If that's not the case, I think it would be possible to remove that assert, you just need to investigate and deal with the consequences elsewhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I'll take a look

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In both cases, either divergent typedef or creating a new decl, if the types are not the same, you have to deal with these consequences.

For example, getCommonSugaredType expects all TypedefTypes of the same typedef declaration to be the same type.

}
}

llvm_unreachable("unknown wrapping kind");
Expand Down
21 changes: 21 additions & 0 deletions clang/test/SemaCXX/function-type-attributes.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// RUN: %clang_cc1 -fsyntax-only -verify %s

namespace GH142608 {

typedef void (*report_fn)(const char *err);

void die_builtin(const char *err);

__attribute__((noreturn))
report_fn die_routine;

template <class T>
void foo(T, typename T::size = 0); // #foo

void bar() {
foo<__attribute__((noreturn)) report_fn>(die_routine);
// expected-error@-1 {{no matching function}}
// expected-note@#foo {{substitution failure [with T = report_fn]: type 'report_fn' (aka 'void (*)(const char *) __attribute__((noreturn))')}}
}

}
Loading