[Sema] Do not load macros from preamble when LoadExternal is false.

[ilya-biryukov]

Maybe add a test?
There is test/Index/complete-pch-skip.cpp that checks similar things for AST completions, adding macros there should be trivial.

[ioeric]

• added lit test

[ilya-biryukov]

LGTM

[nik]

I've bisected https://bugs.llvm.org/show_bug.cgi?id=42649 to this change. Reverting this change fixes the issue.

Please look into it.

include/clang/Sema/CodeCompleteOptions.h

public:
   /// `Sema::CodeCompleteQualifiedId`).
   /// FIXME: consider supporting more completion cases with this option.
   unsigned IncludeNamespaceLevelDecls : 1;
 
   /// Show brief documentation comments in code completion results.
   unsigned IncludeBriefComments : 1;
 
   /// Hint whether to load data from the external AST to provide full results.
-  /// If false, namespace-level declarations from the preamble may be omitted.
+  /// If false, namespace-level declarations and macros from the preamble may be
+  /// omitted.
   unsigned LoadExternal : 1;
 
   /// Include results after corrections (small fix-its), e.g. change '.' to '->'
   /// on member access, etc.
   unsigned IncludeFixIts : 1;
 
   CodeCompleteOptions()
       : IncludeMacros(0), IncludeCodePatterns(0), IncludeGlobals(1),
         IncludeNamespaceLevelDecls(1), IncludeBriefComments(0),
         LoadExternal(1), IncludeFixIts(0) {}
 };
 
 } // namespace clang
 
 #endif
 

lib/Sema/SemaCodeComplete.cpp

default:
         }
       }
   }
 
   return CXCursor_UnexposedDecl;
 }
 
 static void AddMacroResults(Preprocessor &PP, ResultBuilder &Results,
-                            bool IncludeUndefined,
+                            bool LoadExternal, bool IncludeUndefined,
                             bool TargetTypeIsPointer = false) {
   typedef CodeCompletionResult Result;
 
   Results.EnterNewScope();
 
-  for (Preprocessor::macro_iterator M = PP.macro_begin(),
-                                 MEnd = PP.macro_end();
+  for (Preprocessor::macro_iterator M = PP.macro_begin(LoadExternal),
+                                    MEnd = PP.macro_end(LoadExternal);
        M != MEnd; ++M) {
     auto MD = PP.getMacroDefinition(M->first);
     if (IncludeUndefined || MD) {
       MacroInfo *MI = MD.getMacroInfo();
       if (MI && MI->isUsedForHeaderGuard())
         continue;
 
       Results.AddResult(
           Result(M->first, MI,
                  getMacroUsagePriority(M->first->getName(), PP.getLangOpts(),
                                        TargetTypeIsPointer)));
     }
   }
 
   Results.ExitScope();
-
 }
 
 static void AddPrettyFunctionResults(const LangOptions &LangOpts,
                                      ResultBuilder &Results) {
   typedef CodeCompletionResult Result;
 
   Results.EnterNewScope();
 
void Sema::CodeCompleteOrdinaryName(Scope *S,
   case PCC_ForInit:
   case PCC_Condition:
   case PCC_Type:
   case PCC_LocalDeclarationSpecifiers:
     break;
   }
 
   if (CodeCompleter->includeMacros())
-    AddMacroResults(PP, Results, false);
+    AddMacroResults(PP, Results, CodeCompleter->loadExternal(), false);
 
   HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
                             Results.data(),Results.size());
 }
 
 static void AddClassMessageCompletions(Sema &SemaRef, Scope *S,
                                        ParsedType Receiver,
                                        ArrayRef<IdentifierInfo *> SelIdents,
PreferredTypeIsPointer = Data.PreferredType->isAnyPointerType()
       || Data.PreferredType->isBlockPointerType();
 
   if (S->getFnParent() &&
       !Data.ObjCCollection &&
       !Data.IntegralConstantExpression)
     AddPrettyFunctionResults(getLangOpts(), Results);
 
   if (CodeCompleter->includeMacros())
-    AddMacroResults(PP, Results, false, PreferredTypeIsPointer);
+    AddMacroResults(PP, Results, CodeCompleter->loadExternal(), false,
+                    PreferredTypeIsPointer);
   HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
                             Results.data(), Results.size());
 }
 
 void Sema::CodeCompleteExpression(Scope *S, QualType PreferredType) {
   return CodeCompleteExpression(S, CodeCompleteExpressionData(PreferredType));
 }
 
if (EnumeratorsSeen.count(E))
       continue;
 
     CodeCompletionResult R(E, CCP_EnumInCase, Qualifier);
     Results.AddResult(R, CurContext, nullptr, false);
   }
   Results.ExitScope();
 
   if (CodeCompleter->includeMacros()) {
-    AddMacroResults(PP, Results, false);
+    AddMacroResults(PP, Results, CodeCompleter->loadExternal(), false);
   }
   HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
                             Results.data(), Results.size());
 }
 
 static bool anyNullArguments(ArrayRef<Expr *> Args) {
   if (Args.size() && !Args.data())
     return true;
void Sema::CodeCompleteAfterIf(Scope *S) {
   Results.AddResult(Builder.TakeString());
 
   Results.ExitScope();
 
   if (S->getFnParent())
     AddPrettyFunctionResults(getLangOpts(), Results);
 
   if (CodeCompleter->includeMacros())
-    AddMacroResults(PP, Results, false);
+    AddMacroResults(PP, Results, CodeCompleter->loadExternal(), false);
 
   HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
                             Results.data(),Results.size());
 }
 
 void Sema::CodeCompleteAssignmentRHS(Scope *S, Expr *LHS) {
   if (LHS)
     CodeCompleteExpression(S, static_cast<Expr *>(LHS)->getType());
void Sema::CodeCompleteObjCPassingType(Scope *S, ObjCDeclSpec &DS,
   // Add the various type names
   Results.setFilter(&ResultBuilder::IsOrdinaryNonValueName);
   CodeCompletionDeclConsumer Consumer(Results, CurContext);
   LookupVisibleDecls(S, LookupOrdinaryName, Consumer,
                      CodeCompleter->includeGlobals(),
                      CodeCompleter->loadExternal());
 
   if (CodeCompleter->includeMacros())
-    AddMacroResults(PP, Results, false);
+    AddMacroResults(PP, Results, CodeCompleter->loadExternal(), false);
 
   HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
                             Results.data(), Results.size());
 }
 
 /// When we have an expression with type "id", we may assume
 /// that it has some more-specific class type based on knowledge of
 /// common uses of Objective-C. This routine returns that class type,
if (ObjCInterfaceDecl *Iface = Method->getClassInterface())
       }
 
   if (getLangOpts().CPlusPlus11)
     addThisCompletion(*this, Results);
 
   Results.ExitScope();
 
   if (CodeCompleter->includeMacros())
-    AddMacroResults(PP, Results, false);
+    AddMacroResults(PP, Results, CodeCompleter->loadExternal(), false);
   HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
                             Results.data(), Results.size());
-
 }
 
 void Sema::CodeCompleteObjCSuperMessage(Scope *S, SourceLocation SuperLoc,
                                         ArrayRef<IdentifierInfo *> SelIdents,
                                         bool AtArgumentExpression) {
   ObjCInterfaceDecl *CDecl = nullptr;
   if (ObjCMethodDecl *CurMethod = getCurMethodDecl()) {
     // Figure out which interface we're in.

 }
 
 void Sema::CodeCompletePreprocessorExpression() {
   ResultBuilder Results(*this, CodeCompleter->getAllocator(),
                         CodeCompleter->getCodeCompletionTUInfo(),
                         CodeCompletionContext::CCC_PreprocessorExpression);
 
   if (!CodeCompleter || CodeCompleter->includeMacros())
-    AddMacroResults(PP, Results, true);
+    AddMacroResults(PP, Results,
+                    CodeCompleter ? CodeCompleter->loadExternal() : false,
+                    true);
 
     // defined (<macro>)
   Results.EnterNewScope();
   CodeCompletionBuilder Builder(Results.getAllocator(),
                                 Results.getCodeCompletionTUInfo());
   Builder.AddTypedTextChunk("defined");
   Builder.AddChunk(CodeCompletionString::CK_HorizontalSpace);
   Builder.AddChunk(CodeCompletionString::CK_LeftParen);
if (!CodeCompleter || CodeCompleter->includeGlobals()) {
     CodeCompletionDeclConsumer Consumer(Builder,
                                         Context.getTranslationUnitDecl());
     LookupVisibleDecls(Context.getTranslationUnitDecl(), LookupAnyName,
                        Consumer,
                        !CodeCompleter || CodeCompleter->loadExternal());
   }
 
   if (!CodeCompleter || CodeCompleter->includeMacros())
-    AddMacroResults(PP, Builder, true);
+    AddMacroResults(PP, Builder,
+                    CodeCompleter ? CodeCompleter->loadExternal() : false,
+                    true);
 
   Results.clear();
   Results.insert(Results.end(),
                  Builder.data(), Builder.data() + Builder.size());
 }

test/Index/complete-pch-skip.cpp

 namespace ns {
 int bar;
 }
 
 int main() { return ns:: }
 int main2() { return ns::foo(). }
+int main3() { PREAMBLE_ }
 
-// RUN: echo "namespace ns { struct foo { int baz }; }" > %t.h
+// RUN: printf "namespace ns { struct foo { int baz }; }\n#define PREAMBLE_MAC" > %t.h
 // RUN: c-index-test -write-pch %t.h.pch -x c++-header %t.h
 //
 // RUN: c-index-test -code-completion-at=%s:5:26 -include %t.h %s | FileCheck -check-prefix=WITH-PCH %s
 // WITH-PCH: {TypedText bar}
 // WITH-PCH: {TypedText foo}
 
+// RUN: c-index-test -code-completion-at=%s:7:24 -include %t.h %s | FileCheck -check-prefix=WITH-PCH-MACRO %s
+// WITH-PCH-MACRO: {TypedText PREAMBLE_MAC}
+
 // RUN: env CINDEXTEST_COMPLETION_SKIP_PREAMBLE=1 c-index-test -code-completion-at=%s:5:26 -include %t.h %s | FileCheck -check-prefix=SKIP-PCH %s
 // SKIP-PCH-NOT: foo
 // SKIP-PCH: {TypedText bar}
 // SKIP-PCH-NOT: foo
 
+// RUN: env CINDEXTEST_COMPLETION_SKIP_PREAMBLE=1 c-index-test -code-completion-at=%s:7:24 -include %t.h %s | FileCheck -check-prefix=SKIP-PCH-MACRO %s
+// SKIP-PCH-MACRO-NOT: {TypedText PREAMBLE_MAC}
+
 // Verify that with *no* preamble (no -include flag) we still get local results.
 // SkipPreamble used to break this, by making lookup *too* lazy.
 // RUN: env CINDEXTEST_COMPLETION_SKIP_PREAMBLE=1 c-index-test -code-completion-at=%s:5:26 %s | FileCheck -check-prefix=NO-PCH %s
 // NO-PCH-NOT: foo
 // NO-PCH: {TypedText bar}
 // NO-PCH-NOT: foo
 
 // Verify that we still get member results from the preamble.
 // RUN: env CINDEXTEST_COMPLETION_SKIP_PREAMBLE=1 c-index-test -code-completion-at=%s:6:32 -include %t.h %s | FileCheck -check-prefix=MEMBER %s
 // MEMBER: {TypedText baz}