Skip to content

Regression in source fidelity for function return types with qualifiers #55778

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

Jump to bottom
Closed
Abramo-Bagnara opened this issue May 30, 2022 · 8 comments
Closed

Regression in source fidelity for function return types with qualifiers #55778

Abramo-Bagnara opened this issue May 30, 2022 · 8 comments
Labels
bug Indicates an unexpected problem or unintended behavior c11 clang:frontend Language frontend issues, e.g. anything involving "Sema" confirmed Verified by a second party regression

Comments

@Abramo-Bagnara
Copy link
Contributor

After commit d374b65 the syntactic representation of function return type is unexpectedly unqualified in AST, losing the info about qualifiers present in source.
@Abramo-Bagnara Abramo-Bagnara mentioned this issue May 30, 2022
Closed
@EugeneZelenko EugeneZelenko added clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:diagnostics New/improved warning or error message in Clang, but not in clang-tidy or static analyzer and removed new issue labels May 30, 2022
@llvmbot
Copy link
Member

llvmbot commented May 30, 2022

@llvm/issue-subscribers-clang-frontend

@Abramo-Bagnara
Copy link
Contributor Author

Labels added by @EugeneZelenko does not seem right. Am I missing something?

@EugeneZelenko
Copy link
Contributor

@Abramo-Bagnara: Which labels do you consider as proper ones?

@Abramo-Bagnara
Copy link
Contributor Author

clang:frontend is ok, while clang:diagnostics is not.

IMHO also c, bug and regression are correct.

@EugeneZelenko EugeneZelenko added bug Indicates an unexpected problem or unintended behavior regression and removed clang:diagnostics New/improved warning or error message in Clang, but not in clang-tidy or static analyzer labels May 30, 2022
@llvmbot
Copy link
Member

llvmbot commented May 30, 2022

@llvm/issue-subscribers-bug

@AaronBallman AaronBallman added c11 confirmed Verified by a second party labels May 31, 2022
@llvmbot
Copy link
Member

llvmbot commented May 31, 2022

@llvm/issue-subscribers-c11

@AaronBallman
Copy link
Collaborator

I can confirm the issue exists; it's not a particularly user-facing issue, but it will crop up when doing pretty printing or trying to do AST matching, so it'd be nice to resolve this if it's easy to do so.

AaronBallman added a commit that referenced this issue Jun 2, 2022
@AaronBallman
Revert "Drop qualifiers from return types in C (DR423)"
c745f2c 
This reverts commit d374b65.

The changes lose AST fidelity (reported in #55778), but also may be
improperly dropping _Atomic qualifiers. I am rolling the changes back
until I've finished discussions in WG14 about the proper resolution to
DR423.
@AaronBallman
Copy link
Collaborator

Thank you for pushing on this! The resolution to the DR raised questions for me that I'm now asking on the WG14 reflectors. So I'm going to back out the changes from d374b65 and reopen #39595.

The changes were reverted in c745f2c.

Because the changes are backed out, this issue can be closed,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Indicates an unexpected problem or unintended behavior c11 clang:frontend Language frontend issues, e.g. anything involving "Sema" confirmed Verified by a second party regression
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@AaronBallman @EugeneZelenko @llvmbot @Abramo-Bagnara