Skip to content

PR for llvm/llvm-project#64605 #627

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

Merged
merged 2 commits into from
Aug 25, 2023
Merged

PR for llvm/llvm-project#64605 #627

merged 2 commits into from
Aug 25, 2023

Conversation

llvmbot
Copy link
Member

@llvmbot llvmbot commented Aug 22, 2023

@llvmbot
Copy link
Member Author

llvmbot commented Aug 22, 2023

@hubert-reinterpretcast What do you think about merging this PR to the release branch?

@tru
Copy link
Contributor

tru commented Aug 22, 2023

This seems to fail the test SemaCXX/template-64605.cpp.

@spavloff
Copy link
Contributor

This PR contains only one commit, llvm/llvm-project@0baf85c. This fails is fixed in another, llvm/llvm-project@73e5a70, which also must be present.

In llvm/llvm-project#64605 (comment) I mentioned two commits, but it seem only one commit is treated by /cherry-pick.

Sorry, I don't know how to merge these commits properly. I tried to push the combined commit to a separate branch of https://github.com/llvm/llvm-project-release-prs but I have no rights. Attemt to prepare the commit in a repository fork also failed.

@tru
Copy link
Contributor

tru commented Aug 22, 2023

You can specify multiple commits by just adding them to the same cherry-pick comment. Like /cherry-pick sha1 sha2

that will merge them to one PR here. If you do it in the same issue as before they will overwrite this one.

When an expression is instantiated, TreeTransform skips ImplicitCastExpr
nodes, assuming they are recreated when the instantiated expression is
built. It breaks functions that use non-default floating-point options,
because they are kept in these ImplicitCastExprs. In this case the
recreated ImplicitCastExpr takes FP options from the current Sema state
and not from AST node.

To fix this issue the FP options in Sema object are set when a compound
statement is cloned in TreeTransform.

This change fixes llvm/llvm-project#64605
([Regression 16 -> 17] Template instantiation ignores FENV_ACCESS being
ON for both definition and instantiation).

Differential Revision: https://reviews.llvm.org/D158158

(cherry picked from commit 0baf85c331090fbe2d2b42214ed0664d55feb0b5)
The test clang/test/SemaCXX/template-64605.cpp uses pragma FENV_ACCESS,
which is not supported on all targets. Restrict it to x86_64 only.

(cherry picked from commit 73e5a70e676850b79f196e01e2194a2485041584)
@spavloff
Copy link
Contributor

The branch https://github.com/llvm/llvm-project-release-prs/tree/llvm-issue64605 contains correct commits. I don't know how to restart the checks.

Copy link
Contributor

@hubert-reinterpretcast hubert-reinterpretcast left a comment

Choose a reason for hiding this comment

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

LGTM!

@tru tru merged commit e54f483 into release/17.x Aug 25, 2023
@tru tru deleted the llvm-issue64605 branch August 25, 2023 07:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Regression 16 -> 17] Template instantiation ignores FENV_ACCESS being ON for both definition and instantiation
4 participants