-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[clang] Store FPOptions earlier when parsing function #92146
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
Conversation
After llvm#85605 ([clang] Set correct FPOptions if attribute 'optnone' presents) the current FP options in Sema are saved during parsing function because Sema can modify them if optnone is present. However they were saved too late, it caused fails in some cases when precompiled headers are used. This patch moves the storing earlier.
@llvm/pr-subscribers-clang Author: Serge Pavlov (spavloff) ChangesAfter #85605 ([clang] Set correct FPOptions if attribute 'optnone' presents) the current FP options in Sema are saved during parsing function because Sema can modify them if optnone is present. However they were saved too late, it caused fails in some cases when precompiled headers are used. This patch moves the storing earlier. Full diff: https://github.com/llvm/llvm-project/pull/92146.diff 2 Files Affected:
diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp
index adcbe5858bc78..869b9c6669c27 100644
--- a/clang/lib/Parse/Parser.cpp
+++ b/clang/lib/Parse/Parser.cpp
@@ -1439,6 +1439,8 @@ Decl *Parser::ParseFunctionDefinition(ParsingDeclarator &D,
}
}
+ Sema::FPFeaturesStateRAII SaveFPFeatures(Actions);
+
// Tell the actions module that we have entered a function definition with the
// specified Declarator for the function.
SkipBodyInfo SkipBody;
@@ -1497,8 +1499,6 @@ Decl *Parser::ParseFunctionDefinition(ParsingDeclarator &D,
return Actions.ActOnFinishFunctionBody(Res, nullptr, false);
}
- Sema::FPFeaturesStateRAII SaveFPFeatures(Actions);
-
if (Tok.is(tok::kw_try))
return ParseFunctionTryBlock(Res, BodyScope);
diff --git a/clang/test/PCH/optnone.cpp b/clang/test/PCH/optnone.cpp
new file mode 100644
index 0000000000000..734527b0d3f7d
--- /dev/null
+++ b/clang/test/PCH/optnone.cpp
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -emit-pch -DHEADER -x c++-header %s -o %t.pch
+// RUN: %clang_cc1 -emit-llvm -include-pch %t.pch %s -o /dev/null
+
+#ifdef HEADER
+__attribute__((optnone)) void foo() {}
+#endif
\ No newline at end of file
|
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.
I'm not familiar with the surrounding code, but this fixes the issue and looks reasonable.
clang/test/PCH/optnone.cpp
Outdated
#ifdef HEADER | ||
__attribute__((optnone)) void foo() {} | ||
#endif |
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.
It's not particularly important, but is there a reason for the use of the macros here?
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.
It is to avoid redefinition error. Funny, just this error was observed, because the macro also got into the precompiled header.
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.
Ah, that makes sense since you're using a single file as the source and header - as does the resulting error. New test LGTM.
After #85605 ([clang] Set correct FPOptions if attribute 'optnone' presents) the current FP options in Sema are saved during parsing function because Sema can modify them if optnone is present. However they were saved too late, it caused fails in some cases when precompiled headers are used. This patch moves the storing earlier.