-
Notifications
You must be signed in to change notification settings - Fork 45
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
[clang] Extend pointer interpretation handling to track explicitness #714
base: dev
Are you sure you want to change the base?
Conversation
We need a new type like DependentPointerType for non-dependent contexts that can track __capability applied to things like typeof(...) in order to have a corresponding TypeLoc for the qualifier, otherwise things like TypeSpecLocFiller get out of sync and, in some cases assert (in other cases silently use the wrong DeclSpec). However, this then exposes the fact TypePrinter's PrintingPolicy's SuppressCapabilityQualifier isn't always able to be set to the right value (e.g. when printing out a reinterpret_cast<T> to the user, or when dumping the AST), and so we end up with redundant __capability qualifiers appearing for purecap code in some cases, and so we need to track alongside the pointer interpretation whether it was explicit (which also needs care to ensure it doesn't mess with canonicalisation). Whilst the output is now noisier in cases where __capability is used in purecap code, this is more faithful to the source. The churn in the AST output due to __capability in the source always being sugar is an unfortunate side-effect, but this should disappear if llvm/llvm-project#65214 is merged. Fixes #710
f2597f7
to
4d71789
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not managed to review the full change yet, but I wonder if we should rename PointerInterpretationKindExplicit
to PointerInterpretationKind
and the give the current enum a new name to reduce the size of the diff and the length of most new lines?
Maybe... I was struggling with naming. Also not quite happy with where it is currently, found some more issues with our handling of things (around type printing when you have multiple redundant qualifiers, and around |
I agree this is definitely a big improvement over the current state :) How about |
We need a new type like DependentPointerType for non-dependent contexts
that can track __capability applied to things like typeof(...) in order
to have a corresponding TypeLoc for the qualifier, otherwise things like
TypeSpecLocFiller get out of sync and, in some cases assert (in other
cases silently use the wrong DeclSpec). However, this then exposes the
fact TypePrinter's PrintingPolicy's SuppressCapabilityQualifier isn't
always able to be set to the right value (e.g. when printing out a
reinterpret_cast to the user, or when dumping the AST), and so we end
up with redundant __capability qualifiers appearing for purecap code in
some cases, and so we need to track alongside the pointer interpretation
whether it was explicit (which also needs care to ensure it doesn't mess
with canonicalisation). Whilst the output is now noisier in cases where
__capability is used in purecap code, this is more faithful to the
source.
The churn in the AST output due to __capability in the source always
being sugar is an unfortunate side-effect, but this should disappear
if llvm/llvm-project#65214 is merged.
Fixes #710