Re: [PATCH] D19815: Support '#pragma once' in headers when using PCH
This revision was automatically updated to reflect the committed changes.
Closed by commit rL276653: Support '#pragma once' in headers when using PCH
(authored by ssrivastava).
Changed prior to commit:
https://reviews.llvm.org/D19815?vs=57320=65372#toc
Repository:
rL LLVM
https://reviews.llvm.org/D19815
Files:
cfe/trunk/lib/Lex/Pragma.cpp
cfe/trunk/test/PCH/Inputs/pragma-once.h
cfe/trunk/test/PCH/pragma-once.c
Index: cfe/trunk/test/PCH/Inputs/pragma-once.h
===
--- cfe/trunk/test/PCH/Inputs/pragma-once.h
+++ cfe/trunk/test/PCH/Inputs/pragma-once.h
@@ -0,0 +1,5 @@
+#pragma once
+
+/* For use with the pragma-once.c test */
+
+int x = 3;
Index: cfe/trunk/test/PCH/pragma-once.c
===
--- cfe/trunk/test/PCH/pragma-once.c
+++ cfe/trunk/test/PCH/pragma-once.c
@@ -0,0 +1,13 @@
+// Test this without pch.
+// RUN: %clang_cc1 -include %S/Inputs/pragma-once.h -fsyntax-only -verify %s
+
+// Test with pch.
+// RUN: %clang_cc1 -emit-pch -o %t %S/Inputs/pragma-once.h
+// RUN: %clang_cc1 -include-pch %t -fsyntax-only -verify %s
+
+// expected-no-diagnostics
+
+// Including "pragma-once.h" twice, to verify the 'once' aspect is honored.
+#include "Inputs/pragma-once.h"
+#include "Inputs/pragma-once.h"
+int foo(void) { return 0; }
Index: cfe/trunk/lib/Lex/Pragma.cpp
===
--- cfe/trunk/lib/Lex/Pragma.cpp
+++ cfe/trunk/lib/Lex/Pragma.cpp
@@ -352,7 +352,9 @@
/// HandlePragmaOnce - Handle \#pragma once. OnceTok is the 'once'.
///
void Preprocessor::HandlePragmaOnce(Token ) {
- if (isInPrimaryFile()) {
+ // Don't honor the 'once' when handling the primary source file, unless
+ // this is a prefix to a TU, which indicates we're generating a PCH file.
+ if (isInPrimaryFile() && TUKind != TU_Prefix) {
Diag(OnceTok, diag::pp_pragma_once_in_main_file);
return;
}
Index: cfe/trunk/test/PCH/Inputs/pragma-once.h
===
--- cfe/trunk/test/PCH/Inputs/pragma-once.h
+++ cfe/trunk/test/PCH/Inputs/pragma-once.h
@@ -0,0 +1,5 @@
+#pragma once
+
+/* For use with the pragma-once.c test */
+
+int x = 3;
Index: cfe/trunk/test/PCH/pragma-once.c
===
--- cfe/trunk/test/PCH/pragma-once.c
+++ cfe/trunk/test/PCH/pragma-once.c
@@ -0,0 +1,13 @@
+// Test this without pch.
+// RUN: %clang_cc1 -include %S/Inputs/pragma-once.h -fsyntax-only -verify %s
+
+// Test with pch.
+// RUN: %clang_cc1 -emit-pch -o %t %S/Inputs/pragma-once.h
+// RUN: %clang_cc1 -include-pch %t -fsyntax-only -verify %s
+
+// expected-no-diagnostics
+
+// Including "pragma-once.h" twice, to verify the 'once' aspect is honored.
+#include "Inputs/pragma-once.h"
+#include "Inputs/pragma-once.h"
+int foo(void) { return 0; }
Index: cfe/trunk/lib/Lex/Pragma.cpp
===
--- cfe/trunk/lib/Lex/Pragma.cpp
+++ cfe/trunk/lib/Lex/Pragma.cpp
@@ -352,7 +352,9 @@
/// HandlePragmaOnce - Handle \#pragma once. OnceTok is the 'once'.
///
void Preprocessor::HandlePragmaOnce(Token ) {
- if (isInPrimaryFile()) {
+ // Don't honor the 'once' when handling the primary source file, unless
+ // this is a prefix to a TU, which indicates we're generating a PCH file.
+ if (isInPrimaryFile() && TUKind != TU_Prefix) {
Diag(OnceTok, diag::pp_pragma_once_in_main_file);
return;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D19815: Support '#pragma once' in headers when using PCH
wristow added a comment. > @wristow, if this still patches cleanly against trunk (I see that this has > been around a while) and test still pass, think this can be committed > soon-ish? It does still apply cleanly, and testing still looks good. Given that the test was adjusted as suggested by Reid, and the implementation was simplified as Richard suggested, even though you've only recently registered, I'll commit this. (Actually, I don't have commit access, but one of my colleagues will commit it soon.) Thanks! https://reviews.llvm.org/D19815 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D19815: Support '#pragma once' in headers when using PCH
elsteveogrande added a comment. Looks like I have the "accept" ability in this repo, wasn't sure since I just registered for this recently :) I think since the tests look good, it's a nice and elegant 1-line change, per the reviewer's (@rsmith 's) comments, this should be ok to go. This would help greatly on a project I'm working on, which has reams of auto-generated headers, so naturally I'm antsy to get this in trunk :) @wristow, if this still lands cleanly against trunk (I see that this has been around a while) and test still pass, think this can be committed soon-ish? Thanks! https://reviews.llvm.org/D19815 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D19815: Support '#pragma once' in headers when using PCH
wristow added a comment. ping http://reviews.llvm.org/D19815 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D19815: Support '#pragma once' in headers when using PCH
wristow updated this revision to Diff 57320.
wristow added a comment.
Simplified he implementation, by using the existing `TUKind` field.
http://reviews.llvm.org/D19815
Files:
llvm/tools/clang/lib/Lex/Pragma.cpp
llvm/tools/clang/test/PCH/Inputs/pragma-once.h
llvm/tools/clang/test/PCH/pragma-once.c
Index: llvm/tools/clang/test/PCH/pragma-once.c
===
--- /dev/null
+++ llvm/tools/clang/test/PCH/pragma-once.c
@@ -0,0 +1,13 @@
+// Test this without pch.
+// RUN: %clang_cc1 -include %S/Inputs/pragma-once.h -fsyntax-only -verify %s
+
+// Test with pch.
+// RUN: %clang_cc1 -emit-pch -o %t %S/Inputs/pragma-once.h
+// RUN: %clang_cc1 -include-pch %t -fsyntax-only -verify %s
+
+// expected-no-diagnostics
+
+// Including "pragma-once.h" twice, to verify the 'once' aspect is honored.
+#include "Inputs/pragma-once.h"
+#include "Inputs/pragma-once.h"
+int foo(void) { return 0; }
Index: llvm/tools/clang/test/PCH/Inputs/pragma-once.h
===
--- /dev/null
+++ llvm/tools/clang/test/PCH/Inputs/pragma-once.h
@@ -0,0 +1,5 @@
+#pragma once
+
+/* For use with the pragma-once.c test */
+
+int x = 3;
Index: llvm/tools/clang/lib/Lex/Pragma.cpp
===
--- llvm/tools/clang/lib/Lex/Pragma.cpp
+++ llvm/tools/clang/lib/Lex/Pragma.cpp
@@ -354,7 +354,9 @@
/// HandlePragmaOnce - Handle \#pragma once. OnceTok is the 'once'.
///
void Preprocessor::HandlePragmaOnce(Token ) {
- if (isInPrimaryFile()) {
+ // Don't honor the 'once' when handling the primary source file, unless
+ // this is a prefix to a TU, which indicates we're generating a PCH file.
+ if (isInPrimaryFile() && TUKind != TU_Prefix) {
Diag(OnceTok, diag::pp_pragma_once_in_main_file);
return;
}
Index: llvm/tools/clang/test/PCH/pragma-once.c
===
--- /dev/null
+++ llvm/tools/clang/test/PCH/pragma-once.c
@@ -0,0 +1,13 @@
+// Test this without pch.
+// RUN: %clang_cc1 -include %S/Inputs/pragma-once.h -fsyntax-only -verify %s
+
+// Test with pch.
+// RUN: %clang_cc1 -emit-pch -o %t %S/Inputs/pragma-once.h
+// RUN: %clang_cc1 -include-pch %t -fsyntax-only -verify %s
+
+// expected-no-diagnostics
+
+// Including "pragma-once.h" twice, to verify the 'once' aspect is honored.
+#include "Inputs/pragma-once.h"
+#include "Inputs/pragma-once.h"
+int foo(void) { return 0; }
Index: llvm/tools/clang/test/PCH/Inputs/pragma-once.h
===
--- /dev/null
+++ llvm/tools/clang/test/PCH/Inputs/pragma-once.h
@@ -0,0 +1,5 @@
+#pragma once
+
+/* For use with the pragma-once.c test */
+
+int x = 3;
Index: llvm/tools/clang/lib/Lex/Pragma.cpp
===
--- llvm/tools/clang/lib/Lex/Pragma.cpp
+++ llvm/tools/clang/lib/Lex/Pragma.cpp
@@ -354,7 +354,9 @@
/// HandlePragmaOnce - Handle \#pragma once. OnceTok is the 'once'.
///
void Preprocessor::HandlePragmaOnce(Token ) {
- if (isInPrimaryFile()) {
+ // Don't honor the 'once' when handling the primary source file, unless
+ // this is a prefix to a TU, which indicates we're generating a PCH file.
+ if (isInPrimaryFile() && TUKind != TU_Prefix) {
Diag(OnceTok, diag::pp_pragma_once_in_main_file);
return;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D19815: Support '#pragma once' in headers when using PCH
rsmith added a comment. Rather than threading through a new flag, can you test the existing `TUKind` field? http://reviews.llvm.org/D19815 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D19815: Support '#pragma once' in headers when using PCH
wristow added a comment. Ping http://reviews.llvm.org/D19815 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D19815: Support '#pragma once' in headers when using PCH
rnk added a subscriber: rnk. rnk added a comment. I think threading this through PP options is reasonable. Comment at: test/PCH/pragma-once.h:1 @@ +1,2 @@ +#pragma once + This should be in test/PCH/Inputs http://reviews.llvm.org/D19815 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D19815: Support '#pragma once' in headers when using PCH
wristow added a comment. To check for whether we're in "generate a PCH file mode", I added a new flag (`GeneratePCHMode`) to `PreprocessorOptions`. If the `CompilerInstance` had been visible in lexical analysis, it would have been easy to do this without adding a new flag. Is there a better way to determine whether Clang is generating a PCH file? One alternative approach considered was to put a reference to `FrontendOptions` into the `Preprocessor` class (it already includes a reference to `LangOpts`, for example). http://reviews.llvm.org/D19815 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D19815: Support '#pragma once' in headers when using PCH
wristow created this revision.
wristow added a reviewer: rsmith.
wristow added a subscriber: cfe-commits.
The '#pragma once' directive was erroneously ignored when encountered
in the header-file specified in generate-PCH-mode. This resulted in
compile-time errors in some cases with legal code, and also a misleading
warning being produced.
This fixes PR24387.
http://reviews.llvm.org/D19815
Files:
include/clang/Lex/PreprocessorOptions.h
lib/Frontend/InitPreprocessor.cpp
lib/Lex/Pragma.cpp
test/PCH/pragma-once.c
test/PCH/pragma-once.h
Index: test/PCH/pragma-once.h
===
--- /dev/null
+++ test/PCH/pragma-once.h
@@ -0,0 +1,5 @@
+#pragma once
+
+/* For use with the pragma-once.c test */
+
+int x = 3;
Index: test/PCH/pragma-once.c
===
--- /dev/null
+++ test/PCH/pragma-once.c
@@ -0,0 +1,13 @@
+// Test this without pch.
+// RUN: %clang_cc1 -include %S/pragma-once.h -fsyntax-only -verify %s
+
+// Test with pch.
+// RUN: %clang_cc1 -emit-pch -o %t %S/pragma-once.h
+// RUN: %clang_cc1 -include-pch %t -fsyntax-only -verify %s
+
+// expected-no-diagnostics
+
+// Including "pragma-once.h" twice, to verify the 'once' aspect is honored.
+#include "pragma-once.h"
+#include "pragma-once.h"
+int foo(void) { return 0; }
Index: lib/Lex/Pragma.cpp
===
--- lib/Lex/Pragma.cpp
+++ lib/Lex/Pragma.cpp
@@ -19,6 +19,7 @@
#include "clang/Lex/LexDiagnostic.h"
#include "clang/Lex/LiteralSupport.h"
#include "clang/Lex/MacroInfo.h"
+#include "clang/Lex/PreprocessorOptions.h"
#include "clang/Lex/Preprocessor.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/StringSwitch.h"
@@ -354,7 +355,9 @@
/// HandlePragmaOnce - Handle \#pragma once. OnceTok is the 'once'.
///
void Preprocessor::HandlePragmaOnce(Token ) {
- if (isInPrimaryFile()) {
+ // Don't honor the 'once' when handling the primary source file, unless
+ // we're generating a PCH file.
+ if (isInPrimaryFile() && !getPreprocessorOpts().GeneratePCHMode) {
Diag(OnceTok, diag::pp_pragma_once_in_main_file);
return;
}
Index: lib/Frontend/InitPreprocessor.cpp
===
--- lib/Frontend/InitPreprocessor.cpp
+++ lib/Frontend/InitPreprocessor.cpp
@@ -1021,6 +1021,10 @@
if (!InitOpts.ImplicitPTHInclude.empty())
AddImplicitIncludePTH(Builder, PP, InitOpts.ImplicitPTHInclude);
+ // Instruct the preprocessor that we're generating a PCH file.
+ if (FEOpts.ProgramAction == frontend::GeneratePCH)
+PP.getPreprocessorOpts().GeneratePCHMode = true;
+
// Process -include directives.
for (unsigned i = 0, e = InitOpts.Includes.size(); i != e; ++i) {
const std::string = InitOpts.Includes[i];
Index: include/clang/Lex/PreprocessorOptions.h
===
--- include/clang/Lex/PreprocessorOptions.h
+++ include/clang/Lex/PreprocessorOptions.h
@@ -61,6 +61,9 @@
/// \brief Headers that will be converted to chained PCHs in memory.
std::vector ChainedIncludes;
+ /// \brief When true, we are generating a pre-compiled header.
+ bool GeneratePCHMode;
+
/// \brief When true, disables most of the normal validation performed on
/// precompiled headers.
bool DisablePCHValidation;
@@ -141,6 +144,7 @@
public:
PreprocessorOptions() : UsePredefines(true), DetailedRecord(false),
+ GeneratePCHMode(false),
DisablePCHValidation(false),
AllowPCHWithCompilerErrors(false),
DumpDeserializedPCHDecls(false),
Index: test/PCH/pragma-once.h
===
--- /dev/null
+++ test/PCH/pragma-once.h
@@ -0,0 +1,5 @@
+#pragma once
+
+/* For use with the pragma-once.c test */
+
+int x = 3;
Index: test/PCH/pragma-once.c
===
--- /dev/null
+++ test/PCH/pragma-once.c
@@ -0,0 +1,13 @@
+// Test this without pch.
+// RUN: %clang_cc1 -include %S/pragma-once.h -fsyntax-only -verify %s
+
+// Test with pch.
+// RUN: %clang_cc1 -emit-pch -o %t %S/pragma-once.h
+// RUN: %clang_cc1 -include-pch %t -fsyntax-only -verify %s
+
+// expected-no-diagnostics
+
+// Including "pragma-once.h" twice, to verify the 'once' aspect is honored.
+#include "pragma-once.h"
+#include "pragma-once.h"
+int foo(void) { return 0; }
Index: lib/Lex/Pragma.cpp
===
--- lib/Lex/Pragma.cpp
+++ lib/Lex/Pragma.cpp
@@ -19,6 +19,7 @@
#include "clang/Lex/LexDiagnostic.h"
#include "clang/Lex/LiteralSupport.h"
#include "clang/Lex/MacroInfo.h"
+#include "clang/Lex/PreprocessorOptions.h"
#include "clang/Lex/Preprocessor.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/StringSwitch.h"
@@ -354,7 +355,9 @@
///
