Support '#pragma once' in headers when using PCH

[wristow]

Summary

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.

[wristow]

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).

[rnk]

I think threading this through PP options is reasonable.

test/PCH/pragma-once.h

1 ↗

(On Diff #55841)

This should be in test/PCH/Inputs

[wristow]

Moved "test/PCH/pragma-once.h" to the "test/PCH/Inputs" directory (and changed "pragma-once.c" appropriately).

[wristow]

Ping

[rsmith]

Rather than threading through a new flag, can you test the existing TUKind field?

[wristow]

In D19815#428705, @rsmith wrote:

Rather than threading through a new flag, can you test the existing TUKind field?

Yes, that does seem to do the trick. I'll do some testing on it, but with that approach, it should be a simple one-line change. Thanks.

[wristow]

Simplified the implementation, by using the existing TUKind field.

[wristow]

ping

[elsteveogrande]

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 patches cleanly against trunk (I see that this has been around a while) and test still pass, think this can be committed soon-ish?

Thanks!

[wristow]

@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!

cfe/trunk/lib/Lex/Pragma.cpp

void Preprocessor::HandleMicrosoft__pragma(Token &Tok) {
 
   // Finally, return whatever came after the pragma directive.
   return Lex(Tok);
 }
 
 /// HandlePragmaOnce - Handle \#pragma once.  OnceTok is the 'once'.
 ///
 void Preprocessor::HandlePragmaOnce(Token &OnceTok) {
-  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;
   }
 
   // Get the current file lexer we're looking at.  Ignore _Pragma 'files' etc.
   // Mark the file as a once-only file now.
   HeaderInfo.MarkFileIncludeOnce(getCurrentFileLexer()->getFileEntry());
 }

cfe/trunk/test/PCH/Inputs/pragma-once.h

+#pragma once
+
+/* For use with the pragma-once.c test */
+
+int x = 3;

cfe/trunk/test/PCH/pragma-once.c

+// 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; }