[clang] Keep multiple-include optimization for null directives

[IncludeGuardian]

Summary

The multiple-include optimization allows Clang to avoid opening a
files when they contain #pragma once or a proper include guard.

Both GCC and Microsoft Visual Studio allow null directives outside of
the #ifndef/#endif pair without disabling this multiple-include
optimization. GCC documents this behavior here
https://gcc.gnu.org/onlinedocs/cppinternals/Guard-Macros.html.

There must be no directives outside the controlling directive pair,
but the null directive (a line containing nothing other than a
single '#' and possibly whitespace) is permitted.

However, Clang disables the multiple-include optimization when
encountering the null directive.

In particular, this slows down preprocessing of most projects that
depend on boost as many boost libraries depend on the boost
preprocessor library, which contains null directives outside the
include guard on every header file.

[IncludeGuardian]

Differences between the behaviors of compilers can be found here https://github.com/IncludeGuardian/multiple-inclusion-optimization-tests

You can see the number of boost preprocessor files failing the include guard optimization when running IncludeGuardian over Boost Graph https://gist.github.com/IncludeGuardian/0837719e0d1162d5b50e4c8fed4d3c0d#file-boost_graph-yaml-L345-L749

An example boost preprocessor header can be found here https://github.com/boostorg/preprocessor/blob/667e87b3392db338a919cbe0213979713aca52e3/include/boost/preprocessor.hpp. Note the consistent use of null directives outside of the guard.

[aaron.ballman]

clang/lib/Lex/PPDirectives.cpp

1183–1185

NFC but matches our style guide.

One thing to consider: making the call a setter instead of a reset would simplify this logic into: CurPPLexer->SetReadToken(ReadAnyTokensBeforeDirective); without any branching required. WDYT?

[IncludeGuardian]

clang/lib/Lex/PPDirectives.cpp

1183–1185

Sounds good. Removing the condition here should look neater.

[IncludeGuardian]

Replace ResetReadToken with SetReadToken to avoid a conditional

[aaron.ballman]

LGTM, though please add a release note to clang/docs/ReleaseNotes.rst for the fix. Do you need me to commit on your behalf? If so, please let me know what name and email address you'd like used for patch attribution.

[IncludeGuardian]

Update Clang release notes

[IncludeGuardian]

@aaron.ballman If you could commit for me that would be great. Please use "Elliot Goodrich" for the name and "[e-mail]" for the email - thank you!

[IncludeGuardian]

Rebase on upstream to resolve conflicts in release notes

clang/include/clang/Lex/MultipleIncludeOpt.h

public:
   }
 
   // If a token is read, remember that we have seen a side-effect in this file.
   void ReadToken() {
     ReadAnyTokens = true;
     ImmediatelyAfterTopLevelIfndef = false;
   }
 
+  /// SetReadToken - Set whether the value of 'ReadAnyTokens'.  Called to
+  /// override when encountering tokens outside of the include guard that have
+  /// no effect if the file in question is is included multiple times (e.g. the
+  /// null directive).
+  void SetReadToken(bool Value) { ReadAnyTokens = Value; }
+
   /// ExpandedMacro - When a macro is expanded with this lexer as the current
   /// buffer, this method is called to disable the MIOpt if needed.
   void ExpandedMacro() { DidMacroExpansion = true; }
 
   /// Called when entering a top-level \#ifndef directive (or the
   /// "\#if !defined" equivalent) without any preceding tokens.
   ///
   /// Note, we don't care about the input value of 'ReadAnyTokens'.  The caller

clang/lib/Lex/PPDirectives.cpp

void Preprocessor::HandleDirective(Token &Result) {
   // and reset to previous state when returning from this function.
   ResetMacroExpansionHelper helper(this);
 
   if (SkippingUntilPCHThroughHeader || SkippingUntilPragmaHdrStop)
     return HandleSkippedDirectiveWhileUsingPCH(Result, SavedHash.getLocation());
 
   switch (Result.getKind()) {
   case tok::eod:
+    // Ignore the null directive with regards to the multiple-include
+    // optimization, i.e. allow the null directive to appear outside of the
+    // include guard and still enable the multiple-include optimization.
+    CurPPLexer->MIOpt.SetReadToken(ReadAnyTokensBeforeDirective);
     return;   // null directive.
   case tok::code_completion:
     setCodeCompletionReached();
     if (CodeComplete)
       CodeComplete->CodeCompleteDirective(
                                     CurPPLexer->getConditionalStackDepth() > 0);
     return;
   case tok::numeric_constant:  // # 7  GNU line marker directive.
     // In a .S file "# 4" may be a comment so don't treat it as a preprocessor
     // directive. However do permit it in the predefines file, as we use line

clang/test/Preprocessor/multiple-inclusion-opt.h

+# // null directive and comments before include guard
+
+#ifndef MULTIPLE_INCLUSION_OPT
+
+int foo();
+
+// The position of the define should not matter
+#define MULTIPLE_INCLUSION_OPT
+
+int bar();
+
+#endif
+
+#
+#
+/* Two null directives
+   and a multiline comment
+   after the #endif */

clang/test/Preprocessor/multiple-inclusion-opt.cpp

+// RUN: %clang_cc1 -E -P -H %s 2>&1 | grep "multiple-inclusion-opt.h" | count 1
+
+#include "multiple-inclusion-opt.h"
+#include "multiple-inclusion-opt.h"
+#include "multiple-inclusion-opt.h"
+#include "multiple-inclusion-opt.h"
+#include "multiple-inclusion-opt.h"