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

2019-07-17 Thread Nikolai Kosjar via Phabricator via cfe-commits 
nik added a comment.
Herald added a subscriber: arphaman.
Herald added a project: clang.

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.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D52079/new/

https://reviews.llvm.org/D52079



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2018-09-19 Thread Eric Liu via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rC342528: [Sema] Do not load macros from preamble when 
LoadExternal is false. (authored by ioeric, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D52079?vs=165969=166077#toc

Repository:
  rC Clang

https://reviews.llvm.org/D52079

Files:
  include/clang/Sema/CodeCompleteOptions.h
  lib/Sema/SemaCodeComplete.cpp
  test/Index/complete-pch-skip.cpp

Index: include/clang/Sema/CodeCompleteOptions.h
===
--- include/clang/Sema/CodeCompleteOptions.h
+++ include/clang/Sema/CodeCompleteOptions.h
@@ -36,7 +36,8 @@
   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 '->'
Index: test/Index/complete-pch-skip.cpp
===
--- test/Index/complete-pch-skip.cpp
+++ test/Index/complete-pch-skip.cpp
@@ -4,19 +4,26 @@
 
 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
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -3304,14 +3304,14 @@
 }
 
 static void AddMacroResults(Preprocessor , ResultBuilder ,
-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) {
@@ -3327,7 +3327,6 @@
   }
 
   Results.ExitScope();
-
 }
 
 static void AddPrettyFunctionResults(const LangOptions ,
@@ -3611,7 +3610,7 @@
   }
 
   if (CodeCompleter->includeMacros())
-AddMacroResults(PP, Results, false);
+AddMacroResults(PP, Results, CodeCompleter->loadExternal(), false);
 
   HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
 Results.data(),Results.size());
@@ -3749,7 +3748,8 @@
 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());
 }
@@ -4372,7 +4372,7 @@
   Results.ExitScope();
 
   if (CodeCompleter->includeMacros()) {
-AddMacroResults(PP, Results, false);
+AddMacroResults(PP, Results, CodeCompleter->loadExternal(), false);
   }
   HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
 Results.data(), Results.size());
@@ -4688,7 +4688,7 @@
 AddPrettyFunctionResults(getLangOpts(), Results);
 
   if (CodeCompleter->includeMacros())
-AddMacroResults(PP, Results, false);
+AddMacroResults(PP, Results,

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

2018-09-19 Thread Ilya Biryukov via Phabricator via cfe-commits 
ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rC Clang

https://reviews.llvm.org/D52079



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2018-09-18 Thread Eric Liu via Phabricator via cfe-commits 
ioeric updated this revision to Diff 165969.
ioeric added a comment.

- added lit test


Repository:
  rC Clang

https://reviews.llvm.org/D52079

Files:
  include/clang/Sema/CodeCompleteOptions.h
  lib/Sema/SemaCodeComplete.cpp
  test/Index/complete-pch-skip.cpp

Index: test/Index/complete-pch-skip.cpp
===
--- test/Index/complete-pch-skip.cpp
+++ test/Index/complete-pch-skip.cpp
@@ -4,19 +4,26 @@
 
 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
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -3304,14 +3304,14 @@
 }
 
 static void AddMacroResults(Preprocessor , ResultBuilder ,
-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) {
@@ -3327,7 +3327,6 @@
   }
 
   Results.ExitScope();
-
 }
 
 static void AddPrettyFunctionResults(const LangOptions ,
@@ -3611,7 +3610,7 @@
   }
 
   if (CodeCompleter->includeMacros())
-AddMacroResults(PP, Results, false);
+AddMacroResults(PP, Results, CodeCompleter->loadExternal(), false);
 
   HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
 Results.data(),Results.size());
@@ -3749,7 +3748,8 @@
 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());
 }
@@ -4372,7 +4372,7 @@
   Results.ExitScope();
 
   if (CodeCompleter->includeMacros()) {
-AddMacroResults(PP, Results, false);
+AddMacroResults(PP, Results, CodeCompleter->loadExternal(), false);
   }
   HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
 Results.data(), Results.size());
@@ -4688,7 +4688,7 @@
 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());
@@ -5722,7 +5722,7 @@
  CodeCompleter->loadExternal());
 
   if (CodeCompleter->includeMacros())
-AddMacroResults(PP, Results, false);
+AddMacroResults(PP, Results, CodeCompleter->loadExternal(), false);
 
   HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
 Results.data(), Results.size());
@@ -5951,10 +5951,9 @@
   Results.ExitScope();
 
   if (CodeCompleter->includeMacros())
-AddMacroResults(PP, Results, false);
+AddMacroResults(PP, Results, CodeCompleter->loadExternal(), false);
   HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),

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

2018-09-18 Thread Ilya Biryukov via Phabricator via cfe-commits 
ilya-biryukov added a comment.

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.


Repository:
  rC Clang

https://reviews.llvm.org/D52079



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2018-09-14 Thread Eric Liu via Phabricator via cfe-commits 
ioeric created this revision.
ioeric added a reviewer: ilya-biryukov.
Herald added a subscriber: cfe-commits.

Repository:
  rC Clang

https://reviews.llvm.org/D52079

Files:
  include/clang/Sema/CodeCompleteOptions.h
  lib/Sema/SemaCodeComplete.cpp

Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -3302,14 +3302,14 @@
 }
 
 static void AddMacroResults(Preprocessor , ResultBuilder ,
-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) {
@@ -3325,7 +3325,6 @@
   }
 
   Results.ExitScope();
-
 }
 
 static void AddPrettyFunctionResults(const LangOptions ,
@@ -3609,7 +3608,7 @@
   }
 
   if (CodeCompleter->includeMacros())
-AddMacroResults(PP, Results, false);
+AddMacroResults(PP, Results, CodeCompleter->loadExternal(), false);
 
   HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
 Results.data(),Results.size());
@@ -3747,7 +3746,8 @@
 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());
 }
@@ -4370,7 +4370,7 @@
   Results.ExitScope();
 
   if (CodeCompleter->includeMacros()) {
-AddMacroResults(PP, Results, false);
+AddMacroResults(PP, Results, CodeCompleter->loadExternal(), false);
   }
   HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
 Results.data(), Results.size());
@@ -4686,7 +4686,7 @@
 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());
@@ -5720,7 +5720,7 @@
  CodeCompleter->loadExternal());
 
   if (CodeCompleter->includeMacros())
-AddMacroResults(PP, Results, false);
+AddMacroResults(PP, Results, CodeCompleter->loadExternal(), false);
 
   HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
 Results.data(), Results.size());
@@ -5949,10 +5949,9 @@
   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,
@@ -7965,7 +7964,9 @@
 CodeCompletionContext::CCC_PreprocessorExpression);
 
   if (!CodeCompleter || CodeCompleter->includeMacros())
-AddMacroResults(PP, Results, true);
+AddMacroResults(PP, Results,
+CodeCompleter ? CodeCompleter->loadExternal() : false,
+true);
 
 // defined ()
   Results.EnterNewScope();
@@ -8030,7 +8031,9 @@
   }
 
   if (!CodeCompleter || CodeCompleter->includeMacros())
-AddMacroResults(PP, Builder, true);
+AddMacroResults(PP, Builder,
+CodeCompleter ? CodeCompleter->loadExternal() : false,
+true);
 
   Results.clear();
   Results.insert(Results.end(),
Index: include/clang/Sema/CodeCompleteOptions.h
===
--- include/clang/Sema/CodeCompleteOptions.h
+++ include/clang/Sema/CodeCompleteOptions.h
@@ -36,7 +36,8 @@
   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 '->'
___
cfe-commits mailing list