[PATCH] D63508: make -frewrite-includes handle __has_include wrapped in a macro

[Luboš Luňák via Phabricator via cfe-commits]

llunak created this revision.
llunak added reviewers: vsapsai, bkramer.
llunak added a project: clang.
Herald added subscribers: cfe-commits, dexonsmith.

Qt5 has a wrapper macro that makes __has_include simpler to use:
 #ifdef __has_include

1. define QT_HAS_INCLUDE(x) __has_include(x) #else
2. define QT_HAS_INCLUDE(x) 0 #endif #if QT_HAS_INCLUDE()
3. include  #endif

The code handling this so far ignores macros entirely. This patch
handles this specific case in a crude way by checking if an identifier
in #if is a macro whose first token is the checked for macro.


Repository:
  rC Clang

https://reviews.llvm.org/D63508

Files:
  clang/lib/Frontend/Rewrite/InclusionRewriter.cpp
  clang/test/Frontend/rewrite-includes-has-include-macro.c


Index: clang/test/Frontend/rewrite-includes-has-include-macro.c
===
--- /dev/null
+++ clang/test/Frontend/rewrite-includes-has-include-macro.c
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -verify -E -frewrite-includes %s -o - | FileCheck 
-strict-whitespace %s
+// expected-no-diagnostics
+
+#define MACRO_HAS_INCLUDE(x) __has_include(x)
+#if MACRO_HAS_INCLUDE()
+#endif
+
+// CHECK: #define MACRO_HAS_INCLUDE(x) __has_include(x)
+// CHECK-NEXT: #if (0)/*MACRO_HAS_INCLUDE()*/
+// CHECK-NEXT: #endif
Index: clang/lib/Frontend/Rewrite/InclusionRewriter.cpp
===
--- clang/lib/Frontend/Rewrite/InclusionRewriter.cpp
+++ clang/lib/Frontend/Rewrite/InclusionRewriter.cpp
@@ -527,16 +527,28 @@
 PP.LookUpIdentifierInfo(RawToken);
 
   if (RawToken.is(tok::identifier)) {
+// Try to handle macros in the form of
+// '#define QT_HAS_INCLUDE(x) __has_include(x)'.
+const Token *ExpandedToken = &RawToken;
+if (const MacroInfo *macroInfo =
+PP.getMacroInfo(RawToken.getIdentifierInfo())) {
+  if (macroInfo->getNumTokens() > 0 &&
+  macroInfo->getReplacementToken(0).is(tok::identifier)) {
+ExpandedToken = ¯oInfo->getReplacementToken(0);
+  }
+}
+
 bool HasFile;
 SourceLocation Loc = RawToken.getLocation();
 
 // Rewrite __has_include(x)
-if (RawToken.getIdentifierInfo()->isStr("__has_include")) {
+if (ExpandedToken->getIdentifierInfo()->isStr(
+"__has_include")) {
   if (!HandleHasInclude(FileId, RawLex, nullptr, RawToken,
 HasFile))
 continue;
   // Rewrite __has_include_next(x)
-} else if (RawToken.getIdentifierInfo()->isStr(
+} else if (ExpandedToken->getIdentifierInfo()->isStr(
"__has_include_next")) {
   if (DirLookup)
 ++DirLookup;


Index: clang/test/Frontend/rewrite-includes-has-include-macro.c
===
--- /dev/null
+++ clang/test/Frontend/rewrite-includes-has-include-macro.c
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -verify -E -frewrite-includes %s -o - | FileCheck -strict-whitespace %s
+// expected-no-diagnostics
+
+#define MACRO_HAS_INCLUDE(x) __has_include(x)
+#if MACRO_HAS_INCLUDE()
+#endif
+
+// CHECK: #define MACRO_HAS_INCLUDE(x) __has_include(x)
+// CHECK-NEXT: #if (0)/*MACRO_HAS_INCLUDE()*/
+// CHECK-NEXT: #endif
Index: clang/lib/Frontend/Rewrite/InclusionRewriter.cpp
===
--- clang/lib/Frontend/Rewrite/InclusionRewriter.cpp
+++ clang/lib/Frontend/Rewrite/InclusionRewriter.cpp
@@ -527,16 +527,28 @@
 PP.LookUpIdentifierInfo(RawToken);
 
   if (RawToken.is(tok::identifier)) {
+// Try to handle macros in the form of
+// '#define QT_HAS_INCLUDE(x) __has_include(x)'.
+const Token *ExpandedToken = &RawToken;
+if (const MacroInfo *macroInfo =
+PP.getMacroInfo(RawToken.getIdentifierInfo())) {
+  if (macroInfo->getNumTokens() > 0 &&
+  macroInfo->getReplacementToken(0).is(tok::identifier)) {
+ExpandedToken = ¯oInfo->getReplacementToken(0);
+  }
+}
+
 bool HasFile;
 SourceLocation Loc = RawToken.getLocation();
 
 // Rewrite __has_include(x)
-if (RawToken.getIdentifierInfo()->isStr("__has_include")) {
+if (ExpandedToken->getIdentifierInfo()->isStr(
+"__has_include")) {
   if (!HandleHasInclude(FileId, RawLex, nullptr, RawToken,
 HasFile))
 continue;
  

[PATCH] D63508: make -frewrite-includes handle __has_include wrapped in a macro

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

The code uses a raw lexer, so it doesn't expand macros. Thus the piece of code 
quoted will not get the "#if QT_HAS_INCLUDE()" part rewritten. My 
specific use case is the Icecream distributed build tool which does remote 
builds in a chroot and so this #if will be false and the expanded contents of 
 will be ignored, leading to build errors 
(https://github.com/icecc/icecream/issues/471).

I can change the code to use 'while' instead of 'if' to handle nested macros, 
but I think such an approach should go only so far. My patch is simple and 
handles a realistic scenario, but if more complex macro handling is needed 
(which is a question), then IMO the code should be changed to do macro 
expansion properly. It was a long time ago when I wrote the code, but seeing 
that usage of Preprocessor::SetMacroExpansionOnlyInDirectives() I think such an 
expansion could be done similarly how handling macro expansion is handled in 
other directives - which IIRC is getting that information from PPCallbacks.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63508



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

[PATCH] D63508: make -frewrite-includes handle __has_include wrapped in a macro

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Icecream's usage of -frewrite-includes is not special, the purpose is always to 
merge everything necessary for the compilation into one source file that can be 
later compiled on its own as if the original file was compiled. The #include 
expansion done during -frewrite-includes will evaluate all the #if conditions 
etc. the same way they will be evaluated again during the actual compilation.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63508



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

[PATCH] D63508: make -frewrite-includes handle __has_include wrapped in a macro

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

In D63508#1550668 , @rsmith wrote:

> Perhaps we should rewrite all `#if`-like directives to `#if 0` or `#if 1`?


I think that would work too and it'd be in fact a reliable simple solution.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63508



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

[PATCH] D63508: make -frewrite-includes also rewrite conditions in #if/#elif

[Luboš Luňák via Phabricator via cfe-commits]

llunak added inline comments.



Comment at: clang/test/Frontend/rewrite-includes-conditions.c:17
+line4
+#elif value2 < value2
+line5

rsmith wrote:
> Did you mean for this to be `value1 < value2` rather than `value2 < value2`?
Yes, not that it'd matter in practice. I'll fix that in the next iteration of 
the patch.




Comment at: clang/test/Frontend/rewrite-includes-conditions.c:54-67
+// CHECK: #if 0 /* disabled by -frewrite-includes */
+// CHECK-NEXT: #if value1 == value2
+// CHECK-NEXT: #endif
+// CHECK-NEXT: #endif /* disabled by -frewrite-includes */
+// CHECK-NEXT: #if 0 /* evaluated by -frewrite-includes */
+// CHECK-NEXT: # 14 "{{.*}}rewrite-includes-conditions.c"
+

rsmith wrote:
> I find this output pretty hard to read -- it's hard to tell what's an 
> original `#if` / `#endif` that's been effectively commented out, and what's 
> an added `#if` / `#endif` that's doing the commenting.
> 
> What do you think about modifying the original line so it's no longer 
> recognized as a preprocessing directive? For instance, instead of:
> 
> ```
> #if 0 /* disabled by -frewrite-includes */
> #if value1 == value2
> #endif
> #endif /* disabled by -frewrite-includes */
> #if 0 /* evaluated by -frewrite-includes */
> # 14 "{{.*}}rewrite-includes-conditions.c"
> line3
> #if 0 /* disabled by -frewrite-includes */
> #if 0
> #elif value1 > value2
> #endif
> #endif /* disabled by -frewrite-includes */
> #elif 0 /* evaluated by -frewrite-includes */
> # 16 "{{.*}}rewrite-includes-conditions.c"
> line4
> #if 0 /* disabled by -frewrite-includes */
> #if 0
> #elif value1 < value2
> #endif
> #endif /* disabled by -frewrite-includes */
> #elif 1 /* evaluated by -frewrite-includes */
> # 18 "{{.*}}rewrite-includes-conditions.c"
> line5
> [...]
> ```
> 
> you might produce
> 
> ```
> #if 0 /* rewritten by -frewrite-includes */
> !#if value1 == value2
> #endif
> #if 0 /* evaluated by -frewrite-includes */
> # 14 "{{.*}}rewrite-includes-conditions.c"
> line3
> #if 0 /* rewritten by -frewrite-includes */
> !#elif value1 > value2
> #endif
> #elif 0 /* evaluated by -frewrite-includes */
> # 16 "{{.*}}rewrite-includes-conditions.c"
> line4
> #if 0 /* rewritten by -frewrite-includes */
> !#elif value1 < value2
> #endif
> #elif 1 /* evaluated by -frewrite-includes */
> # 18 "{{.*}}rewrite-includes-conditions.c"
> line5
> [...]
> ```
> 
> (or whatever transformation you like that prevents recognition of a 
> `#if`/`#elif` within the `#if 0` block).
> 
> Also, maybe we could move the quoted directives inside their respective `#if` 
> / `#elif` block, and only comment them out if the block was entered:
> 
> ```
> #if 0 /* evaluated by -frewrite-includes */
> was: #if value1 == value2
> # 14 "{{.*}}rewrite-includes-conditions.c"
> line3
> #elif 0 /* evaluated by -frewrite-includes */
> was: #elif value1 > value2
> # 16 "{{.*}}rewrite-includes-conditions.c"
> line4
> #elif 1 /* evaluated by -frewrite-includes */
> #if 0
> was: #elif value1 < value2
> #endif
> # 18 "{{.*}}rewrite-includes-conditions.c"
> line5
> [...]
> ```
I think that's a matter of taste/opinion. I find your proposal visually harder 
to read, because although it saves one line, it also is incorrect syntax, it 
visually breaks if-endif nesting and is inconsistent with how 
-frewrite-includes does other rewriting. Moreover I think this will be so 
rarely examined by a human that it doesn't really matter. So unless you insist 
I'd prefer to keep it as it is.

BTW, the other related review I linked above, is it enough to mention it here, 
or should I do something more about it? I'm not sure how to pick reviewers if 
there's not an obvious one.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63508



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

[PATCH] D65371: do not emit -Wunused-macros warnings in -frewrite-includes mode (PR15614)

[Luboš Luňák via Phabricator via cfe-commits]

llunak created this revision.
llunak added a reviewer: rsmith.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This is an updated patch from https://bugs.llvm.org/show_bug.cgi?id=15614. See 
there for testcase etc.


Repository:
  rC Clang

https://reviews.llvm.org/D65371

Files:
  clang/lib/Lex/PPDirectives.cpp


Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -2723,7 +2723,8 @@
   // If we need warning for not using the macro, add its location in the
   // warn-because-unused-macro set. If it gets used it will be removed from 
set.
   if (getSourceManager().isInMainFile(MI->getDefinitionLoc()) &&
-  !Diags->isIgnored(diag::pp_macro_not_used, MI->getDefinitionLoc())) {
+  !Diags->isIgnored(diag::pp_macro_not_used, MI->getDefinitionLoc()) &&
+  !MacroExpansionInDirectivesOverride) {
 MI->setIsWarnIfUnused(true);
 WarnUnusedMacroLocs.insert(MI->getDefinitionLoc());
   }


Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -2723,7 +2723,8 @@
   // If we need warning for not using the macro, add its location in the
   // warn-because-unused-macro set. If it gets used it will be removed from set.
   if (getSourceManager().isInMainFile(MI->getDefinitionLoc()) &&
-  !Diags->isIgnored(diag::pp_macro_not_used, MI->getDefinitionLoc())) {
+  !Diags->isIgnored(diag::pp_macro_not_used, MI->getDefinitionLoc()) &&
+  !MacroExpansionInDirectivesOverride) {
 MI->setIsWarnIfUnused(true);
 WarnUnusedMacroLocs.insert(MI->getDefinitionLoc());
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63508: make -frewrite-includes also rewrite conditions in #if/#elif

[Luboš Luňák via Phabricator via cfe-commits]

llunak updated this revision to Diff 206070.
llunak retitled this revision from "make -frewrite-includes handle 
__has_include wrapped in a  macro" to "make -frewrite-includes also rewrite 
conditions in #if/#elif".
llunak edited the summary of this revision.

Repository:
  rC Clang

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

https://reviews.llvm.org/D63508

Files:
  clang/lib/Frontend/Rewrite/InclusionRewriter.cpp
  clang/test/Frontend/rewrite-includes-conditions.c
  clang/test/Frontend/rewrite-includes.c
  clang/test/Modules/preprocess-module.cpp

Index: clang/test/Modules/preprocess-module.cpp
===
--- clang/test/Modules/preprocess-module.cpp
+++ clang/test/Modules/preprocess-module.cpp
@@ -51,7 +51,7 @@
 // RUN: %clang_cc1 -fmodules -fmodule-file=%t/file.rewrite.pcm %s -I%t -verify -fno-modules-error-recovery -DFILE_REWRITE -DINCLUDE -I%S/Inputs/preprocess
 //
 // Check that we can preprocess this user of the .pcm file.
-// RUN: %clang_cc1 -fmodules -fmodule-file=%t/file.pcm %s -I%t -E -frewrite-imports -o %t/preprocess-module.ii
+// RUN: %clang_cc1 -fmodules -fmodule-file=%t/file.pcm %s -I%t -E -frewrite-imports -DFILE_REWRITE_FULL -o %t/preprocess-module.ii
 // RUN: %clang_cc1 -fmodules %t/preprocess-module.ii -verify -fno-modules-error-recovery -DFILE_REWRITE_FULL
 //
 // Check that language / header search options are ignored when preprocessing from a .pcm file.
Index: clang/test/Frontend/rewrite-includes.c
===
--- clang/test/Frontend/rewrite-includes.c
+++ clang/test/Frontend/rewrite-includes.c
@@ -110,12 +110,17 @@
 // CHECK-NEXT: {{^}}#endif /* expanded by -frewrite-includes */{{$}}
 // CHECK-NEXT: {{^}}# 22 "{{.*}}rewrite-includes.c"{{$}}
 // CHECK-NEXT: {{^}}# 1 "{{.*[/\\]Inputs(/|)}}rewrite-includes8.h" 1{{$}}
-// CHECK-NEXT: {{^}}#if (0)/*__has_include_next()*/{{$}}
-// CHECK-NEXT: {{^}}#elif (0)/*__has_include()*/{{$}}
+// CHECK-NEXT: {{^}}/* #if __has_include_next() - evaluated by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}#if (0) /* evaluated by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}# 2 "{{.*[/\\]Inputs(/|)}}rewrite-includes8.h"{{$}}
+// CHECK-NEXT: {{^}}/* #elif __has_include() - evaluated by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}#elif (0) /* evaluated by -frewrite-includes */{{$}}
 // CHECK-NEXT: {{^}}# 3 "{{.*[/\\]Inputs(/|)}}rewrite-includes8.h"{{$}}
 // CHECK-NEXT: {{^}}#endif{{$}}
 // CHECK-NEXT: {{^}}# 4 "{{.*[/\\]Inputs(/|)}}rewrite-includes8.h"{{$}}
-// CHECK-NEXT: {{^}}#if !(1)/*__has_include("rewrite-includes8.h")*/{{$}}
+// CHECK-NEXT: {{^}}/* #if !__has_include("rewrite-includes8.h") - evaluated by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}#if (0) /* evaluated by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}# 5 "{{.*[/\\]Inputs(/|)}}rewrite-includes8.h"{{$}}
 // CHECK-NEXT: {{^}}#endif{{$}}
 // CHECK-NEXT: {{^}}# 6 "{{.*[/\\]Inputs(/|)}}rewrite-includes8.h"{{$}}
 // CHECK-NEXT: {{^}}# 23 "{{.*}}rewrite-includes.c" 2{{$}}
@@ -124,7 +129,9 @@
 // CHECK-NEXT: {{^}}#endif /* expanded by -frewrite-includes */{{$}}
 // CHECK-NEXT: {{^}}# 23 "{{.*}}rewrite-includes.c"{{$}}
 // CHECK-NEXT: {{^}}# 1 "{{.*[/\\]Inputs(/|)}}rewrite-includes9.h" 1{{$}}
-// CHECK-NEXT: {{^}}#if (1)/*__has_include_next()*/{{$}}
+// CHECK-NEXT: {{^}}/* #if __has_include_next() - evaluated by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}#if (1) /* evaluated by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}# 2 "{{.*[/\\]Inputs(/|)}}rewrite-includes9.h"{{$}}
 // CHECK-NEXT: {{^}}#if 0 /* expanded by -frewrite-includes */{{$}}
 // CHECK-NEXT: {{^}}#include_next {{$}}
 // CHECK-NEXT: {{^}}#endif /* expanded by -frewrite-includes */{{$}}
@@ -193,15 +200,19 @@
 // CHECKNL-NEXT: {{^}}#if 0 /* expanded by -frewrite-includes */{{$}}
 // CHECKNL-NEXT: {{^}}#include "rewrite-includes8.h"{{$}}
 // CHECKNL-NEXT: {{^}}#endif /* expanded by -frewrite-includes */{{$}}
-// CHECKNL-NEXT: {{^}}#if (0)/*__has_include_next()*/{{$}}
-// CHECKNL-NEXT: {{^}}#elif (0)/*__has_include()*/{{$}}
+// CHECKNL-NEXT: {{^}}/* #if __has_include_next() - evaluated by -frewrite-includes */{{$}}
+// CHECKNL-NEXT: {{^}}#if (0) /* evaluated by -frewrite-includes */{{$}}
+// CHECKNL-NEXT: {{^}}/* #elif __has_include() - evaluated by -frewrite-includes */{{$}}
+// CHECKNL-NEXT: {{^}}#elif (0) /* evaluated by -frewrite-includes */{{$}}
 // CHECKNL-NEXT: {{^}}#endif{{$}}
-// CHECKNL-NEXT: {{^}}#if !(1)/*__has_include("rewrite-includes8.h")*/{{$}}
+// CHECKNL-NEXT: {{^}}/* #if !__has_include("rewrite-includes8.h") - evaluated by -frewrite-includes */{{$}}
+// CHECKNL-NEXT: {{^}}#if (0) /* evaluated by -frewrite-includes */{{$}}
 // CHECKNL-NEXT: {{^}}#endif{{$}}
 // CHECKNL-NEXT: {{^}}#if 0 /* expanded by -frewrite-includes */{{$}}
 // CHECKNL-NEXT: {{^}}#include "rewrite-includes9.h"{{$}}
 // CHECKNL-NEXT: {{^}}#endif /* expanded by -f

[PATCH] D63508: make -frewrite-includes also rewrite conditions in #if/#elif

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Implemented in the current version of the patch.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63508



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

[PATCH] D63508: make -frewrite-includes also rewrite conditions in #if/#elif

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

In D63508#1558390 , @rsmith wrote:

> Patch generally looks good; just a minor concern about the output format.




> Also, we don't need parentheses around the constant 0 or 1.

That was consistent with what the current code does, but I don't mind changing 
that.

> Adding an extra line here is going to mess up presumed line numbering.

That's ok. The code already inserts extra lines and always fixes them up using 
line markers. And I think that's unavoidable due to reasons below.

> Perhaps instead we could prepend a #if 0 /* or #if 1 /* to the directive:

That's fragile. Do it with something like "#if value /*blah*/ " and you'll have 
a problem with comment nesting. That is why all the changes are inserted 
between the two extra #if 0 / #endif lines. IIRC I thought about this for a 
while when designing -frewrite-includes and this is the best I came up with.
But you made me realize that the previous patch didn't handle this properly, so 
the current one has been fixed to be consistent there.

> I don't think we really need the "evaluated by -frewrite-includes" part, but 
> I have no objection to including it.

It's strictly speaking not necessary, but it's there to make it easier to 
identify those extra #if lines added by -frewrite-includes.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63508



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

[PATCH] D63508: make -frewrite-includes also rewrite conditions in #if/#elif

[Luboš Luňák via Phabricator via cfe-commits]

llunak updated this revision to Diff 207220.
llunak added a comment.

Updated patch, removed ()'s around 1/0, fixed to use CommentOutDirective().


Repository:
  rC Clang

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

https://reviews.llvm.org/D63508

Files:
  clang/lib/Frontend/Rewrite/InclusionRewriter.cpp
  clang/test/Frontend/rewrite-includes-conditions.c
  clang/test/Frontend/rewrite-includes.c
  clang/test/Modules/preprocess-module.cpp

Index: clang/test/Modules/preprocess-module.cpp
===
--- clang/test/Modules/preprocess-module.cpp
+++ clang/test/Modules/preprocess-module.cpp
@@ -51,7 +51,7 @@
 // RUN: %clang_cc1 -fmodules -fmodule-file=%t/file.rewrite.pcm %s -I%t -verify -fno-modules-error-recovery -DFILE_REWRITE -DINCLUDE -I%S/Inputs/preprocess
 //
 // Check that we can preprocess this user of the .pcm file.
-// RUN: %clang_cc1 -fmodules -fmodule-file=%t/file.pcm %s -I%t -E -frewrite-imports -o %t/preprocess-module.ii
+// RUN: %clang_cc1 -fmodules -fmodule-file=%t/file.pcm %s -I%t -E -frewrite-imports -DFILE_REWRITE_FULL -o %t/preprocess-module.ii
 // RUN: %clang_cc1 -fmodules %t/preprocess-module.ii -verify -fno-modules-error-recovery -DFILE_REWRITE_FULL
 //
 // Check that language / header search options are ignored when preprocessing from a .pcm file.
Index: clang/test/Frontend/rewrite-includes.c
===
--- clang/test/Frontend/rewrite-includes.c
+++ clang/test/Frontend/rewrite-includes.c
@@ -110,12 +110,23 @@
 // CHECK-NEXT: {{^}}#endif /* expanded by -frewrite-includes */{{$}}
 // CHECK-NEXT: {{^}}# 22 "{{.*}}rewrite-includes.c"{{$}}
 // CHECK-NEXT: {{^}}# 1 "{{.*[/\\]Inputs(/|)}}rewrite-includes8.h" 1{{$}}
-// CHECK-NEXT: {{^}}#if (0)/*__has_include_next()*/{{$}}
-// CHECK-NEXT: {{^}}#elif (0)/*__has_include()*/{{$}}
+// CHECK-NEXT: {{^}}#if 0 /* expanded by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}#/*if*/ __has_include_next(){{$}}
+// CHECK-NEXT: {{^}}#endif /* expanded by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}#if 0 /* evaluated by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}# 2 "{{.*[/\\]Inputs(/|)}}rewrite-includes8.h"{{$}}
+// CHECK-NEXT: {{^}}#if 0 /* expanded by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}#/*elif*/ __has_include(){{$}}
+// CHECK-NEXT: {{^}}#endif /* expanded by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}#elif 0 /* evaluated by -frewrite-includes */{{$}}
 // CHECK-NEXT: {{^}}# 3 "{{.*[/\\]Inputs(/|)}}rewrite-includes8.h"{{$}}
 // CHECK-NEXT: {{^}}#endif{{$}}
 // CHECK-NEXT: {{^}}# 4 "{{.*[/\\]Inputs(/|)}}rewrite-includes8.h"{{$}}
-// CHECK-NEXT: {{^}}#if !(1)/*__has_include("rewrite-includes8.h")*/{{$}}
+// CHECK-NEXT: {{^}}#if 0 /* expanded by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}#/*if*/ !__has_include("rewrite-includes8.h"){{$}}
+// CHECK-NEXT: {{^}}#endif /* expanded by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}#if 0 /* evaluated by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}# 5 "{{.*[/\\]Inputs(/|)}}rewrite-includes8.h"{{$}}
 // CHECK-NEXT: {{^}}#endif{{$}}
 // CHECK-NEXT: {{^}}# 6 "{{.*[/\\]Inputs(/|)}}rewrite-includes8.h"{{$}}
 // CHECK-NEXT: {{^}}# 23 "{{.*}}rewrite-includes.c" 2{{$}}
@@ -124,7 +135,11 @@
 // CHECK-NEXT: {{^}}#endif /* expanded by -frewrite-includes */{{$}}
 // CHECK-NEXT: {{^}}# 23 "{{.*}}rewrite-includes.c"{{$}}
 // CHECK-NEXT: {{^}}# 1 "{{.*[/\\]Inputs(/|)}}rewrite-includes9.h" 1{{$}}
-// CHECK-NEXT: {{^}}#if (1)/*__has_include_next()*/{{$}}
+// CHECK-NEXT: {{^}}#if 0 /* expanded by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}#/*if*/ __has_include_next(){{$}}
+// CHECK-NEXT: {{^}}#endif /* expanded by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}#if 1 /* evaluated by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}# 2 "{{.*[/\\]Inputs(/|)}}rewrite-includes9.h"{{$}}
 // CHECK-NEXT: {{^}}#if 0 /* expanded by -frewrite-includes */{{$}}
 // CHECK-NEXT: {{^}}#include_next {{$}}
 // CHECK-NEXT: {{^}}#endif /* expanded by -frewrite-includes */{{$}}
@@ -193,15 +208,27 @@
 // CHECKNL-NEXT: {{^}}#if 0 /* expanded by -frewrite-includes */{{$}}
 // CHECKNL-NEXT: {{^}}#include "rewrite-includes8.h"{{$}}
 // CHECKNL-NEXT: {{^}}#endif /* expanded by -frewrite-includes */{{$}}
-// CHECKNL-NEXT: {{^}}#if (0)/*__has_include_next()*/{{$}}
-// CHECKNL-NEXT: {{^}}#elif (0)/*__has_include()*/{{$}}
+// CHECKNL-NEXT: {{^}}#if 0 /* expanded by -frewrite-includes */{{$}}
+// CHECKNL-NEXT: {{^}}#/*if*/ __has_include_next(){{$}}
+// CHECKNL-NEXT: {{^}}#endif /* expanded by -frewrite-includes */{{$}}
+// CHECKNL-NEXT: {{^}}#if 0 /* evaluated by -frewrite-includes */{{$}}
+// CHECKNL-NEXT: {{^}}#if 0 /* expanded by -frewrite-includes */{{$}}
+// CHECKNL-NEXT: {{^}}#/*elif*/ __has_include(){{$}}
+// CHECKNL-NEXT: {{^}}#endif /* expanded by -frewrite-includes */{{$}}
+// CHECKNL-NEXT: {{^}}#elif 0 /* evaluated by -frewrite-includes */{{$}}
 // CH

[PATCH] D63979: actually also compile output in tests for -frewrite-includes

[Luboš Luňák via Phabricator via cfe-commits]

llunak created this revision.
llunak added a project: clang.
Herald added a subscriber: cfe-commits.

This depends on https://reviews.llvm.org/D63508 (where I posted one version of 
a patch that created broken output without the tests catching it).


Repository:
  rC Clang

https://reviews.llvm.org/D63979

Files:
  clang/test/Frontend/Inputs/NextIncludes/rewrite-includes9.h
  clang/test/Frontend/Inputs/rewrite-includes1.h
  clang/test/Frontend/Inputs/rewrite-includes2.h
  clang/test/Frontend/Inputs/rewrite-includes3.h
  clang/test/Frontend/Inputs/rewrite-includes4.h
  clang/test/Frontend/Inputs/rewrite-includes5.h
  clang/test/Frontend/Inputs/rewrite-includes6.h
  clang/test/Frontend/Inputs/rewrite-includes7.h
  clang/test/Frontend/rewrite-includes-cli-include.c
  clang/test/Frontend/rewrite-includes-conditions.c
  clang/test/Frontend/rewrite-includes.c

Index: clang/test/Frontend/rewrite-includes.c
===
--- clang/test/Frontend/rewrite-includes.c
+++ clang/test/Frontend/rewrite-includes.c
@@ -1,8 +1,9 @@
-// RUN: not %clang_cc1 -verify -E -frewrite-includes -DFIRST -I %S/Inputs -I %S/Inputs/NextIncludes %s -o - | FileCheck -strict-whitespace %s
-// RUN: not %clang_cc1 -verify -E -frewrite-includes -P -DFIRST -I %S/Inputs -I %S/Inputs/NextIncludes %s -o - | FileCheck -check-prefix=CHECKNL -strict-whitespace %s
+// RUN: %clang_cc1 -E -frewrite-includes -DFIRST -I %S/Inputs -I %S/Inputs/NextIncludes %s -o - | FileCheck -strict-whitespace %s
+// RUN: %clang_cc1 -E -frewrite-includes -P -DFIRST -I %S/Inputs -I %S/Inputs/NextIncludes %s -o - | FileCheck -check-prefix=CHECKNL -strict-whitespace %s
+// RUN: %clang_cc1 -E -frewrite-includes -DFIRST -I %S/Inputs -I %S/Inputs/NextIncludes %s -o - | %clang_cc1 -Wall -Wextra -Wconversion -DFIRST -x c -fsyntax-only 2>&1 | FileCheck -check-prefix=COMPILE --implicit-check-not warning: %s
 // STARTCOMPARE
 #define A(a,b) a ## b
-A(1,2)
+A(in,t) a;
 #include "rewrite-includes1.h"
 #ifdef FIRST
 #define HEADER "rewrite-includes3.h"
@@ -21,94 +22,95 @@
 #include "rewrite-includes7.h"
 #include "rewrite-includes8.h"
 #include "rewrite-includes9.h"
+static int unused;
 // ENDCOMPARE
 // CHECK: {{^}}# 1 "{{.*}}rewrite-includes.c"{{$}}
 // CHECK: {{^}}// STARTCOMPARE{{$}}
 // CHECK-NEXT: {{^}}#define A(a,b) a ## b{{$}}
-// CHECK-NEXT: {{^}}A(1,2){{$}}
+// CHECK-NEXT: {{^}}A(in,t) a;{{$}}
 // CHECK-NEXT: {{^}}#if 0 /* expanded by -frewrite-includes */{{$}}
 // CHECK-NEXT: {{^}}#include "rewrite-includes1.h"{{$}}
 // CHECK-NEXT: {{^}}#endif /* expanded by -frewrite-includes */{{$}}
-// CHECK-NEXT: {{^}}# 6 "{{.*}}rewrite-includes.c"{{$}}
+// CHECK-NEXT: {{^}}# 7 "{{.*}}rewrite-includes.c"{{$}}
 // CHECK-NEXT: {{^}}# 1 "{{.*[/\\]Inputs(/|)}}rewrite-includes1.h" 1{{$}}
 // CHECK-NEXT: {{^}}#if 0 /* expanded by -frewrite-includes */{{$}}
 // CHECK-NEXT: {{^}}#pragma clang system_header{{$}}
 // CHECK-NEXT: {{^}}#endif /* expanded by -frewrite-includes */{{$}}
 // CHECK-NEXT: {{^}}# 2 "{{.*[/\\]Inputs(/|)}}rewrite-includes1.h" 3{{$}}
-// CHECK-NEXT: {{^}}included_line1{{$}}
+// CHECK-NEXT: {{^}}int included_line1;{{$}}
 // CHECK-NEXT: {{^}}#if 0 /* expanded by -frewrite-includes */{{$}}
 // CHECK-NEXT: {{^}}#include "rewrite-includes2.h"{{$}}
 // CHECK-NEXT: {{^}}#endif /* expanded by -frewrite-includes */{{$}}
 // CHECK-NEXT: {{^}}# 3 "{{.*[/\\]Inputs(/|)}}rewrite-includes1.h" 3{{$}}
 // CHECK-NEXT: {{^}}# 1 "{{.*[/\\]Inputs(/|)}}rewrite-includes2.h" 1 3{{$}}
-// CHECK-NEXT: {{^}}included_line2{{$}}
+// CHECK-NEXT: {{^}}int included_line2;{{$}}
 // CHECK-NEXT: {{^}}# 4 "{{.*[/\\]Inputs(/|)}}rewrite-includes1.h" 2 3{{$}}
-// CHECK-NEXT: {{^}}# 7 "{{.*}}rewrite-includes.c" 2{{$}}
+// CHECK-NEXT: {{^}}# 8 "{{.*}}rewrite-includes.c" 2{{$}}
 // CHECK-NEXT: {{^}}#ifdef FIRST{{$}}
 // CHECK-NEXT: {{^}}#define HEADER "rewrite-includes3.h"{{$}}
 // CHECK-NEXT: {{^}}#if 0 /* expanded by -frewrite-includes */{{$}}
 // CHECK-NEXT: {{^}}#include HEADER{{$}}
 // CHECK-NEXT: {{^}}#endif /* expanded by -frewrite-includes */{{$}}
-// CHECK-NEXT: {{^}}# 9 "{{.*}}rewrite-includes.c"{{$}}
+// CHECK-NEXT: {{^}}# 10 "{{.*}}rewrite-includes.c"{{$}}
 // CHECK-NEXT: {{^}}# 1 "{{.*[/\\]Inputs(/|)}}rewrite-includes3.h" 1{{$}}
-// CHECK-NEXT: {{^}}included_line3{{$}}
-// CHECK-NEXT: {{^}}# 10 "{{.*}}rewrite-includes.c" 2{{$}}
+// CHECK-NEXT: {{^}}unsigned int included_line3 = -10;{{$}}
+// CHECK-NEXT: {{^}}# 11 "{{.*}}rewrite-includes.c" 2{{$}}
 // CHECK-NEXT: {{^}}#else{{$}}
-// CHECK-NEXT: {{^}}# 11 "{{.*}}rewrite-includes.c"{{$}}
+// CHECK-NEXT: {{^}}# 12 "{{.*}}rewrite-includes.c"{{$}}
 // CHECK-NEXT: {{^}}#if 0 /* expanded by -frewrite-includes */{{$}}
 // CHECK-NEXT: {{^}}#include "rewrite-includes4.h"{{$}}
 // CHECK-NEXT: {{^}}#endif /* expanded by -frewrite-includes */{{$}}
-// CHECK-NEXT: {{^}}# 11 "{{.*}}rewrite-includes.c"{{$}}
 // CHECK-NEXT: {{^}}# 12 "{{.*}}rewrite-includes.c"{{$}}
-// CHECK-NEXT: {{^}}#endif{{$}

[PATCH] D63508: make -frewrite-includes also rewrite conditions in #if/#elif

[Luboš Luňák via Phabricator via cfe-commits]

llunak updated this revision to Diff 207229.
llunak added a comment.

Updated the patch again, commenting out just a part of #if didn't work either, 
so it seems there's no good way to comment out unwanted #if. This patch 
surrounds rewritten #if conditions by extra #if 0 #endif block to disable them, 
moreover it makes the conditions be part of an empty #if #endif block, so they 
do not break nesting.

I've also created https://reviews.llvm.org/D63979 to verify that the results in 
fact do compile.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63508

Files:
  clang/lib/Frontend/Rewrite/InclusionRewriter.cpp
  clang/test/Frontend/rewrite-includes-conditions.c
  clang/test/Frontend/rewrite-includes.c
  clang/test/Modules/preprocess-module.cpp

Index: clang/test/Modules/preprocess-module.cpp
===
--- clang/test/Modules/preprocess-module.cpp
+++ clang/test/Modules/preprocess-module.cpp
@@ -51,7 +51,7 @@
 // RUN: %clang_cc1 -fmodules -fmodule-file=%t/file.rewrite.pcm %s -I%t -verify -fno-modules-error-recovery -DFILE_REWRITE -DINCLUDE -I%S/Inputs/preprocess
 //
 // Check that we can preprocess this user of the .pcm file.
-// RUN: %clang_cc1 -fmodules -fmodule-file=%t/file.pcm %s -I%t -E -frewrite-imports -o %t/preprocess-module.ii
+// RUN: %clang_cc1 -fmodules -fmodule-file=%t/file.pcm %s -I%t -E -frewrite-imports -DFILE_REWRITE_FULL -o %t/preprocess-module.ii
 // RUN: %clang_cc1 -fmodules %t/preprocess-module.ii -verify -fno-modules-error-recovery -DFILE_REWRITE_FULL
 //
 // Check that language / header search options are ignored when preprocessing from a .pcm file.
Index: clang/test/Frontend/rewrite-includes.c
===
--- clang/test/Frontend/rewrite-includes.c
+++ clang/test/Frontend/rewrite-includes.c
@@ -110,12 +110,27 @@
 // CHECK-NEXT: {{^}}#endif /* expanded by -frewrite-includes */{{$}}
 // CHECK-NEXT: {{^}}# 22 "{{.*}}rewrite-includes.c"{{$}}
 // CHECK-NEXT: {{^}}# 1 "{{.*[/\\]Inputs(/|)}}rewrite-includes8.h" 1{{$}}
-// CHECK-NEXT: {{^}}#if (0)/*__has_include_next()*/{{$}}
-// CHECK-NEXT: {{^}}#elif (0)/*__has_include()*/{{$}}
+// CHECK-NEXT: {{^}}#if 0 /* disabled by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}#if __has_include_next(){{$}}
+// CHECK-NEXT: {{^}}#endif{{$}}
+// CHECK-NEXT: {{^}}#endif /* disabled by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}#if 0 /* evaluated by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}# 2 "{{.*[/\\]Inputs(/|)}}rewrite-includes8.h"{{$}}
+// CHECK-NEXT: {{^}}#if 0 /* disabled by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}#if 0{{$}}
+// CHECK-NEXT: {{^}}#elif __has_include(){{$}}
+// CHECK-NEXT: {{^}}#endif{{$}}
+// CHECK-NEXT: {{^}}#endif /* disabled by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}#elif 0 /* evaluated by -frewrite-includes */{{$}}
 // CHECK-NEXT: {{^}}# 3 "{{.*[/\\]Inputs(/|)}}rewrite-includes8.h"{{$}}
 // CHECK-NEXT: {{^}}#endif{{$}}
 // CHECK-NEXT: {{^}}# 4 "{{.*[/\\]Inputs(/|)}}rewrite-includes8.h"{{$}}
-// CHECK-NEXT: {{^}}#if !(1)/*__has_include("rewrite-includes8.h")*/{{$}}
+// CHECK-NEXT: {{^}}#if 0 /* disabled by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}#if !__has_include("rewrite-includes8.h"){{$}}
+// CHECK-NEXT: {{^}}#endif{{$}}
+// CHECK-NEXT: {{^}}#endif /* disabled by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}#if 0 /* evaluated by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}# 5 "{{.*[/\\]Inputs(/|)}}rewrite-includes8.h"{{$}}
 // CHECK-NEXT: {{^}}#endif{{$}}
 // CHECK-NEXT: {{^}}# 6 "{{.*[/\\]Inputs(/|)}}rewrite-includes8.h"{{$}}
 // CHECK-NEXT: {{^}}# 23 "{{.*}}rewrite-includes.c" 2{{$}}
@@ -124,7 +139,12 @@
 // CHECK-NEXT: {{^}}#endif /* expanded by -frewrite-includes */{{$}}
 // CHECK-NEXT: {{^}}# 23 "{{.*}}rewrite-includes.c"{{$}}
 // CHECK-NEXT: {{^}}# 1 "{{.*[/\\]Inputs(/|)}}rewrite-includes9.h" 1{{$}}
-// CHECK-NEXT: {{^}}#if (1)/*__has_include_next()*/{{$}}
+// CHECK-NEXT: {{^}}#if 0 /* disabled by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}#if __has_include_next(){{$}}
+// CHECK-NEXT: {{^}}#endif{{$}}
+// CHECK-NEXT: {{^}}#endif /* disabled by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}#if 1 /* evaluated by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}# 2 "{{.*[/\\]Inputs(/|)}}rewrite-includes9.h"{{$}}
 // CHECK-NEXT: {{^}}#if 0 /* expanded by -frewrite-includes */{{$}}
 // CHECK-NEXT: {{^}}#include_next {{$}}
 // CHECK-NEXT: {{^}}#endif /* expanded by -frewrite-includes */{{$}}
@@ -193,15 +213,32 @@
 // CHECKNL-NEXT: {{^}}#if 0 /* expanded by -frewrite-includes */{{$}}
 // CHECKNL-NEXT: {{^}}#include "rewrite-includes8.h"{{$}}
 // CHECKNL-NEXT: {{^}}#endif /* expanded by -frewrite-includes */{{$}}
-// CHECKNL-NEXT: {{^}}#if (0)/*__has_include_next()*/{{$}}
-// CHECKNL-NEXT: {{^}}#elif (0)/*__has_include()*/{{$}}
+// CHECKNL-NEXT: {{^}}#if 0 /* disabled b

[PATCH] D64284: (WIP) share template instantiations from PCH in one .o if -building-pch-with-obj

[Luboš Luňák via Phabricator via cfe-commits]

llunak created this revision.
llunak added reviewers: rsmith, dblaikie, hans.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This is a WIP patch that can significantly reduce build time by avoiding 
needlessly repeatedly instantiating templates when using PCH.

As described in http://lists.llvm.org/pipermail/cfe-dev/2019-May/062371.html / 
http://llunak.blogspot.com/2019/05/why-precompiled-headers-do-not-improve.html 
, whenever a compilation uses PCH it instantiates every template that's been 
instantiated in the PCH, which can add significant build time. This patch 
allows avoiding that, if -building-pch-with-obj option is used, the templates 
will be instantiated just once in the object file that should accompany the 
PCH, and then it can be skipped during normal compilations.

Testcase: I've built an entire debug build of LibreOffice with this patch and 
it works.

Pending problems/questions:

- The patch currently skips C++ constructors and destructors, because (at least 
with the Itanium ABI) they can be emitted in several variants (complete vs base 
ctor/dtor) and I don't know how to force emitting all variants. Ctors/dtors are 
a significant portion of PCH template instantiations, so without this the 
building time improvement is not as large as it could be.

- The patch currently does not handle function inlining well (=it works only 
with -O0 builds). This optimization cannot be done with template functions that 
should be inlined by the compilation, so the code needs to check whether a 
function would possibly be inlined. The problem is that this depends on 
settings in CodeGenOptions and Sema (understandably) doesn't have an access to 
that. How can I access that in Sema? Does it make sense to try to handle this, 
or is the deciding whether a function will get actually inlined too complex and 
this should rather be skipped for CodeGen::NormalInlining?

- In rate circumstances it is possible to get undefined references because of 
instantiations in the PCH's object file. Specifically this happens if the PCH 
includes a header providing non-exported code from another module (=shared 
library), in that case the PCH may refer to it. I think this is conceptually 
not possible to handle in Clang, either the code should be cleaned up to not 
provide non-exported code to other modules, or it's possible to use linker's 
--gc-sections to drop such instantiations.

- In Sema::InstantiateFunctionDefinition() the code for extern templates still 
instantiates a function if it has getContainedAutoType(), so my code should 
probably also check that. But I'm not sure what that is (is that 'auto foo() { 
return 1; }' ?) or why that would need an instance in every TU.

- Is there a guideline on when to use data members in class Decl vs functions? 
The patch uses DeclIsPendingInstantiationFromPCHWithObjectFile() to check 
whether a FunctionDecl comes from the PCH, which may get called quite often. 
This could be optimized away by storing a flag in class Decl.


Repository:
  rC Clang

https://reviews.llvm.org/D64284

Files:
  clang/include/clang/AST/ExternalASTSource.h
  clang/include/clang/Sema/MultiplexExternalSemaSource.h
  clang/include/clang/Serialization/ASTReader.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Sema/MultiplexExternalSemaSource.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Serialization/ASTReader.cpp

Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -8293,14 +8293,14 @@
 
 void ASTReader::ReadPendingInstantiations(
SmallVectorImpl> &Pending) {
-  for (unsigned Idx = 0, N = PendingInstantiations.size(); Idx < N;) {
+  for (unsigned Idx = PendingInstantiationsReadCount, N = PendingInstantiations.size(); Idx < N;) {
 ValueDecl *D = cast(GetDecl(PendingInstantiations[Idx++]));
 SourceLocation Loc
   = SourceLocation::getFromRawEncoding(PendingInstantiations[Idx++]);
 
 Pending.push_back(std::make_pair(D, Loc));
   }
-  PendingInstantiations.clear();
+  PendingInstantiationsReadCount = PendingInstantiations.size();
 }
 
 void ASTReader::ReadLateParsedTemplates(
@@ -8522,6 +8522,17 @@
   return MF && MF->PCHHasObjectFile;
 }
 
+bool ASTReader::DeclIsPendingInstantiationFromPCHWithObjectFile(const Decl *D) {
+  if( !DeclIsFromPCHWithObjectFile(D))
+return false;
+  for (unsigned Idx = 0, N = PendingInstantiations.size(); Idx < N;) {
+if( D == cast(GetDecl(PendingInstantiations[Idx++])))
+return true;
+Idx++;
+  }
+  return false;
+}
+
 ModuleFile *ASTReader::getLocalModuleFile(ModuleFile &F, unsigned ID) {
   if (ID & 1) {
 // It's a module, look it up by submodule ID.
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

[PATCH] D61822: make -ftime-trace also print template arguments

[Luboš Luňák via Phabricator via cfe-commits]

llunak created this revision.
llunak added a reviewer: anton-afanasyev.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Without this, I get e.g. 'PerformPendingInstantiations' -> 'std::fill', now I 
get 'std::fill'.


Repository:
  rC Clang

https://reviews.llvm.org/D61822

Files:
  lib/CodeGen/CodeGenModule.cpp
  lib/Parse/ParseDeclCXX.cpp
  lib/Sema/SemaTemplateInstantiate.cpp
  lib/Sema/SemaTemplateInstantiateDecl.cpp


Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -4156,7 +4156,11 @@
   }
 
   llvm::TimeTraceScope TimeScope("InstantiateFunction", [&]() {
-return Function->getQualifiedNameAsString();
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+Function->getNameForDiagnostic(OS, getPrintingPolicy(),
+   /*Qualified=*/true);
+return Name;
   });
 
   // If we're performing recursive template instantiation, create our own
Index: lib/Sema/SemaTemplateInstantiate.cpp
===
--- lib/Sema/SemaTemplateInstantiate.cpp
+++ lib/Sema/SemaTemplateInstantiate.cpp
@@ -2014,7 +2014,11 @@
 return true;
 
   llvm::TimeTraceScope TimeScope("InstantiateClass", [&]() {
-return Instantiation->getQualifiedNameAsString();
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+Instantiation->getNameForDiagnostic(OS, getPrintingPolicy(),
+/*Qualified=*/true);
+return Name;
   });
 
   Pattern = PatternDef;
Index: lib/Parse/ParseDeclCXX.cpp
===
--- lib/Parse/ParseDeclCXX.cpp
+++ lib/Parse/ParseDeclCXX.cpp
@@ -3154,8 +3154,13 @@
  TagType == DeclSpec::TST_class) && "Invalid TagType!");
 
   llvm::TimeTraceScope TimeScope("ParseClass", [&]() {
-if (auto *TD = dyn_cast_or_null(TagDecl))
-  return TD->getQualifiedNameAsString();
+if (auto *TD = dyn_cast_or_null(TagDecl)) {
+  std::string Name;
+  llvm::raw_string_ostream OS(Name);
+  TD->getNameForDiagnostic(OS, Actions.getASTContext().getPrintingPolicy(),
+   /*Qualified=*/true);
+  return Name;
+}
 return std::string("");
   });
 
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -2489,8 +2489,13 @@
 if (!shouldEmitFunction(GD))
   return;
 
-llvm::TimeTraceScope TimeScope(
-"CodeGen Function", [&]() { return FD->getQualifiedNameAsString(); });
+llvm::TimeTraceScope TimeScope("CodeGen Function", [&]() {
+  std::string Name;
+  llvm::raw_string_ostream OS(Name);
+  FD->getNameForDiagnostic(OS, getContext().getPrintingPolicy(),
+   /*Qualified=*/true);
+  return Name;
+});
 
 if (const auto *Method = dyn_cast(D)) {
   // Make sure to emit the definition(s) before we emit the thunks.


Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -4156,7 +4156,11 @@
   }
 
   llvm::TimeTraceScope TimeScope("InstantiateFunction", [&]() {
-return Function->getQualifiedNameAsString();
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+Function->getNameForDiagnostic(OS, getPrintingPolicy(),
+   /*Qualified=*/true);
+return Name;
   });
 
   // If we're performing recursive template instantiation, create our own
Index: lib/Sema/SemaTemplateInstantiate.cpp
===
--- lib/Sema/SemaTemplateInstantiate.cpp
+++ lib/Sema/SemaTemplateInstantiate.cpp
@@ -2014,7 +2014,11 @@
 return true;
 
   llvm::TimeTraceScope TimeScope("InstantiateClass", [&]() {
-return Instantiation->getQualifiedNameAsString();
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+Instantiation->getNameForDiagnostic(OS, getPrintingPolicy(),
+/*Qualified=*/true);
+return Name;
   });
 
   Pattern = PatternDef;
Index: lib/Parse/ParseDeclCXX.cpp
===
--- lib/Parse/ParseDeclCXX.cpp
+++ lib/Parse/ParseDeclCXX.cpp
@@ -3154,8 +3154,13 @@
  TagType == DeclSpec::TST_class) && "Invalid TagType!");
 
   llvm::TimeTraceScope TimeScope("ParseClass", [&]() {
-if (auto *TD = dyn_cast_or_null(TagDecl))
-  return TD->getQualifiedNameAsString();
+if (auto *TD = dyn_cast_or_null(TagDecl)) {
+  std::string Name;
+  llvm::raw_string_ostream OS(Name);
+  TD->getNameForDiagnostic(OS, Actions.getASTContext().getPrintingPolicy(

[PATCH] D61822: make -ftime-trace also print template arguments

[Luboš Luňák via Phabricator via cfe-commits]

llunak updated this revision to Diff 199133.
llunak added a comment.

Removed the parse case, getNameForDiagnostic() apparently prints nothing there.


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

https://reviews.llvm.org/D61822

Files:
  lib/CodeGen/CodeGenModule.cpp
  lib/Sema/SemaTemplateInstantiate.cpp
  lib/Sema/SemaTemplateInstantiateDecl.cpp


Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -4156,7 +4156,11 @@
   }
 
   llvm::TimeTraceScope TimeScope("InstantiateFunction", [&]() {
-return Function->getQualifiedNameAsString();
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+Function->getNameForDiagnostic(OS, getPrintingPolicy(),
+   /*Qualified=*/true);
+return Name;
   });
 
   // If we're performing recursive template instantiation, create our own
Index: lib/Sema/SemaTemplateInstantiate.cpp
===
--- lib/Sema/SemaTemplateInstantiate.cpp
+++ lib/Sema/SemaTemplateInstantiate.cpp
@@ -2014,7 +2014,11 @@
 return true;
 
   llvm::TimeTraceScope TimeScope("InstantiateClass", [&]() {
-return Instantiation->getQualifiedNameAsString();
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+Instantiation->getNameForDiagnostic(OS, getPrintingPolicy(),
+/*Qualified=*/true);
+return Name;
   });
 
   Pattern = PatternDef;
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -2489,8 +2489,13 @@
 if (!shouldEmitFunction(GD))
   return;
 
-llvm::TimeTraceScope TimeScope(
-"CodeGen Function", [&]() { return FD->getQualifiedNameAsString(); });
+llvm::TimeTraceScope TimeScope("CodeGen Function", [&]() {
+  std::string Name;
+  llvm::raw_string_ostream OS(Name);
+  FD->getNameForDiagnostic(OS, getContext().getPrintingPolicy(),
+   /*Qualified=*/true);
+  return Name;
+});
 
 if (const auto *Method = dyn_cast(D)) {
   // Make sure to emit the definition(s) before we emit the thunks.


Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -4156,7 +4156,11 @@
   }
 
   llvm::TimeTraceScope TimeScope("InstantiateFunction", [&]() {
-return Function->getQualifiedNameAsString();
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+Function->getNameForDiagnostic(OS, getPrintingPolicy(),
+   /*Qualified=*/true);
+return Name;
   });
 
   // If we're performing recursive template instantiation, create our own
Index: lib/Sema/SemaTemplateInstantiate.cpp
===
--- lib/Sema/SemaTemplateInstantiate.cpp
+++ lib/Sema/SemaTemplateInstantiate.cpp
@@ -2014,7 +2014,11 @@
 return true;
 
   llvm::TimeTraceScope TimeScope("InstantiateClass", [&]() {
-return Instantiation->getQualifiedNameAsString();
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+Instantiation->getNameForDiagnostic(OS, getPrintingPolicy(),
+/*Qualified=*/true);
+return Name;
   });
 
   Pattern = PatternDef;
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -2489,8 +2489,13 @@
 if (!shouldEmitFunction(GD))
   return;
 
-llvm::TimeTraceScope TimeScope(
-"CodeGen Function", [&]() { return FD->getQualifiedNameAsString(); });
+llvm::TimeTraceScope TimeScope("CodeGen Function", [&]() {
+  std::string Name;
+  llvm::raw_string_ostream OS(Name);
+  FD->getNameForDiagnostic(OS, getContext().getPrintingPolicy(),
+   /*Qualified=*/true);
+  return Name;
+});
 
 if (const auto *Method = dyn_cast(D)) {
   // Make sure to emit the definition(s) before we emit the thunks.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61822: make -ftime-trace also print template arguments

[Luboš Luňák via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC360539: make -ftime-trace also print template arguments 
(authored by llunak, committed by ).

Repository:
  rC Clang

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

https://reviews.llvm.org/D61822

Files:
  lib/CodeGen/CodeGenModule.cpp
  lib/Sema/SemaTemplateInstantiate.cpp
  lib/Sema/SemaTemplateInstantiateDecl.cpp


Index: lib/Sema/SemaTemplateInstantiate.cpp
===
--- lib/Sema/SemaTemplateInstantiate.cpp
+++ lib/Sema/SemaTemplateInstantiate.cpp
@@ -2014,7 +2014,11 @@
 return true;
 
   llvm::TimeTraceScope TimeScope("InstantiateClass", [&]() {
-return Instantiation->getQualifiedNameAsString();
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+Instantiation->getNameForDiagnostic(OS, getPrintingPolicy(),
+/*Qualified=*/true);
+return Name;
   });
 
   Pattern = PatternDef;
Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -4156,7 +4156,11 @@
   }
 
   llvm::TimeTraceScope TimeScope("InstantiateFunction", [&]() {
-return Function->getQualifiedNameAsString();
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+Function->getNameForDiagnostic(OS, getPrintingPolicy(),
+   /*Qualified=*/true);
+return Name;
   });
 
   // If we're performing recursive template instantiation, create our own
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -2695,8 +2695,13 @@
 if (!shouldEmitFunction(GD))
   return;
 
-llvm::TimeTraceScope TimeScope(
-"CodeGen Function", [&]() { return FD->getQualifiedNameAsString(); });
+llvm::TimeTraceScope TimeScope("CodeGen Function", [&]() {
+  std::string Name;
+  llvm::raw_string_ostream OS(Name);
+  FD->getNameForDiagnostic(OS, getContext().getPrintingPolicy(),
+   /*Qualified=*/true);
+  return Name;
+});
 
 if (const auto *Method = dyn_cast(D)) {
   // Make sure to emit the definition(s) before we emit the thunks.


Index: lib/Sema/SemaTemplateInstantiate.cpp
===
--- lib/Sema/SemaTemplateInstantiate.cpp
+++ lib/Sema/SemaTemplateInstantiate.cpp
@@ -2014,7 +2014,11 @@
 return true;
 
   llvm::TimeTraceScope TimeScope("InstantiateClass", [&]() {
-return Instantiation->getQualifiedNameAsString();
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+Instantiation->getNameForDiagnostic(OS, getPrintingPolicy(),
+/*Qualified=*/true);
+return Name;
   });
 
   Pattern = PatternDef;
Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -4156,7 +4156,11 @@
   }
 
   llvm::TimeTraceScope TimeScope("InstantiateFunction", [&]() {
-return Function->getQualifiedNameAsString();
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+Function->getNameForDiagnostic(OS, getPrintingPolicy(),
+   /*Qualified=*/true);
+return Name;
   });
 
   // If we're performing recursive template instantiation, create our own
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -2695,8 +2695,13 @@
 if (!shouldEmitFunction(GD))
   return;
 
-llvm::TimeTraceScope TimeScope(
-"CodeGen Function", [&]() { return FD->getQualifiedNameAsString(); });
+llvm::TimeTraceScope TimeScope("CodeGen Function", [&]() {
+  std::string Name;
+  llvm::raw_string_ostream OS(Name);
+  FD->getNameForDiagnostic(OS, getContext().getPrintingPolicy(),
+   /*Qualified=*/true);
+  return Name;
+});
 
 if (const auto *Method = dyn_cast(D)) {
   // Make sure to emit the definition(s) before we emit the thunks.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65371: do not emit -Wunused-macros warnings in -frewrite-includes mode (PR15614)

[Luboš Luňák via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa507a5ec8f1d: do not emit -Wunused-macros warnings in 
-frewrite-includes mode (PR15614) (authored by llunak).

Changed prior to commit:
  https://reviews.llvm.org/D65371?vs=219370&id=220370#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65371

Files:
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Frontend/rewrite-includes-warnings.c


Index: clang/test/Frontend/rewrite-includes-warnings.c
===
--- clang/test/Frontend/rewrite-includes-warnings.c
+++ clang/test/Frontend/rewrite-includes-warnings.c
@@ -1,4 +1,7 @@
-// RUN: %clang_cc1 -verify -Wall -Wextra -E -frewrite-includes %s
+// RUN: %clang_cc1 -verify -Wall -Wextra -Wunused-macros -E -frewrite-includes 
%s
 // expected-no-diagnostics
 
 #pragma GCC visibility push (default)
+
+#define USED_MACRO 1
+int test() { return USED_MACRO; }
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -2782,7 +2782,8 @@
   // If we need warning for not using the macro, add its location in the
   // warn-because-unused-macro set. If it gets used it will be removed from 
set.
   if (getSourceManager().isInMainFile(MI->getDefinitionLoc()) &&
-  !Diags->isIgnored(diag::pp_macro_not_used, MI->getDefinitionLoc())) {
+  !Diags->isIgnored(diag::pp_macro_not_used, MI->getDefinitionLoc()) &&
+  !MacroExpansionInDirectivesOverride) {
 MI->setIsWarnIfUnused(true);
 WarnUnusedMacroLocs.insert(MI->getDefinitionLoc());
   }


Index: clang/test/Frontend/rewrite-includes-warnings.c
===
--- clang/test/Frontend/rewrite-includes-warnings.c
+++ clang/test/Frontend/rewrite-includes-warnings.c
@@ -1,4 +1,7 @@
-// RUN: %clang_cc1 -verify -Wall -Wextra -E -frewrite-includes %s
+// RUN: %clang_cc1 -verify -Wall -Wextra -Wunused-macros -E -frewrite-includes %s
 // expected-no-diagnostics
 
 #pragma GCC visibility push (default)
+
+#define USED_MACRO 1
+int test() { return USED_MACRO; }
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -2782,7 +2782,8 @@
   // If we need warning for not using the macro, add its location in the
   // warn-because-unused-macro set. If it gets used it will be removed from set.
   if (getSourceManager().isInMainFile(MI->getDefinitionLoc()) &&
-  !Diags->isIgnored(diag::pp_macro_not_used, MI->getDefinitionLoc())) {
+  !Diags->isIgnored(diag::pp_macro_not_used, MI->getDefinitionLoc()) &&
+  !MacroExpansionInDirectivesOverride) {
 MI->setIsWarnIfUnused(true);
 WarnUnusedMacroLocs.insert(MI->getDefinitionLoc());
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63508: make -frewrite-includes also rewrite conditions in #if/#elif

[Luboš Luňák via Phabricator via cfe-commits]

llunak updated this revision to Diff 220378.
llunak added a comment.

- updated to apply to current trunk
- changed misleading condition in a test


Repository:
  rC Clang

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

https://reviews.llvm.org/D63508

Files:
  clang/lib/Frontend/Rewrite/InclusionRewriter.cpp
  clang/test/Frontend/rewrite-includes-conditions.c
  clang/test/Frontend/rewrite-includes.c
  clang/test/Modules/preprocess-module.cpp

Index: clang/test/Modules/preprocess-module.cpp
===
--- clang/test/Modules/preprocess-module.cpp
+++ clang/test/Modules/preprocess-module.cpp
@@ -51,7 +51,7 @@
 // RUN: %clang_cc1 -fmodules -fmodule-file=%t/file.rewrite.pcm %s -I%t -verify -fno-modules-error-recovery -DFILE_REWRITE -DINCLUDE -I%S/Inputs/preprocess
 //
 // Check that we can preprocess this user of the .pcm file.
-// RUN: %clang_cc1 -fmodules -fmodule-file=%t/file.pcm %s -I%t -E -frewrite-imports -o %t/preprocess-module.ii
+// RUN: %clang_cc1 -fmodules -fmodule-file=%t/file.pcm %s -I%t -E -frewrite-imports -DFILE_REWRITE_FULL -o %t/preprocess-module.ii
 // RUN: %clang_cc1 -fmodules %t/preprocess-module.ii -verify -fno-modules-error-recovery -DFILE_REWRITE_FULL
 //
 // Check that language / header search options are ignored when preprocessing from a .pcm file.
Index: clang/test/Frontend/rewrite-includes.c
===
--- clang/test/Frontend/rewrite-includes.c
+++ clang/test/Frontend/rewrite-includes.c
@@ -110,12 +110,27 @@
 // CHECK-NEXT: {{^}}#endif /* expanded by -frewrite-includes */{{$}}
 // CHECK-NEXT: {{^}}# 22 "{{.*}}rewrite-includes.c"{{$}}
 // CHECK-NEXT: {{^}}# 1 "{{.*[/\\]Inputs(/|)}}rewrite-includes8.h" 1{{$}}
-// CHECK-NEXT: {{^}}#if (0)/*__has_include_next()*/{{$}}
-// CHECK-NEXT: {{^}}#elif (0)/*__has_include()*/{{$}}
+// CHECK-NEXT: {{^}}#if 0 /* disabled by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}#if __has_include_next(){{$}}
+// CHECK-NEXT: {{^}}#endif{{$}}
+// CHECK-NEXT: {{^}}#endif /* disabled by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}#if 0 /* evaluated by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}# 2 "{{.*[/\\]Inputs(/|)}}rewrite-includes8.h"{{$}}
+// CHECK-NEXT: {{^}}#if 0 /* disabled by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}#if 0{{$}}
+// CHECK-NEXT: {{^}}#elif __has_include(){{$}}
+// CHECK-NEXT: {{^}}#endif{{$}}
+// CHECK-NEXT: {{^}}#endif /* disabled by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}#elif 0 /* evaluated by -frewrite-includes */{{$}}
 // CHECK-NEXT: {{^}}# 3 "{{.*[/\\]Inputs(/|)}}rewrite-includes8.h"{{$}}
 // CHECK-NEXT: {{^}}#endif{{$}}
 // CHECK-NEXT: {{^}}# 4 "{{.*[/\\]Inputs(/|)}}rewrite-includes8.h"{{$}}
-// CHECK-NEXT: {{^}}#if !(1)/*__has_include("rewrite-includes8.h")*/{{$}}
+// CHECK-NEXT: {{^}}#if 0 /* disabled by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}#if !__has_include("rewrite-includes8.h"){{$}}
+// CHECK-NEXT: {{^}}#endif{{$}}
+// CHECK-NEXT: {{^}}#endif /* disabled by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}#if 0 /* evaluated by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}# 5 "{{.*[/\\]Inputs(/|)}}rewrite-includes8.h"{{$}}
 // CHECK-NEXT: {{^}}#endif{{$}}
 // CHECK-NEXT: {{^}}# 6 "{{.*[/\\]Inputs(/|)}}rewrite-includes8.h"{{$}}
 // CHECK-NEXT: {{^}}# 23 "{{.*}}rewrite-includes.c" 2{{$}}
@@ -124,7 +139,12 @@
 // CHECK-NEXT: {{^}}#endif /* expanded by -frewrite-includes */{{$}}
 // CHECK-NEXT: {{^}}# 23 "{{.*}}rewrite-includes.c"{{$}}
 // CHECK-NEXT: {{^}}# 1 "{{.*[/\\]Inputs(/|)}}rewrite-includes9.h" 1{{$}}
-// CHECK-NEXT: {{^}}#if (1)/*__has_include_next()*/{{$}}
+// CHECK-NEXT: {{^}}#if 0 /* disabled by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}#if __has_include_next(){{$}}
+// CHECK-NEXT: {{^}}#endif{{$}}
+// CHECK-NEXT: {{^}}#endif /* disabled by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}#if 1 /* evaluated by -frewrite-includes */{{$}}
+// CHECK-NEXT: {{^}}# 2 "{{.*[/\\]Inputs(/|)}}rewrite-includes9.h"{{$}}
 // CHECK-NEXT: {{^}}#if 0 /* expanded by -frewrite-includes */{{$}}
 // CHECK-NEXT: {{^}}#include_next {{$}}
 // CHECK-NEXT: {{^}}#endif /* expanded by -frewrite-includes */{{$}}
@@ -193,15 +213,32 @@
 // CHECKNL-NEXT: {{^}}#if 0 /* expanded by -frewrite-includes */{{$}}
 // CHECKNL-NEXT: {{^}}#include "rewrite-includes8.h"{{$}}
 // CHECKNL-NEXT: {{^}}#endif /* expanded by -frewrite-includes */{{$}}
-// CHECKNL-NEXT: {{^}}#if (0)/*__has_include_next()*/{{$}}
-// CHECKNL-NEXT: {{^}}#elif (0)/*__has_include()*/{{$}}
+// CHECKNL-NEXT: {{^}}#if 0 /* disabled by -frewrite-includes */{{$}}
+// CHECKNL-NEXT: {{^}}#if __has_include_next(){{$}}
 // CHECKNL-NEXT: {{^}}#endif{{$}}
-// CHECKNL-NEXT: {{^}}#if !(1)/*__has_include("rewrite-includes8.h")*/{{$}}
+// CHECKNL-NEXT: {{^}}#endif /* disabled by -frewrite-includes */{{$}}
+// CHECKNL-NEXT: {{^}}#if 0 /* evaluated by -frewrite-includes */{{$}}
+// CHECKNL-NEXT: {{

[PATCH] D63508: make -frewrite-includes also rewrite conditions in #if/#elif

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

I've noticed that the Developer Policy says that it's actually allowed to 
commit patches without approval for parts "that you have contributed or 
maintain". Given that I'm the author of -rewrite-includes I take it that it's 
ok if I commit this if there are no further comments.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63508



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

[PATCH] D63508: make -frewrite-includes also rewrite conditions in #if/#elif

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

In D63508#1671776 , @torarnv wrote:

> So, this will make `-frewrite-includes` do more work, to ensure that it not 
> only covers the "top level" `#include` or `__has_include` case, but also 
> `__has_include` in one or more levels of macros?
>
> Does that effectively means it needs to do full preprocessing?


This patch does not make -frewrite-includes do any more work. It already does a 
partial preprocessing run which evaluates all preprocessor directives. The 
patch just make better use of that information.

> Does it depend on whether `-E` is passed with `-frewrite-includes`?

You cannot pass -frewrite-includes without -E.

> Will this mean that cases like icecream and distcc that use 
> `-frewrite-includes` will have fully preprocessed sources to send to the 
> remote hosts (with correct line/column number when compilation fails), or 
> should they still pass `-D` options during the remote compilation?

The patch does not change anything about the usage. -frewrite-includes still 
only expands all #include directives and that's it.

> How does `ICECC_REMOTE_CPP` work in this case?

The same way as before. You preferably do nothing, which defaults to 
ICECC_REMOTE_CPP=1, which uses -frewrite-includes to send full source to the 
remote, which does full preprocessing and compilation and you get pretty much 
the same result as if compiling locally. Or you may go with ICECC_REMOTE_CPP=0, 
which will not do this and then you'll most likely get warnings caused by 
compiling already preprocessed source. As for ccache, I personally consider 
suggesting explicit passing of -frewrite-includes to the compilation as 
somewhat lame; if ccache requires it for the cpp run, it can silently add it 
under the hood the same way icecream does, or at least it can have it 
configurable the same as other options and not require messing with build 
options. I myself use ccache with CCACHE_DEPEND=1, which completely avoids any 
preprocessor usage.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63508



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

[PATCH] D63508: make -frewrite-includes also rewrite conditions in #if/#elif

[Luboš Luňák via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL372248: make -frewrite-includes also rewrite conditions in 
#if/#elif (authored by llunak, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63508?vs=220378&id=220722#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63508

Files:
  cfe/trunk/lib/Frontend/Rewrite/InclusionRewriter.cpp
  cfe/trunk/test/Frontend/rewrite-includes-conditions.c
  cfe/trunk/test/Frontend/rewrite-includes.c
  cfe/trunk/test/Modules/preprocess-module.cpp

Index: cfe/trunk/lib/Frontend/Rewrite/InclusionRewriter.cpp
===
--- cfe/trunk/lib/Frontend/Rewrite/InclusionRewriter.cpp
+++ cfe/trunk/lib/Frontend/Rewrite/InclusionRewriter.cpp
@@ -49,6 +49,8 @@
   std::map ModuleIncludes;
   /// Tracks where inclusions that enter modules (in a module build) are found.
   std::map ModuleEntryIncludes;
+  /// Tracks where #if and #elif directives get evaluated and whether to true.
+  std::map IfConditions;
   /// Used transitively for building up the FileIncludes mapping over the
   /// various \c PPCallbacks callbacks.
   SourceLocation LastInclusionLocation;
@@ -78,6 +80,10 @@
   StringRef SearchPath, StringRef RelativePath,
   const Module *Imported,
   SrcMgr::CharacteristicKind FileType) override;
+  void If(SourceLocation Loc, SourceRange ConditionRange,
+  ConditionValueKind ConditionValue) override;
+  void Elif(SourceLocation Loc, SourceRange ConditionRange,
+ConditionValueKind ConditionValue, SourceLocation IfLoc) override;
   void WriteLineInfo(StringRef Filename, int Line,
  SrcMgr::CharacteristicKind FileType,
  StringRef Extra = StringRef());
@@ -89,12 +95,10 @@
   void CommentOutDirective(Lexer &DirectivesLex, const Token &StartToken,
const MemoryBuffer &FromFile, StringRef EOL,
unsigned &NextToWrite, int &Lines);
-  bool HandleHasInclude(FileID FileId, Lexer &RawLex,
-const DirectoryLookup *Lookup, Token &Tok,
-bool &FileExists);
   const IncludedFile *FindIncludeAtLocation(SourceLocation Loc) const;
   const Module *FindModuleAtLocation(SourceLocation Loc) const;
   const Module *FindEnteredModule(SourceLocation Loc) const;
+  bool IsIfAtLocationTrue(SourceLocation Loc) const;
   StringRef NextIdentifierName(Lexer &RawLex, Token &RawToken);
 };
 
@@ -203,6 +207,23 @@
 LastInclusionLocation = HashLoc;
 }
 
+void InclusionRewriter::If(SourceLocation Loc, SourceRange ConditionRange,
+   ConditionValueKind ConditionValue) {
+  auto P = IfConditions.insert(
+  std::make_pair(Loc.getRawEncoding(), ConditionValue == CVK_True));
+  (void)P;
+  assert(P.second && "Unexpected revisitation of the same if directive");
+}
+
+void InclusionRewriter::Elif(SourceLocation Loc, SourceRange ConditionRange,
+ ConditionValueKind ConditionValue,
+ SourceLocation IfLoc) {
+  auto P = IfConditions.insert(
+  std::make_pair(Loc.getRawEncoding(), ConditionValue == CVK_True));
+  (void)P;
+  assert(P.second && "Unexpected revisitation of the same elif directive");
+}
+
 /// Simple lookup for a SourceLocation (specifically one denoting the hash in
 /// an inclusion directive) in the map of inclusion information, FileChanges.
 const InclusionRewriter::IncludedFile *
@@ -233,6 +254,13 @@
   return nullptr;
 }
 
+bool InclusionRewriter::IsIfAtLocationTrue(SourceLocation Loc) const {
+  const auto I = IfConditions.find(Loc.getRawEncoding());
+  if (I != IfConditions.end())
+return I->second;
+  return false;
+}
+
 /// Detect the likely line ending style of \p FromFile by examining the first
 /// newline found within it.
 static StringRef DetectEOL(const MemoryBuffer &FromFile) {
@@ -346,80 +374,6 @@
   return StringRef();
 }
 
-// Expand __has_include and __has_include_next if possible. If there's no
-// definitive answer return false.
-bool InclusionRewriter::HandleHasInclude(
-FileID FileId, Lexer &RawLex, const DirectoryLookup *Lookup, Token &Tok,
-bool &FileExists) {
-  // Lex the opening paren.
-  RawLex.LexFromRawLexer(Tok);
-  if (Tok.isNot(tok::l_paren))
-return false;
-
-  RawLex.LexFromRawLexer(Tok);
-
-  SmallString<128> FilenameBuffer;
-  StringRef Filename;
-  // Since the raw lexer doesn't give us angle_literals we have to parse them
-  // ourselves.
-  // FIXME: What to do if the file name is a macro?
-  if (Tok.is(tok::less)) {
-RawLex.LexFromRawLexer(Tok);
-
-FilenameBuffer += '<';
-do {
-  if (Tok.is(tok::eod)) // Sanity check.
-return false;
-
-  if (Tok.is(tok::raw

[PATCH] D63979: actually also compile output in tests for -frewrite-includes

[Luboš Luňák via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL372250: actually also compile output in tests for 
-frewrite-includes (authored by llunak, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63979?vs=207226&id=220725#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63979

Files:
  cfe/trunk/test/Frontend/Inputs/NextIncludes/rewrite-includes9.h
  cfe/trunk/test/Frontend/Inputs/rewrite-includes1.h
  cfe/trunk/test/Frontend/Inputs/rewrite-includes2.h
  cfe/trunk/test/Frontend/Inputs/rewrite-includes3.h
  cfe/trunk/test/Frontend/Inputs/rewrite-includes4.h
  cfe/trunk/test/Frontend/Inputs/rewrite-includes5.h
  cfe/trunk/test/Frontend/Inputs/rewrite-includes6.h
  cfe/trunk/test/Frontend/Inputs/rewrite-includes7.h
  cfe/trunk/test/Frontend/rewrite-includes-cli-include.c
  cfe/trunk/test/Frontend/rewrite-includes-conditions.c
  cfe/trunk/test/Frontend/rewrite-includes.c

Index: cfe/trunk/test/Frontend/rewrite-includes.c
===
--- cfe/trunk/test/Frontend/rewrite-includes.c
+++ cfe/trunk/test/Frontend/rewrite-includes.c
@@ -1,8 +1,9 @@
-// RUN: not %clang_cc1 -verify -E -frewrite-includes -DFIRST -I %S/Inputs -I %S/Inputs/NextIncludes %s -o - | FileCheck -strict-whitespace %s
-// RUN: not %clang_cc1 -verify -E -frewrite-includes -P -DFIRST -I %S/Inputs -I %S/Inputs/NextIncludes %s -o - | FileCheck -check-prefix=CHECKNL -strict-whitespace %s
+// RUN: %clang_cc1 -E -frewrite-includes -DFIRST -I %S/Inputs -I %S/Inputs/NextIncludes %s -o - | FileCheck -strict-whitespace %s
+// RUN: %clang_cc1 -E -frewrite-includes -P -DFIRST -I %S/Inputs -I %S/Inputs/NextIncludes %s -o - | FileCheck -check-prefix=CHECKNL -strict-whitespace %s
+// RUN: %clang_cc1 -E -frewrite-includes -DFIRST -I %S/Inputs -I %S/Inputs/NextIncludes %s -o - | %clang_cc1 -Wall -Wextra -Wconversion -DFIRST -x c -fsyntax-only 2>&1 | FileCheck -check-prefix=COMPILE --implicit-check-not warning: %s
 // STARTCOMPARE
 #define A(a,b) a ## b
-A(1,2)
+A(in,t) a;
 #include "rewrite-includes1.h"
 #ifdef FIRST
 #define HEADER "rewrite-includes3.h"
@@ -21,94 +22,95 @@
 #include "rewrite-includes7.h"
 #include "rewrite-includes8.h"
 #include "rewrite-includes9.h"
+static int unused;
 // ENDCOMPARE
 // CHECK: {{^}}# 1 "{{.*}}rewrite-includes.c"{{$}}
 // CHECK: {{^}}// STARTCOMPARE{{$}}
 // CHECK-NEXT: {{^}}#define A(a,b) a ## b{{$}}
-// CHECK-NEXT: {{^}}A(1,2){{$}}
+// CHECK-NEXT: {{^}}A(in,t) a;{{$}}
 // CHECK-NEXT: {{^}}#if 0 /* expanded by -frewrite-includes */{{$}}
 // CHECK-NEXT: {{^}}#include "rewrite-includes1.h"{{$}}
 // CHECK-NEXT: {{^}}#endif /* expanded by -frewrite-includes */{{$}}
-// CHECK-NEXT: {{^}}# 6 "{{.*}}rewrite-includes.c"{{$}}
+// CHECK-NEXT: {{^}}# 7 "{{.*}}rewrite-includes.c"{{$}}
 // CHECK-NEXT: {{^}}# 1 "{{.*[/\\]Inputs(/|)}}rewrite-includes1.h" 1{{$}}
 // CHECK-NEXT: {{^}}#if 0 /* expanded by -frewrite-includes */{{$}}
 // CHECK-NEXT: {{^}}#pragma clang system_header{{$}}
 // CHECK-NEXT: {{^}}#endif /* expanded by -frewrite-includes */{{$}}
 // CHECK-NEXT: {{^}}# 2 "{{.*[/\\]Inputs(/|)}}rewrite-includes1.h" 3{{$}}
-// CHECK-NEXT: {{^}}included_line1{{$}}
+// CHECK-NEXT: {{^}}int included_line1;{{$}}
 // CHECK-NEXT: {{^}}#if 0 /* expanded by -frewrite-includes */{{$}}
 // CHECK-NEXT: {{^}}#include "rewrite-includes2.h"{{$}}
 // CHECK-NEXT: {{^}}#endif /* expanded by -frewrite-includes */{{$}}
 // CHECK-NEXT: {{^}}# 3 "{{.*[/\\]Inputs(/|)}}rewrite-includes1.h" 3{{$}}
 // CHECK-NEXT: {{^}}# 1 "{{.*[/\\]Inputs(/|)}}rewrite-includes2.h" 1 3{{$}}
-// CHECK-NEXT: {{^}}included_line2{{$}}
+// CHECK-NEXT: {{^}}int included_line2;{{$}}
 // CHECK-NEXT: {{^}}# 4 "{{.*[/\\]Inputs(/|)}}rewrite-includes1.h" 2 3{{$}}
-// CHECK-NEXT: {{^}}# 7 "{{.*}}rewrite-includes.c" 2{{$}}
+// CHECK-NEXT: {{^}}# 8 "{{.*}}rewrite-includes.c" 2{{$}}
 // CHECK-NEXT: {{^}}#ifdef FIRST{{$}}
 // CHECK-NEXT: {{^}}#define HEADER "rewrite-includes3.h"{{$}}
 // CHECK-NEXT: {{^}}#if 0 /* expanded by -frewrite-includes */{{$}}
 // CHECK-NEXT: {{^}}#include HEADER{{$}}
 // CHECK-NEXT: {{^}}#endif /* expanded by -frewrite-includes */{{$}}
-// CHECK-NEXT: {{^}}# 9 "{{.*}}rewrite-includes.c"{{$}}
+// CHECK-NEXT: {{^}}# 10 "{{.*}}rewrite-includes.c"{{$}}
 // CHECK-NEXT: {{^}}# 1 "{{.*[/\\]Inputs(/|)}}rewrite-includes3.h" 1{{$}}
-// CHECK-NEXT: {{^}}included_line3{{$}}
-// CHECK-NEXT: {{^}}# 10 "{{.*}}rewrite-includes.c" 2{{$}}
+// CHECK-NEXT: {{^}}unsigned int included_line3 = -10;{{$}}
+// CHECK-NEXT: {{^}}# 11 "{{.*}}rewrite-includes.c" 2{{$}}
 // CHECK-NEXT: {{^}}#else{{$}}
-// CHECK-NEXT: {{^}}# 11 "{{.*}}rewrite-includes.c"{{$}}
+// CHECK-NEXT: {{^}}# 12 "{{.*}}rewrite-includes.c"{{$}}
 // CHECK-NEXT: {{^}}#if 0 /* expa

[PATCH] D69585: Add option to instantiate templates already in the PCH

[Luboš Luňák via Phabricator via cfe-commits]

llunak marked an inline comment as done.
llunak added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5610
+  if (Args.hasFlag(options::OPT_fpch_instantiate_templates,
+   options::OPT_fno_pch_instantiate_templates, false))
+CmdArgs.push_back("-fpch-instantiate-templates");

rnk wrote:
> Does MSVC default to this behavior? Should this default to true with clang-cl 
> /Yu / /Yc? This can be future work and does not need to be part of this patch.
Since MSVC is noticeably faster for an equivalent PCH compile than current 
Clang, presumably it instantiates templates already in the PCH. But that 
doesn't really matter for this patch, if it were ok to enable this by default 
for clang-cl, than it would be ok also for clang itself. That cannot be done 
now though, https://reviews.llvm.org/D69585#1946765 points out a corner case 
where this change makes a valid compilation error out, and that's the reason 
for having this behind a flag. I expect Clang could possibly be adjusted to 
bail out and delay template instantiantion in such a case until it can be 
performed successfully, but given the response rate to my PCH patches I first 
wanted to get the feature in somehow, and I can try to make the flag 
default/irrelevant later.



Repository:
  rC Clang

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

https://reviews.llvm.org/D69585



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

[PATCH] D69585: Add option to instantiate templates already in the PCH

[Luboš Luňák via Phabricator via cfe-commits]

llunak updated this revision to Diff 265918.
llunak edited the summary of this revision.
llunak added a comment.

Enabled the option by default for clang-cl to match MSVC.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69585

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/PCH/codegen.cpp
  clang/test/PCH/crash-12631281.cpp
  clang/test/PCH/cxx-alias-decl.cpp
  clang/test/PCH/cxx-dependent-sized-ext-vector.cpp
  clang/test/PCH/cxx-explicit-specifier.cpp
  clang/test/PCH/cxx-exprs.cpp
  clang/test/PCH/cxx-friends.cpp
  clang/test/PCH/cxx-member-init.cpp
  clang/test/PCH/cxx-ms-function-specialization-class-scope.cpp
  clang/test/PCH/cxx-static_assert.cpp
  clang/test/PCH/cxx-templates.cpp
  clang/test/PCH/cxx-variadic-templates-with-default-params.cpp
  clang/test/PCH/cxx-variadic-templates.cpp
  clang/test/PCH/cxx0x-default-delete.cpp
  clang/test/PCH/cxx11-constexpr.cpp
  clang/test/PCH/cxx11-enum-template.cpp
  clang/test/PCH/cxx11-exception-spec.cpp
  clang/test/PCH/cxx11-inheriting-ctors.cpp
  clang/test/PCH/cxx11-user-defined-literals.cpp
  clang/test/PCH/cxx1y-decltype-auto.cpp
  clang/test/PCH/cxx1y-deduced-return-type.cpp
  clang/test/PCH/cxx1y-default-initializer.cpp
  clang/test/PCH/cxx1y-init-captures.cpp
  clang/test/PCH/cxx1y-variable-templates.cpp
  clang/test/PCH/cxx1z-aligned-alloc.cpp
  clang/test/PCH/cxx1z-decomposition.cpp
  clang/test/PCH/cxx1z-using-declaration.cpp
  clang/test/PCH/cxx2a-bitfield-init.cpp
  clang/test/PCH/cxx2a-concept-specialization-expr.cpp
  clang/test/PCH/cxx2a-constraints.cpp
  clang/test/PCH/cxx2a-defaulted-comparison.cpp
  clang/test/PCH/cxx2a-requires-expr.cpp
  clang/test/PCH/cxx2a-template-lambdas.cpp
  clang/test/PCH/delayed-pch-instantiate.cpp
  clang/test/PCH/friend-template.cpp
  clang/test/PCH/implicitly-deleted.cpp
  clang/test/PCH/late-parsed-instantiations.cpp
  clang/test/PCH/local_static.cpp
  clang/test/PCH/macro-undef.cpp
  clang/test/PCH/make-integer-seq.cpp
  clang/test/PCH/ms-if-exists.cpp
  clang/test/PCH/pch-instantiate-templates-forward-decl.cpp
  clang/test/PCH/pch-instantiate-templates.cpp
  clang/test/PCH/pr18806.cpp
  clang/test/PCH/pragma-diag-section.cpp
  clang/test/PCH/rdar10830559.cpp
  clang/test/PCH/specialization-after-instantiation.cpp
  clang/test/PCH/type_pack_element.cpp

Index: clang/test/PCH/type_pack_element.cpp
===
--- clang/test/PCH/type_pack_element.cpp
+++ clang/test/PCH/type_pack_element.cpp
@@ -1,6 +1,9 @@
 // RUN: %clang_cc1 -std=c++14 -x c++-header %s -emit-pch -o %t.pch
 // RUN: %clang_cc1 -std=c++14 -x c++ /dev/null -include-pch %t.pch
 
+// RUN: %clang_cc1 -std=c++14 -x c++-header %s -emit-pch -fpch-instantiate-templates -o %t.pch
+// RUN: %clang_cc1 -std=c++14 -x c++ /dev/null -include-pch %t.pch
+
 template 
 struct X { };
 
Index: clang/test/PCH/specialization-after-instantiation.cpp
===
--- /dev/null
+++ clang/test/PCH/specialization-after-instantiation.cpp
@@ -0,0 +1,32 @@
+// Test this without pch.
+// RUN: %clang_cc1 -fsyntax-only -verify -DBODY %s
+
+// Test with pch.
+// RUN: %clang_cc1 -emit-pch -o %t %s
+// RUN: %clang_cc1 -include-pch %t -fsyntax-only -verify -DBODY %s
+
+// RUN: %clang_cc1 -emit-pch -fpch-instantiate-templates -o %t %s
+// RUN: %clang_cc1 -include-pch %t -fsyntax-only -verify -DBODY %s
+
+#ifndef HEADER_H
+#define HEADER_H
+
+template 
+struct A {
+  int foo() const;
+};
+
+int bar(A *a) {
+  return a->foo();
+}
+
+#endif // HEADER_H
+
+#ifdef BODY
+
+template <>
+int A::foo() const { // expected-error {{explicit specialization of 'foo' after instantiation}}  // expected-note@20 {{implicit instantiation first required here}}
+  return 10;
+}
+
+#endif // BODY
Index: clang/test/PCH/rdar10830559.cpp
===
--- clang/test/PCH/rdar10830559.cpp
+++ clang/test/PCH/rdar10830559.cpp
@@ -6,6 +6,9 @@
 // RUN: %clang_cc1 -emit-pch -o %t %s
 // RUN: %clang_cc1 -include-pch %t -emit-llvm-only %t.empty.cpp 
 
+// RUN: %clang_cc1 -emit-pch -fpch-instantiate-templates -o %t %s
+// RUN: %clang_cc1 -include-pch %t -emit-llvm-only %t.empty.cpp
+
 // rdar://10830559
 
 //#pragma ms_struct on
Index: clang/test/PCH/pragma-diag-section.cpp
===
--- clang/test/PCH/pragma-diag-section.cpp
+++ clang/test/PCH/pragma-diag-section.cpp
@@ -5,6 +5,9 @@
 // RUN: %clang_cc1 %s -emit-pch -o %t
 // RUN: %clang_cc1 %s -include-pch %t -verify -fsyntax-only -Wuninitialized
 
+// RUN: %clang_cc1 %s -emit-pch -fpch-instantiate-templates -o %t
+// RUN: %clang_cc

[PATCH] D69585: Add option to instantiate templates already in the PCH

[Luboš Luňák via Phabricator via cfe-commits]

llunak marked an inline comment as done.
llunak added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5610
+  if (Args.hasFlag(options::OPT_fpch_instantiate_templates,
+   options::OPT_fno_pch_instantiate_templates, false))
+CmdArgs.push_back("-fpch-instantiate-templates");

rnk wrote:
> llunak wrote:
> > rnk wrote:
> > > Does MSVC default to this behavior? Should this default to true with 
> > > clang-cl /Yu / /Yc? This can be future work and does not need to be part 
> > > of this patch.
> > Since MSVC is noticeably faster for an equivalent PCH compile than current 
> > Clang, presumably it instantiates templates already in the PCH. But that 
> > doesn't really matter for this patch, if it were ok to enable this by 
> > default for clang-cl, than it would be ok also for clang itself. That 
> > cannot be done now though, https://reviews.llvm.org/D69585#1946765 points 
> > out a corner case where this change makes a valid compilation error out, 
> > and that's the reason for having this behind a flag. I expect Clang could 
> > possibly be adjusted to bail out and delay template instantiantion in such 
> > a case until it can be performed successfully, but given the response rate 
> > to my PCH patches I first wanted to get the feature in somehow, and I can 
> > try to make the flag default/irrelevant later.
> > 
> Right, I guess what I mean is, for that example where instantiation at the 
> end of the PCH creates an error, does MSVC emit an error? I just checked, and 
> it looks like the answer is yes, so it seems like we could make this the 
> default behavior for `clang-cl /Yc` without much further discussion.
That's a good point. Yes, since MSVC creates PCHs by basically compiling an 
empty .cpp, it apparently does instantiate templates as a side-effect of that, 
and https://reviews.llvm.org/D69585#1946765 indeed doesn't work with MSVC. So 
no harm in enabling the option for clang-cl.

I'll update the patch.



Repository:
  rC Clang

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

https://reviews.llvm.org/D69585



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

[PATCH] D83652: Merge some of the PCH object support with modular codegen

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

> @llunak Would you be able to test this on anything you've got?

No, but thinking more about this, I think dllexport specifically voids the 
possible problems I listed. If I'm getting it right, dllexport is used only for 
code in the current library, so codegen won't create code referencing private 
symbols in other libraries, and dllexport means the code will be emitted 
anyway, so no unnecessary code generating. So I'm actually fine with the patch 
as it is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83652



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

[PATCH] D83623: add -fpch-codegen/debuginfo options mapping to -fmodules-codegen/debuginfo

[Luboš Luňák via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG54eea6127c4d: add -fpch-codegen/debuginfo mapping to 
-fmodules-codegen/debuginfo (authored by llunak).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D83623?vs=278998&id=279720#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83623

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/pch-codegen.cpp

Index: clang/test/Driver/pch-codegen.cpp
===
--- clang/test/Driver/pch-codegen.cpp
+++ clang/test/Driver/pch-codegen.cpp
@@ -7,21 +7,21 @@
 // CHECK-PCH-CREATE-NOT: -fmodules-codegen
 // CHECK-PCH-CREATE-NOT: -fmodules-debuginfo
 
-// Create PCH with -fmodules-codegen.
-// RUN: %clang -x c++-header -Xclang -fmodules-codegen %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-cg.pch -### 2>&1 | FileCheck %s -check-prefix=CHECK-PCH-CODEGEN-CREATE
+// Create PCH with -fpch-codegen.
+// RUN: %clang -x c++-header -fpch-codegen %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-cg.pch -### 2>&1 | FileCheck %s -check-prefix=CHECK-PCH-CODEGEN-CREATE
 // CHECK-PCH-CODEGEN-CREATE: -emit-pch
 // CHECK-PCH-CODEGEN-CREATE: -fmodules-codegen
 // CHECK-PCH-CODEGEN-CREATE: "-x" "c++-header"
 // CHECK-PCH-CODEGEN-CREATE-NOT: -fmodules-debuginfo
 
-// Create PCH with -fmodules-debuginfo.
-// RUN: %clang -x c++-header -Xclang -fmodules-debuginfo %S/../Modules/Inputs/codegen-flags/foo.h -g -o %t/foo-di.pch -### 2>&1 | FileCheck %s -check-prefix=CHECK-PCH-DEBUGINFO-CREATE
+// Create PCH with -fpch-debuginfo.
+// RUN: %clang -x c++-header -fpch-debuginfo %S/../Modules/Inputs/codegen-flags/foo.h -g -o %t/foo-di.pch -### 2>&1 | FileCheck %s -check-prefix=CHECK-PCH-DEBUGINFO-CREATE
 // CHECK-PCH-DEBUGINFO-CREATE: -emit-pch
 // CHECK-PCH-DEBUGINFO-CREATE: -fmodules-debuginfo
 // CHECK-PCH-DEBUGINFO-CREATE: "-x" "c++-header"
 // CHECK-PCH-DEBUGINFO-CREATE-NOT: -fmodules-codegen
 
-// Create PCH's object file for -fmodules-codegen.
+// Create PCH's object file for -fpch-codegen.
 // RUN: touch %t/foo-cg.pch
 // RUN: %clang -c %t/foo-cg.pch -o %t/foo-cg.o -### 2>&1 | FileCheck %s -check-prefix=CHECK-PCH-CODEGEN-OBJ
 // CHECK-PCH-CODEGEN-OBJ: -emit-obj
@@ -29,7 +29,7 @@
 // CHECK-PCH-CODEGEN-OBJ: "-o" "{{.*}}foo-cg.o"
 // CHECK-PCH-CODEGEN-OBJ: "-x" "precompiled-header"
 
-// Create PCH's object file for -fmodules-debuginfo.
+// Create PCH's object file for -fpch-debuginfo.
 // RUN: touch %t/foo-di.pch
 // RUN: %clang -c %t/foo-di.pch -g -o %t/foo-di.o -### 2>&1 | FileCheck %s -check-prefix=CHECK-PCH-DEBUGINFO-OBJ
 // CHECK-PCH-DEBUGINFO-OBJ: -emit-obj
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5642,6 +5642,12 @@
   if (Args.hasFlag(options::OPT_fpch_instantiate_templates,
options::OPT_fno_pch_instantiate_templates, false))
 CmdArgs.push_back("-fpch-instantiate-templates");
+  if (Args.hasFlag(options::OPT_fpch_codegen, options::OPT_fno_pch_codegen,
+   false))
+CmdArgs.push_back("-fmodules-codegen");
+  if (Args.hasFlag(options::OPT_fpch_debuginfo, options::OPT_fno_pch_debuginfo,
+   false))
+CmdArgs.push_back("-fmodules-debuginfo");
 
   Args.AddLastArg(CmdArgs, options::OPT_fexperimental_new_pass_manager,
   options::OPT_fno_experimental_new_pass_manager);
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1440,6 +1440,10 @@
 def fno_pch_instantiate_templates:
   Flag <["-"], "fno-pch-instantiate-templates">,
   Group, Flags<[CC1Option]>;
+defm pch_codegen: OptInFFlag<"pch-codegen", "Generate ", "Do not generate ",
+  "code for uses of this PCH that assumes an explicit object file will be built for the PCH">;
+defm pch_debuginfo: OptInFFlag<"pch-debuginfo", "Generate ", "Do not generate ",
+  "debug info for types in an object file built from this PCH and do not generate them elsewhere">;
 
 def fmodules : Flag <["-"], "fmodules">, Group,
   Flags<[DriverOption, CC1Option]>,
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -63,6 +63,32 @@
 
 - ...
 
+- -fpch-codegen and -fpch-debuginfo generate shared code and/or debuginfo
+  for contents of a precompiled header in a separate object file. This object
+  file needs to be linked in, but its contents do not need to be generated
+  for other objects using the precompiled header. This should us

[Differential] D83623: add -fpch-codegen/debuginfo options mapping to -fmodules-codegen/debuginfo

[Luboš Luňák via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG54eea6127c4d: add -fpch-codegen/debuginfo mapping to 
-fmodules-codegen/debuginfo (authored by llunak).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D83623?vs=277671&id=279074#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83623

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/pch-codegen.cpp

Index: clang/test/Driver/pch-codegen.cpp
===
--- clang/test/Driver/pch-codegen.cpp
+++ clang/test/Driver/pch-codegen.cpp
@@ -7,21 +7,21 @@
 // CHECK-PCH-CREATE-NOT: -fmodules-codegen
 // CHECK-PCH-CREATE-NOT: -fmodules-debuginfo
 
-// Create PCH with -fmodules-codegen.
-// RUN: %clang -x c++-header -Xclang -fmodules-codegen %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-cg.pch -### 2>&1 | FileCheck %s -check-prefix=CHECK-PCH-CODEGEN-CREATE
+// Create PCH with -fpch-codegen.
+// RUN: %clang -x c++-header -fpch-codegen %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-cg.pch -### 2>&1 | FileCheck %s -check-prefix=CHECK-PCH-CODEGEN-CREATE
 // CHECK-PCH-CODEGEN-CREATE: -emit-pch
 // CHECK-PCH-CODEGEN-CREATE: -fmodules-codegen
 // CHECK-PCH-CODEGEN-CREATE: "-x" "c++-header"
 // CHECK-PCH-CODEGEN-CREATE-NOT: -fmodules-debuginfo
 
-// Create PCH with -fmodules-debuginfo.
-// RUN: %clang -x c++-header -Xclang -fmodules-debuginfo %S/../Modules/Inputs/codegen-flags/foo.h -g -o %t/foo-di.pch -### 2>&1 | FileCheck %s -check-prefix=CHECK-PCH-DEBUGINFO-CREATE
+// Create PCH with -fpch-debuginfo.
+// RUN: %clang -x c++-header -fpch-debuginfo %S/../Modules/Inputs/codegen-flags/foo.h -g -o %t/foo-di.pch -### 2>&1 | FileCheck %s -check-prefix=CHECK-PCH-DEBUGINFO-CREATE
 // CHECK-PCH-DEBUGINFO-CREATE: -emit-pch
 // CHECK-PCH-DEBUGINFO-CREATE: -fmodules-debuginfo
 // CHECK-PCH-DEBUGINFO-CREATE: "-x" "c++-header"
 // CHECK-PCH-DEBUGINFO-CREATE-NOT: -fmodules-codegen
 
-// Create PCH's object file for -fmodules-codegen.
+// Create PCH's object file for -fpch-codegen.
 // RUN: touch %t/foo-cg.pch
 // RUN: %clang -c %t/foo-cg.pch -o %t/foo-cg.o -### 2>&1 | FileCheck %s -check-prefix=CHECK-PCH-CODEGEN-OBJ
 // CHECK-PCH-CODEGEN-OBJ: -emit-obj
@@ -29,7 +29,7 @@
 // CHECK-PCH-CODEGEN-OBJ: "-o" "{{.*}}foo-cg.o"
 // CHECK-PCH-CODEGEN-OBJ: "-x" "precompiled-header"
 
-// Create PCH's object file for -fmodules-debuginfo.
+// Create PCH's object file for -fpch-debuginfo.
 // RUN: touch %t/foo-di.pch
 // RUN: %clang -c %t/foo-di.pch -g -o %t/foo-di.o -### 2>&1 | FileCheck %s -check-prefix=CHECK-PCH-DEBUGINFO-OBJ
 // CHECK-PCH-DEBUGINFO-OBJ: -emit-obj
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5642,6 +5642,12 @@
   if (Args.hasFlag(options::OPT_fpch_instantiate_templates,
options::OPT_fno_pch_instantiate_templates, false))
 CmdArgs.push_back("-fpch-instantiate-templates");
+  if (Args.hasFlag(options::OPT_fpch_codegen, options::OPT_fno_pch_codegen,
+   false))
+CmdArgs.push_back("-fmodules-codegen");
+  if (Args.hasFlag(options::OPT_fpch_debuginfo, options::OPT_fno_pch_debuginfo,
+   false))
+CmdArgs.push_back("-fmodules-debuginfo");
 
   Args.AddLastArg(CmdArgs, options::OPT_fexperimental_new_pass_manager,
   options::OPT_fno_experimental_new_pass_manager);
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1440,6 +1440,10 @@
 def fno_pch_instantiate_templates:
   Flag <["-"], "fno-pch-instantiate-templates">,
   Group, Flags<[CC1Option]>;
+defm pch_codegen: OptInFFlag<"pch-codegen", "Generate ", "Do not generate ",
+  "code for uses of this PCH that assumes an explicit object file will be built for the PCH">;
+defm pch_debuginfo: OptInFFlag<"pch-debuginfo", "Generate ", "Do not generate ",
+  "debug info for types in an object file built from this PCH and do not generate them elsewhere">;
 
 def fmodules : Flag <["-"], "fmodules">, Group,
   Flags<[DriverOption, CC1Option]>,
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -63,6 +63,32 @@
 
 - ...
 
+- -fpch-codegen and -fpch-debuginfo generate shared code and/or debuginfo
+  for contents of a precompiled header in a separate object file. This object
+  file needs to be linked in, but its contents do not need to be generated
+  for other objects using the precompiled header. This should us

[PATCH] D83716: Accept 'clang++ -c a.pch -o a.o' to create PCH's object file

[Luboš Luňák via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3895466e2c33: accept 'clang++ -c a.pch -o a.o' to 
create PCH's object file (authored by llunak).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83716

Files:
  clang/lib/Driver/Types.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Driver/pch-codegen.cpp
  clang/test/PCH/codegen.cpp

Index: clang/test/PCH/codegen.cpp
===
--- clang/test/PCH/codegen.cpp
+++ clang/test/PCH/codegen.cpp
@@ -9,8 +9,8 @@
 // RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules-codegen -x c++-header -building-pch-with-obj -emit-pch %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-cg.pch
 // RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules-debuginfo -x c++-header -building-pch-with-obj -emit-pch %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-di.pch
 
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - %s -include-pch %t/foo-cg.pch -building-pch-with-obj -fmodules-codegen | FileCheck --check-prefix=CG %s
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - %s -include-pch %t/foo-di.pch -building-pch-with-obj -fmodules-debuginfo | FileCheck --check-prefix=DI %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -x precompiled-header %t/foo-cg.pch | FileCheck --check-prefix=CG %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -x precompiled-header %t/foo-di.pch | FileCheck --check-prefix=DI %s
 
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -include-pch %t/foo-cg.pch %S/../Modules/Inputs/codegen-flags/use.cpp | FileCheck --check-prefix=CG-USE %s
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -include-pch %t/foo-di.pch %S/../Modules/Inputs/codegen-flags/use.cpp | FileCheck --check-prefix=DI-USE %s
@@ -20,8 +20,8 @@
 // RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules-codegen -x c++-header -building-pch-with-obj -emit-pch -fpch-instantiate-templates %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-cg.pch
 // RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules-debuginfo -x c++-header -building-pch-with-obj -emit-pch -fpch-instantiate-templates %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-di.pch
 
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - %s -include-pch %t/foo-cg.pch -building-pch-with-obj -fmodules-codegen | FileCheck --check-prefix=CG %s
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - %s -include-pch %t/foo-di.pch -building-pch-with-obj -fmodules-debuginfo | FileCheck --check-prefix=DI %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -x precompiled-header %t/foo-cg.pch | FileCheck --check-prefix=CG %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -x precompiled-header %t/foo-di.pch | FileCheck --check-prefix=DI %s
 
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -include-pch %t/foo-cg.pch %S/../Modules/Inputs/codegen-flags/use.cpp | FileCheck --check-prefix=CG-USE %s
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -include-pch %t/foo-di.pch %S/../Modules/Inputs/codegen-flags/use.cpp | FileCheck --check-prefix=DI-USE %s
Index: clang/test/Driver/pch-codegen.cpp
===
--- /dev/null
+++ clang/test/Driver/pch-codegen.cpp
@@ -0,0 +1,38 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+
+// Create PCH without codegen.
+// RUN: %clang -x c++-header %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-cg.pch -### 2>&1 | FileCheck %s -check-prefix=CHECK-PCH-CREATE
+// CHECK-PCH-CREATE: -emit-pch
+// CHECK-PCH-CREATE-NOT: -fmodules-codegen
+// CHECK-PCH-CREATE-NOT: -fmodules-debuginfo
+
+// Create PCH with -fmodules-codegen.
+// RUN: %clang -x c++-header -Xclang -fmodules-codegen %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-cg.pch -### 2>&1 | FileCheck %s -check-prefix=CHECK-PCH-CODEGEN-CREATE
+// CHECK-PCH-CODEGEN-CREATE: -emit-pch
+// CHECK-PCH-CODEGEN-CREATE: -fmodules-codegen
+// CHECK-PCH-CODEGEN-CREATE: "-x" "c++-header"
+// CHECK-PCH-CODEGEN-CREATE-NOT: -fmodules-debuginfo
+
+// Create PCH with -fmodules-debuginfo.
+// RUN: %clang -x c++-header -Xclang -fmodules-debuginfo %S/../Modules/Inputs/codegen-flags/foo.h -g -o %t/foo-di.pch -### 2>&1 | FileCheck %s -check-prefix=CHECK-PCH-DEBUGINFO-CREATE
+// CHECK-PCH-DEBUGINFO-CREATE: -emit-pch
+// CHECK-PCH-DEBUGINFO-CREATE: -fmodules-debuginfo
+// CHECK-PCH-DEBUGINFO-CREATE: "-x" "c++-header"
+// CHECK-PCH-DEBUGINFO-CREATE-NOT: -fmodules-codegen
+
+//

[Differential] D83716: Accept 'clang++ -c a.pch -o a.o' to create PCH's object file

[Luboš Luňák via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3895466e2c33: accept 'clang++ -c a.pch -o a.o' to 
create PCH's object file (authored by llunak).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D83716?vs=277670&id=279073#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83716

Files:
  clang/lib/Driver/Types.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Driver/pch-codegen.cpp
  clang/test/PCH/codegen.cpp

Index: clang/test/PCH/codegen.cpp
===
--- clang/test/PCH/codegen.cpp
+++ clang/test/PCH/codegen.cpp
@@ -9,8 +9,8 @@
 // RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules-codegen -x c++-header -building-pch-with-obj -emit-pch %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-cg.pch
 // RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules-debuginfo -x c++-header -building-pch-with-obj -emit-pch %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-di.pch
 
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - %s -include-pch %t/foo-cg.pch -building-pch-with-obj -fmodules-codegen | FileCheck --check-prefix=CG %s
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - %s -include-pch %t/foo-di.pch -building-pch-with-obj -fmodules-debuginfo | FileCheck --check-prefix=DI %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -x precompiled-header %t/foo-cg.pch | FileCheck --check-prefix=CG %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -x precompiled-header %t/foo-di.pch | FileCheck --check-prefix=DI %s
 
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -include-pch %t/foo-cg.pch %S/../Modules/Inputs/codegen-flags/use.cpp | FileCheck --check-prefix=CG-USE %s
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -include-pch %t/foo-di.pch %S/../Modules/Inputs/codegen-flags/use.cpp | FileCheck --check-prefix=DI-USE %s
@@ -20,8 +20,8 @@
 // RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules-codegen -x c++-header -building-pch-with-obj -emit-pch -fpch-instantiate-templates %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-cg.pch
 // RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules-debuginfo -x c++-header -building-pch-with-obj -emit-pch -fpch-instantiate-templates %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-di.pch
 
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - %s -include-pch %t/foo-cg.pch -building-pch-with-obj -fmodules-codegen | FileCheck --check-prefix=CG %s
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - %s -include-pch %t/foo-di.pch -building-pch-with-obj -fmodules-debuginfo | FileCheck --check-prefix=DI %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -x precompiled-header %t/foo-cg.pch | FileCheck --check-prefix=CG %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -x precompiled-header %t/foo-di.pch | FileCheck --check-prefix=DI %s
 
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -include-pch %t/foo-cg.pch %S/../Modules/Inputs/codegen-flags/use.cpp | FileCheck --check-prefix=CG-USE %s
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -include-pch %t/foo-di.pch %S/../Modules/Inputs/codegen-flags/use.cpp | FileCheck --check-prefix=DI-USE %s
Index: clang/test/Driver/pch-codegen.cpp
===
--- /dev/null
+++ clang/test/Driver/pch-codegen.cpp
@@ -0,0 +1,38 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+
+// Create PCH without codegen.
+// RUN: %clang -x c++-header %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-cg.pch -### 2>&1 | FileCheck %s -check-prefix=CHECK-PCH-CREATE
+// CHECK-PCH-CREATE: -emit-pch
+// CHECK-PCH-CREATE-NOT: -fmodules-codegen
+// CHECK-PCH-CREATE-NOT: -fmodules-debuginfo
+
+// Create PCH with -fmodules-codegen.
+// RUN: %clang -x c++-header -Xclang -fmodules-codegen %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-cg.pch -### 2>&1 | FileCheck %s -check-prefix=CHECK-PCH-CODEGEN-CREATE
+// CHECK-PCH-CODEGEN-CREATE: -emit-pch
+// CHECK-PCH-CODEGEN-CREATE: -fmodules-codegen
+// CHECK-PCH-CODEGEN-CREATE: "-x" "c++-header"
+// CHECK-PCH-CODEGEN-CREATE-NOT: -fmodules-debuginfo
+
+// Create PCH with -fmodules-debuginfo.
+// RUN: %clang -x c++-header -Xclang -fmodules-debuginfo %S/../Modules/Inputs/codegen-flags/foo.h -g -o %t/foo-di.pch -### 2>&1 | FileCheck %s -check-prefix=CHECK-PCH-DEBUGINFO-CREATE
+// CHECK-PCH-DEBUGINFO-CREATE: -emit-pch
+// CHECK-PCH-DEBUGINFO-CREATE: -fmodules-debuginfo
+// CHECK-PCH-DEBUGINFO-C

[PATCH] D69778: Make -fmodules-codegen and -fmodules-debuginfo work also with precompiled headers

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Ping ...


Repository:
  rC Clang

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

https://reviews.llvm.org/D69778



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

[PATCH] D69778: Make -fmodules-codegen and -fmodules-debuginfo work also with precompiled headers

[Luboš Luňák via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG31b05692cd33: make -fmodules-codegen and -fmodules-debuginfo 
work also with PCHs (authored by llunak).

Changed prior to commit:
  https://reviews.llvm.org/D69778?vs=263839&id=276726#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69778

Files:
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/Modules/Inputs/codegen-flags/foo.h
  clang/test/PCH/codegen.cpp

Index: clang/test/PCH/codegen.cpp
===
--- /dev/null
+++ clang/test/PCH/codegen.cpp
@@ -0,0 +1,42 @@
+// This test is the PCH version of Modules/codegen-flags.test . It uses its inputs.
+// The purpose of this test is just verify that the codegen options work with PCH as well.
+// All the codegen functionality should be tested in Modules/.
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// REQUIRES: x86-registered-target
+
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules-codegen -x c++-header -building-pch-with-obj -emit-pch %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-cg.pch
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules-debuginfo -x c++-header -building-pch-with-obj -emit-pch %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-di.pch
+
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - %s -include-pch %t/foo-cg.pch -building-pch-with-obj -fmodules-codegen | FileCheck --check-prefix=CG %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - %s -include-pch %t/foo-di.pch -building-pch-with-obj -fmodules-debuginfo | FileCheck --check-prefix=DI %s
+
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -include-pch %t/foo-cg.pch %S/../Modules/Inputs/codegen-flags/use.cpp | FileCheck --check-prefix=CG-USE %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -include-pch %t/foo-di.pch %S/../Modules/Inputs/codegen-flags/use.cpp | FileCheck --check-prefix=DI-USE %s
+
+// Test with template instantiation in the pch.
+
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules-codegen -x c++-header -building-pch-with-obj -emit-pch -fpch-instantiate-templates %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-cg.pch
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules-debuginfo -x c++-header -building-pch-with-obj -emit-pch -fpch-instantiate-templates %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-di.pch
+
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - %s -include-pch %t/foo-cg.pch -building-pch-with-obj -fmodules-codegen | FileCheck --check-prefix=CG %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - %s -include-pch %t/foo-di.pch -building-pch-with-obj -fmodules-debuginfo | FileCheck --check-prefix=DI %s
+
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -include-pch %t/foo-cg.pch %S/../Modules/Inputs/codegen-flags/use.cpp | FileCheck --check-prefix=CG-USE %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -include-pch %t/foo-di.pch %S/../Modules/Inputs/codegen-flags/use.cpp | FileCheck --check-prefix=DI-USE %s
+
+
+// CG: define weak_odr void @_Z2f1v
+// CG: DICompileUnit
+// CG-NOT: DICompositeType
+
+// CG-USE: declare void @_Z2f1v
+// CG-USE: DICompileUnit
+// CG-USE: distinct !DICompositeType(tag: DW_TAG_structure_type, name: "foo"
+
+// DI-NOT: define
+// DI: distinct !DICompositeType(tag: DW_TAG_structure_type, name: "foo"
+
+// DI-USE: define linkonce_odr void @_Z2f1v
+// DI-USE: = !DICompositeType(tag: DW_TAG_structure_type, name: "foo", {{.*}}, flags: DIFlagFwdDecl
Index: clang/test/Modules/Inputs/codegen-flags/foo.h
===
--- clang/test/Modules/Inputs/codegen-flags/foo.h
+++ clang/test/Modules/Inputs/codegen-flags/foo.h
@@ -1,4 +1,7 @@
+#ifndef FOO_H
+#define FOO_H
 struct foo {
 };
 inline void f1() {
 }
+#endif
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -2458,9 +2458,10 @@
 
   assert(FD->doesThisDeclarationHaveABody());
   bool ModulesCodegen = false;
-  if (Writer->WritingModule && !FD->isDependentContext()) {
+  if (!FD->isDependentContext()) {
 Optional Linkage;
-if (Writer->WritingModule->Kind == Module::ModuleInterfaceUnit) {
+if (Writer->WritingModule &&
+Writer->WritingModule->Kind == Module::ModuleInterfaceUnit) {
   // When building a C++ Modules TS module interface unit, a strong
   // definition in the module interf

[PATCH] D83652: Merge some of the PCH object support with modular codegen

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

The patch is incomplete, isn't it? It removes DeclIsFromPCHWithObjectFile(), 
but it's still called from ASTContext::DeclMustBeEmitted(). The description 
also mentions updating of the pch-codegen test, but that's not included.

But assuming this is intended to replace the D48426 
 functionality with modular codegen, I 
mentioned already in D69778  (starting with 
'Mar 17 2020, 10:59 PM') that it's a question if this is possible. Modular 
codegen is a much bigger hammer than  D48426  
and it has also bigger possible problems: Possibly making compilation actually 
slower (even f63556d8b44b209e741833b0e6be3f8e72a1d91a mentions this), and 
possibly causing build problems because of referencing private symbols.  D48426 
 is much smaller both in gains and in problems 
(=safer).

> Do either of you know if it'd be practical to move to something more similar 
> to .pcm handling, where the pch itself is passed to the compilation, rather 
> than homed as a side effect of compiling some other source file?

Do you mean dropping compatibility with 'cl.exe /Yc'?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83652



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

[PATCH] D83622: document -fpch-instantiate-templates in release notes

[Luboš Luňák via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd1ca9960bc19: document -fpch-instantiate-templates in 
release notes (authored by llunak).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83622

Files:
  clang/docs/ReleaseNotes.rst


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -108,6 +108,13 @@
   By default, this is ~/.cache but on some platforms or installations, this
   might be elsewhere. The -fmodules-cache-path=... flag continues to work.
 
+- -fpch-instantiate-templates tries to instantiate templates already while
+  generating a precompiled header. Such templates do not need to be
+  instantiated every time the precompiled header is used, which saves compile
+  time. This may result in an error during the precompiled header generation
+  if the source header file is not self-contained. This option is enabled
+  by default for clang-cl.
+
 Deprecated Compiler Flags
 -
 


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -108,6 +108,13 @@
   By default, this is ~/.cache but on some platforms or installations, this
   might be elsewhere. The -fmodules-cache-path=... flag continues to work.
 
+- -fpch-instantiate-templates tries to instantiate templates already while
+  generating a precompiled header. Such templates do not need to be
+  instantiated every time the precompiled header is used, which saves compile
+  time. This may result in an error during the precompiled header generation
+  if the source header file is not self-contained. This option is enabled
+  by default for clang-cl.
+
 Deprecated Compiler Flags
 -
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D83652: Merge some of the PCH object support with modular codegen

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

In D83652#2147539 , @dblaikie wrote:

> Not quite - it's intended to implement the D48426 
>  functionality using an implementation 
> strategy that is closer to modular code generation. Removing the need for the 
> module flag, using the MODULAR_CODEGEN_DECLS list, etc. But only putting the 
> dllexported entities in there (when using only building-pch-with-obj without 
> -fmodules-codegen).


I see. I still suggest testing this on a real codebase for the reasons outlined 
above.

> Yep, that was essentially what I was wondering - since you were proposing a 
> build mode/flags that would not be /Yc compatible, adding an explicit pch 
> build and pch object build step - was wondering if the build system support 
> for that was sufficiently available that it would be practical for users 
> (like Chromium) to migrate to that kind of model and no longer need /Yc 
> compatibility.

Not that I know for sure, but I personally doubt there's build system support 
for D83716 . The only build system I've tried 
is LibreOffice's custom gbuild, where I've added the support for 
-fpch-codegen/debuginfo myself, and even there's I'm going to stick with 
-building-pch-with-obj, since it's simpler to compile yet another .cxx file 
with an extra flag rather than have a build step with .pch as yet another kind 
of input file. But /Yc is presumably supported by pretty much anything on 
Windows.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83652



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

[PATCH] D69778: Make -fmodules-codegen and -fmodules-debuginfo work also with precompiled headers

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

So what more needs to be done here to get this change accepted? It's already 
missed 10.0 and 11.1 is getting branched off in about 3 weeks.

- The change was already accepted once and the problem leading to its revert 
has already been fixed.
- It just enables for PCH something that's been enabled for modules for a long 
time (so whatever may be wrong with it will probably be wrong with modules as 
well).
- I've been using the feature for 8 months.
- There's no publicly reported problem with it (I know aganea reported them 
above, but they are private, so nobody can do anything about them).
- The feature is opt-in and disabled by default.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69778



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

[PATCH] D69585: Add option to instantiate templates already in the PCH

[Luboš Luňák via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa45f713c6730: add option to instantiate templates already in 
the PCH (authored by llunak).

Changed prior to commit:
  https://reviews.llvm.org/D69585?vs=265918&id=272312#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69585

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/PCH/crash-12631281.cpp
  clang/test/PCH/cxx-alias-decl.cpp
  clang/test/PCH/cxx-dependent-sized-ext-vector.cpp
  clang/test/PCH/cxx-explicit-specifier.cpp
  clang/test/PCH/cxx-exprs.cpp
  clang/test/PCH/cxx-friends.cpp
  clang/test/PCH/cxx-member-init.cpp
  clang/test/PCH/cxx-ms-function-specialization-class-scope.cpp
  clang/test/PCH/cxx-static_assert.cpp
  clang/test/PCH/cxx-templates.cpp
  clang/test/PCH/cxx-variadic-templates-with-default-params.cpp
  clang/test/PCH/cxx-variadic-templates.cpp
  clang/test/PCH/cxx0x-default-delete.cpp
  clang/test/PCH/cxx11-constexpr.cpp
  clang/test/PCH/cxx11-enum-template.cpp
  clang/test/PCH/cxx11-exception-spec.cpp
  clang/test/PCH/cxx11-inheriting-ctors.cpp
  clang/test/PCH/cxx11-user-defined-literals.cpp
  clang/test/PCH/cxx1y-decltype-auto.cpp
  clang/test/PCH/cxx1y-deduced-return-type.cpp
  clang/test/PCH/cxx1y-default-initializer.cpp
  clang/test/PCH/cxx1y-init-captures.cpp
  clang/test/PCH/cxx1y-variable-templates.cpp
  clang/test/PCH/cxx1z-aligned-alloc.cpp
  clang/test/PCH/cxx1z-decomposition.cpp
  clang/test/PCH/cxx1z-using-declaration.cpp
  clang/test/PCH/cxx2a-bitfield-init.cpp
  clang/test/PCH/cxx2a-concept-specialization-expr.cpp
  clang/test/PCH/cxx2a-constraints.cpp
  clang/test/PCH/cxx2a-defaulted-comparison.cpp
  clang/test/PCH/cxx2a-requires-expr.cpp
  clang/test/PCH/cxx2a-template-lambdas.cpp
  clang/test/PCH/delayed-pch-instantiate.cpp
  clang/test/PCH/friend-template.cpp
  clang/test/PCH/implicitly-deleted.cpp
  clang/test/PCH/late-parsed-instantiations.cpp
  clang/test/PCH/local_static.cpp
  clang/test/PCH/macro-undef.cpp
  clang/test/PCH/make-integer-seq.cpp
  clang/test/PCH/ms-if-exists.cpp
  clang/test/PCH/pch-instantiate-templates-forward-decl.cpp
  clang/test/PCH/pch-instantiate-templates.cpp
  clang/test/PCH/pr18806.cpp
  clang/test/PCH/pragma-diag-section.cpp
  clang/test/PCH/rdar10830559.cpp
  clang/test/PCH/specialization-after-instantiation.cpp
  clang/test/PCH/type_pack_element.cpp

Index: clang/test/PCH/type_pack_element.cpp
===
--- clang/test/PCH/type_pack_element.cpp
+++ clang/test/PCH/type_pack_element.cpp
@@ -1,6 +1,9 @@
 // RUN: %clang_cc1 -std=c++14 -x c++-header %s -emit-pch -o %t.pch
 // RUN: %clang_cc1 -std=c++14 -x c++ /dev/null -include-pch %t.pch
 
+// RUN: %clang_cc1 -std=c++14 -x c++-header %s -emit-pch -fpch-instantiate-templates -o %t.pch
+// RUN: %clang_cc1 -std=c++14 -x c++ /dev/null -include-pch %t.pch
+
 template 
 struct X { };
 
Index: clang/test/PCH/specialization-after-instantiation.cpp
===
--- /dev/null
+++ clang/test/PCH/specialization-after-instantiation.cpp
@@ -0,0 +1,32 @@
+// Test this without pch.
+// RUN: %clang_cc1 -fsyntax-only -verify -DBODY %s
+
+// Test with pch.
+// RUN: %clang_cc1 -emit-pch -o %t %s
+// RUN: %clang_cc1 -include-pch %t -fsyntax-only -verify -DBODY %s
+
+// RUN: %clang_cc1 -emit-pch -fpch-instantiate-templates -o %t %s
+// RUN: %clang_cc1 -include-pch %t -fsyntax-only -verify -DBODY %s
+
+#ifndef HEADER_H
+#define HEADER_H
+
+template 
+struct A {
+  int foo() const;
+};
+
+int bar(A *a) {
+  return a->foo();
+}
+
+#endif // HEADER_H
+
+#ifdef BODY
+
+template <>
+int A::foo() const { // expected-error {{explicit specialization of 'foo' after instantiation}}  // expected-note@20 {{implicit instantiation first required here}}
+  return 10;
+}
+
+#endif // BODY
Index: clang/test/PCH/rdar10830559.cpp
===
--- clang/test/PCH/rdar10830559.cpp
+++ clang/test/PCH/rdar10830559.cpp
@@ -6,6 +6,9 @@
 // RUN: %clang_cc1 -emit-pch -o %t %s
 // RUN: %clang_cc1 -include-pch %t -emit-llvm-only %t.empty.cpp 
 
+// RUN: %clang_cc1 -emit-pch -fpch-instantiate-templates -o %t %s
+// RUN: %clang_cc1 -include-pch %t -emit-llvm-only %t.empty.cpp
+
 // rdar://10830559
 
 //#pragma ms_struct on
Index: clang/test/PCH/pragma-diag-section.cpp
===
--- clang/test/PCH/pragma-diag-section.cpp
+++ clang/test/PCH/pragma-diag-section.cpp
@@ -5,6 +5,9 @@
 // RUN: %clang_cc1 %s -emit-pch -o %t
 // RUN: %clang_cc1 %s -include-pch %t -verify -fsyntax-only -Wuninitialized
 
+//

[PATCH] D69778: Make -fmodules-codegen and -fmodules-debuginfo work also with precompiled headers

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Ping.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69778



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

[PATCH] D74846: fix -fcodegen-modules code when used with PCH (PR44953)

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Ping...


Repository:
  rC Clang

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

https://reviews.llvm.org/D74846



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

[PATCH] D73846: [PCH] make sure to not warn about unused macros from -D

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Ping..


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

https://reviews.llvm.org/D73846



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

[PATCH] D69585: Add option to instantiate templates already in the PCH

[Luboš Luňák via Phabricator via cfe-commits]

llunak updated this revision to Diff 258605.
llunak retitled this revision from "PerformPendingInstatiations() already in 
the PCH" to "Add option to instantiate templates already in the PCH".
llunak edited the summary of this revision.
llunak added a comment.

Changed to use -fpch-instantiate-templates to control the feature.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69585

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/PCH/codegen.cpp
  clang/test/PCH/crash-12631281.cpp
  clang/test/PCH/cxx-alias-decl.cpp
  clang/test/PCH/cxx-dependent-sized-ext-vector.cpp
  clang/test/PCH/cxx-explicit-specifier.cpp
  clang/test/PCH/cxx-exprs.cpp
  clang/test/PCH/cxx-friends.cpp
  clang/test/PCH/cxx-member-init.cpp
  clang/test/PCH/cxx-ms-function-specialization-class-scope.cpp
  clang/test/PCH/cxx-static_assert.cpp
  clang/test/PCH/cxx-templates.cpp
  clang/test/PCH/cxx-variadic-templates-with-default-params.cpp
  clang/test/PCH/cxx-variadic-templates.cpp
  clang/test/PCH/cxx0x-default-delete.cpp
  clang/test/PCH/cxx11-constexpr.cpp
  clang/test/PCH/cxx11-enum-template.cpp
  clang/test/PCH/cxx11-exception-spec.cpp
  clang/test/PCH/cxx11-inheriting-ctors.cpp
  clang/test/PCH/cxx11-user-defined-literals.cpp
  clang/test/PCH/cxx1y-decltype-auto.cpp
  clang/test/PCH/cxx1y-deduced-return-type.cpp
  clang/test/PCH/cxx1y-default-initializer.cpp
  clang/test/PCH/cxx1y-init-captures.cpp
  clang/test/PCH/cxx1y-variable-templates.cpp
  clang/test/PCH/cxx1z-aligned-alloc.cpp
  clang/test/PCH/cxx1z-decomposition.cpp
  clang/test/PCH/cxx1z-using-declaration.cpp
  clang/test/PCH/cxx2a-bitfield-init.cpp
  clang/test/PCH/cxx2a-concept-specialization-expr.cpp
  clang/test/PCH/cxx2a-constraints.cpp
  clang/test/PCH/cxx2a-defaulted-comparison.cpp
  clang/test/PCH/cxx2a-requires-expr.cpp
  clang/test/PCH/cxx2a-template-lambdas.cpp
  clang/test/PCH/delayed-pch-instantiate.cpp
  clang/test/PCH/friend-template.cpp
  clang/test/PCH/implicitly-deleted.cpp
  clang/test/PCH/late-parsed-instantiations.cpp
  clang/test/PCH/local_static.cpp
  clang/test/PCH/macro-undef.cpp
  clang/test/PCH/make-integer-seq.cpp
  clang/test/PCH/ms-if-exists.cpp
  clang/test/PCH/pch-instantiate-templates-forward-decl.cpp
  clang/test/PCH/pch-instantiate-templates.cpp
  clang/test/PCH/pr18806.cpp
  clang/test/PCH/pragma-diag-section.cpp
  clang/test/PCH/rdar10830559.cpp
  clang/test/PCH/specialization-after-instantiation.cpp
  clang/test/PCH/type_pack_element.cpp

Index: clang/test/PCH/type_pack_element.cpp
===
--- clang/test/PCH/type_pack_element.cpp
+++ clang/test/PCH/type_pack_element.cpp
@@ -1,6 +1,9 @@
 // RUN: %clang_cc1 -std=c++14 -x c++-header %s -emit-pch -o %t.pch
 // RUN: %clang_cc1 -std=c++14 -x c++ /dev/null -include-pch %t.pch
 
+// RUN: %clang_cc1 -std=c++14 -x c++-header %s -emit-pch -fpch-instantiate-templates -o %t.pch
+// RUN: %clang_cc1 -std=c++14 -x c++ /dev/null -include-pch %t.pch
+
 template 
 struct X { };
 
Index: clang/test/PCH/specialization-after-instantiation.cpp
===
--- /dev/null
+++ clang/test/PCH/specialization-after-instantiation.cpp
@@ -0,0 +1,32 @@
+// Test this without pch.
+// RUN: %clang_cc1 -fsyntax-only -verify -DBODY %s
+
+// Test with pch.
+// RUN: %clang_cc1 -emit-pch -o %t %s
+// RUN: %clang_cc1 -include-pch %t -fsyntax-only -verify -DBODY %s
+
+// RUN: %clang_cc1 -emit-pch -fpch-instantiate-templates -o %t %s
+// RUN: %clang_cc1 -include-pch %t -fsyntax-only -verify -DBODY %s
+
+#ifndef HEADER_H
+#define HEADER_H
+
+template 
+struct A {
+  int foo() const;
+};
+
+int bar(A *a) {
+  return a->foo();
+}
+
+#endif // HEADER_H
+
+#ifdef BODY
+
+template <>
+int A::foo() const { // expected-error {{explicit specialization of 'foo' after instantiation}}  // expected-note@20 {{implicit instantiation first required here}}
+  return 10;
+}
+
+#endif // BODY
Index: clang/test/PCH/rdar10830559.cpp
===
--- clang/test/PCH/rdar10830559.cpp
+++ clang/test/PCH/rdar10830559.cpp
@@ -6,6 +6,9 @@
 // RUN: %clang_cc1 -emit-pch -o %t %s
 // RUN: %clang_cc1 -include-pch %t -emit-llvm-only %t.empty.cpp 
 
+// RUN: %clang_cc1 -emit-pch -fpch-instantiate-templates -o %t %s
+// RUN: %clang_cc1 -include-pch %t -emit-llvm-only %t.empty.cpp
+
 // rdar://10830559
 
 //#pragma ms_struct on
Index: clang/test/PCH/pragma-diag-section.cpp
===
--- clang/test/PCH/pragma-diag-section.cpp
+++ clang/test/PCH/pragma-diag-section.cpp
@@ -5,6 +5,9 @@
 // RUN: %clang_cc1 %s -emit-pch -o %t
 // RUN

[PATCH] D73846: [PCH] make sure to not warn about unused macros from -D

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

In D73846#1991330 , @rsmith wrote:

> Looks OK as a workaround. Do you know why we consider these to be in the main 
> file? If we could fix that in the source manager, that'd seem preferable.


According to my testing, SourceManager::isInMainFile() can handle "" 
locations in 3 ways:

- for macros defined using -D on the command line the control flow returns 
false in the "if (FI.hasLineDirectives())" block
- for built-in macros such as __clang__ the control flow enters the same "if 
(FI.hasLineDirectives())" block, but Entry->IncludeOffset is 0, so the flow 
then reaches the final "return FI.getIncludeLoc().isInvalid();", which returns 
true
- if PCH is used, macros defined using -D on the command line do not even enter 
"if (FI.hasLineDirectives())" and so they end up returning true the same way 
built-in macros do

But I don't understand this enough to know why and what that actually means.

I've also tried a patch that added SourceManager::setPredefinesFileID() and 
moved the check from this patch to SourceManager::isInMainFile(), but then 
tests fail because apparently Preprocessor::setPredefinesFileID() may be called 
multiple times.


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

https://reviews.llvm.org/D73846



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

[PATCH] D69585: Add option to instantiate templates already in the PCH

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Ping.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69585



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

[PATCH] D74846: fix -fcodegen-modules code when used with PCH (PR44953)

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Ping


Repository:
  rC Clang

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

https://reviews.llvm.org/D74846



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

[PATCH] D73846: [PCH] make sure to not warn about unused macros from -D

[Luboš Luňák via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG5c8c9905c249: make sure to not warn about unused macros from 
-D (authored by llunak).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73846

Files:
  clang/lib/Lex/PPDirectives.cpp
  clang/test/PCH/cli-macro.c


Index: clang/test/PCH/cli-macro.c
===
--- /dev/null
+++ clang/test/PCH/cli-macro.c
@@ -0,0 +1,12 @@
+// Test this without pch.
+// RUN: %clang_cc1 -Wunused-macros -Dunused=1 -fsyntax-only -verify %s
+
+// Test with pch.
+// RUN: %clang_cc1 -Wunused-macros -emit-pch -o %t %s
+// RUN: %clang_cc1 -Wunused-macros -Dunused=1 -include-pch %t -fsyntax-only 
-verify %s
+
+// expected-no-diagnostics
+
+// -Dunused=1 is intentionally not set for the pch.
+// There still should be no unused warning for a macro from the command line.
+
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -2802,7 +2802,9 @@
   // warn-because-unused-macro set. If it gets used it will be removed from 
set.
   if (getSourceManager().isInMainFile(MI->getDefinitionLoc()) &&
   !Diags->isIgnored(diag::pp_macro_not_used, MI->getDefinitionLoc()) &&
-  !MacroExpansionInDirectivesOverride) {
+  !MacroExpansionInDirectivesOverride &&
+  getSourceManager().getFileID(MI->getDefinitionLoc()) !=
+  getPredefinesFileID()) {
 MI->setIsWarnIfUnused(true);
 WarnUnusedMacroLocs.insert(MI->getDefinitionLoc());
   }


Index: clang/test/PCH/cli-macro.c
===
--- /dev/null
+++ clang/test/PCH/cli-macro.c
@@ -0,0 +1,12 @@
+// Test this without pch.
+// RUN: %clang_cc1 -Wunused-macros -Dunused=1 -fsyntax-only -verify %s
+
+// Test with pch.
+// RUN: %clang_cc1 -Wunused-macros -emit-pch -o %t %s
+// RUN: %clang_cc1 -Wunused-macros -Dunused=1 -include-pch %t -fsyntax-only -verify %s
+
+// expected-no-diagnostics
+
+// -Dunused=1 is intentionally not set for the pch.
+// There still should be no unused warning for a macro from the command line.
+
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -2802,7 +2802,9 @@
   // warn-because-unused-macro set. If it gets used it will be removed from set.
   if (getSourceManager().isInMainFile(MI->getDefinitionLoc()) &&
   !Diags->isIgnored(diag::pp_macro_not_used, MI->getDefinitionLoc()) &&
-  !MacroExpansionInDirectivesOverride) {
+  !MacroExpansionInDirectivesOverride &&
+  getSourceManager().getFileID(MI->getDefinitionLoc()) !=
+  getPredefinesFileID()) {
 MI->setIsWarnIfUnused(true);
 WarnUnusedMacroLocs.insert(MI->getDefinitionLoc());
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74846: fix -fcodegen-modules code when used with PCH (PR44953)

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

@aganea: This change reverts only a small part of my previous change (see my 
comment here from Feb 23). Hans reverted the original change only until this 
problem gets sorted out (see his comment from Feb 27 right before the revert).


Repository:
  rC Clang

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

https://reviews.llvm.org/D74846



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

[PATCH] D74846: fix -fcodegen-modules code when used with PCH (PR44953)

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

What is the practical difference? Either way the end result will be the same. 
And given that I get to wait ages for reviews of my PCH changes I find it more 
likely to get a review for a small patch rather than a large one.


Repository:
  rC Clang

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

https://reviews.llvm.org/D74846



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

[PATCH] D74846: fix -fcodegen-modules code when used with PCH (PR44953)

[Luboš Luňák via Phabricator via cfe-commits]

llunak abandoned this revision.
llunak added a comment.

This review did make sense when it was created, it's just that it took so long 
to get this processed that it was simpler to revert the original patch.

Anyway, I've merged the change to https://reviews.llvm.org/D69778 .


Repository:
  rC Clang

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

https://reviews.llvm.org/D74846



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

[PATCH] D69778: Make -fmodules-codegen and -fmodules-debuginfo work also with precompiled headers

[Luboš Luňák via Phabricator via cfe-commits]

llunak reopened this revision.
llunak added a comment.
This revision is now accepted and ready to land.

Reopening because of https://reviews.llvm.org/D74846, updated version includes 
the partial revert from there.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69778



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

[PATCH] D69778: Make -fmodules-codegen and -fmodules-debuginfo work also with precompiled headers

[Luboš Luňák via Phabricator via cfe-commits]

llunak updated this revision to Diff 261722.
llunak added a comment.

Reopening because of https://reviews.llvm.org/D74846, updated version includes 
the partial revert from there.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69778

Files:
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/Modules/Inputs/codegen-flags/foo.h
  clang/test/PCH/codegen.cpp

Index: clang/test/PCH/codegen.cpp
===
--- /dev/null
+++ clang/test/PCH/codegen.cpp
@@ -0,0 +1,30 @@
+// This test is the PCH version of Modules/codegen-flags.test . It uses its inputs.
+// The purpose of this test is just verify that the codegen options work with PCH as well.
+// All the codegen functionality should be tested in Modules/.
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// REQUIRES: x86-registered-target
+
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules-codegen -x c++-header -building-pch-with-obj -emit-pch %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-cg.pch
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules-debuginfo -x c++-header -building-pch-with-obj -emit-pch %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-di.pch
+
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - %s -include-pch %t/foo-cg.pch -building-pch-with-obj -fmodules-codegen | FileCheck --check-prefix=CG %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - %s -include-pch %t/foo-di.pch -building-pch-with-obj -fmodules-debuginfo | FileCheck --check-prefix=DI %s
+
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -include-pch %t/foo-cg.pch %S/../Modules/Inputs/codegen-flags/use.cpp | FileCheck --check-prefix=CG-USE %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -include-pch %t/foo-di.pch %S/../Modules/Inputs/codegen-flags/use.cpp | FileCheck --check-prefix=DI-USE %s
+
+// CG: define weak_odr void @_Z2f1v
+// CG: DICompileUnit
+// CG-NOT: DICompositeType
+
+// CG-USE: declare void @_Z2f1v
+// CG-USE: DICompileUnit
+// CG-USE: distinct !DICompositeType(tag: DW_TAG_structure_type, name: "foo"
+
+// DI-NOT: define
+// DI: distinct !DICompositeType(tag: DW_TAG_structure_type, name: "foo"
+
+// DI-USE: define linkonce_odr void @_Z2f1v
+// DI-USE: = !DICompositeType(tag: DW_TAG_structure_type, name: "foo", {{.*}}, flags: DIFlagFwdDecl
Index: clang/test/Modules/Inputs/codegen-flags/foo.h
===
--- clang/test/Modules/Inputs/codegen-flags/foo.h
+++ clang/test/Modules/Inputs/codegen-flags/foo.h
@@ -1,4 +1,7 @@
+#ifndef FOO_H
+#define FOO_H
 struct foo {
 };
 inline void f1() {
 }
+#endif
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -2460,9 +2460,10 @@
 
   assert(FD->doesThisDeclarationHaveABody());
   bool ModulesCodegen = false;
-  if (Writer->WritingModule && !FD->isDependentContext()) {
+  if (!FD->isDependentContext()) {
 Optional Linkage;
-if (Writer->WritingModule->Kind == Module::ModuleInterfaceUnit) {
+if (Writer->WritingModule &&
+Writer->WritingModule->Kind == Module::ModuleInterfaceUnit) {
   // When building a C++ Modules TS module interface unit, a strong
   // definition in the module interface is provided by the compilation of
   // that module interface unit, not by its users. (Inline functions are
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -5631,8 +5631,8 @@
 
   // getODRHash will compute the ODRHash if it has not been previously computed.
   Record->push_back(D->getODRHash());
-  bool ModulesDebugInfo = Writer->Context->getLangOpts().ModulesDebugInfo &&
-  Writer->WritingModule && !D->isDependentType();
+  bool ModulesDebugInfo =
+  Writer->Context->getLangOpts().ModulesDebugInfo && !D->isDependentType();
   Record->push_back(ModulesDebugInfo);
   if (ModulesDebugInfo)
 Writer->ModularCodegenDecls.push_back(Writer->GetDeclRef(D));
Index: clang/lib/Serialization/ASTReaderDecl.cpp
===
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -503,8 +503,12 @@
 }
 
 void ASTDeclReader::ReadFunctionDefinition(FunctionDecl *FD) {
-  if (Record.readInt())
+  if (Record.readInt()) {
 Reader.DefinitionSource[FD] = Loc.F->Kind == ModuleKind::MK_MainFile;
+if (Reader.getContext().getLangOpts().BuildingPCHWith

[PATCH] D69778: Make -fmodules-codegen and -fmodules-debuginfo work also with precompiled headers

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Ping? I've reopened this one as suggested in D74846 
, but apparently it's kept the accepted state, 
so I'm not sure if this needs another approval or what to do here.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69778



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

[PATCH] D69585: Add option to instantiate templates already in the PCH

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Ping.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69585



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

[PATCH] D69778: Make -fmodules-codegen and -fmodules-debuginfo work also with precompiled headers

[Luboš Luňák via Phabricator via cfe-commits]

llunak updated this revision to Diff 263838.
llunak edited the summary of this revision.
llunak added a comment.

Updated commit message.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69778

Files:
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/Modules/Inputs/codegen-flags/foo.h
  clang/test/PCH/codegen.cpp

Index: clang/test/PCH/codegen.cpp
===
--- /dev/null
+++ clang/test/PCH/codegen.cpp
@@ -0,0 +1,30 @@
+// This test is the PCH version of Modules/codegen-flags.test . It uses its inputs.
+// The purpose of this test is just verify that the codegen options work with PCH as well.
+// All the codegen functionality should be tested in Modules/.
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// REQUIRES: x86-registered-target
+
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules-codegen -x c++-header -building-pch-with-obj -emit-pch %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-cg.pch
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules-debuginfo -x c++-header -building-pch-with-obj -emit-pch %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-di.pch
+
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - %s -include-pch %t/foo-cg.pch -building-pch-with-obj -fmodules-codegen | FileCheck --check-prefix=CG %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - %s -include-pch %t/foo-di.pch -building-pch-with-obj -fmodules-debuginfo | FileCheck --check-prefix=DI %s
+
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -include-pch %t/foo-cg.pch %S/../Modules/Inputs/codegen-flags/use.cpp | FileCheck --check-prefix=CG-USE %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -include-pch %t/foo-di.pch %S/../Modules/Inputs/codegen-flags/use.cpp | FileCheck --check-prefix=DI-USE %s
+
+// CG: define weak_odr void @_Z2f1v
+// CG: DICompileUnit
+// CG-NOT: DICompositeType
+
+// CG-USE: declare void @_Z2f1v
+// CG-USE: DICompileUnit
+// CG-USE: distinct !DICompositeType(tag: DW_TAG_structure_type, name: "foo"
+
+// DI-NOT: define
+// DI: distinct !DICompositeType(tag: DW_TAG_structure_type, name: "foo"
+
+// DI-USE: define linkonce_odr void @_Z2f1v
+// DI-USE: = !DICompositeType(tag: DW_TAG_structure_type, name: "foo", {{.*}}, flags: DIFlagFwdDecl
Index: clang/test/Modules/Inputs/codegen-flags/foo.h
===
--- clang/test/Modules/Inputs/codegen-flags/foo.h
+++ clang/test/Modules/Inputs/codegen-flags/foo.h
@@ -1,4 +1,7 @@
+#ifndef FOO_H
+#define FOO_H
 struct foo {
 };
 inline void f1() {
 }
+#endif
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -2460,9 +2460,10 @@
 
   assert(FD->doesThisDeclarationHaveABody());
   bool ModulesCodegen = false;
-  if (Writer->WritingModule && !FD->isDependentContext()) {
+  if (!FD->isDependentContext()) {
 Optional Linkage;
-if (Writer->WritingModule->Kind == Module::ModuleInterfaceUnit) {
+if (Writer->WritingModule &&
+Writer->WritingModule->Kind == Module::ModuleInterfaceUnit) {
   // When building a C++ Modules TS module interface unit, a strong
   // definition in the module interface is provided by the compilation of
   // that module interface unit, not by its users. (Inline functions are
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -5631,8 +5631,8 @@
 
   // getODRHash will compute the ODRHash if it has not been previously computed.
   Record->push_back(D->getODRHash());
-  bool ModulesDebugInfo = Writer->Context->getLangOpts().ModulesDebugInfo &&
-  Writer->WritingModule && !D->isDependentType();
+  bool ModulesDebugInfo =
+  Writer->Context->getLangOpts().ModulesDebugInfo && !D->isDependentType();
   Record->push_back(ModulesDebugInfo);
   if (ModulesDebugInfo)
 Writer->ModularCodegenDecls.push_back(Writer->GetDeclRef(D));
Index: clang/lib/Serialization/ASTReaderDecl.cpp
===
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -503,8 +503,12 @@
 }
 
 void ASTDeclReader::ReadFunctionDefinition(FunctionDecl *FD) {
-  if (Record.readInt())
+  if (Record.readInt()) {
 Reader.DefinitionSource[FD] = Loc.F->Kind == ModuleKind::MK_MainFile;
+if (Reader.getContext().getLangOpts().BuildingPCHWithObjectFile &&
+Reader.DeclIsFromPCH

[PATCH] D69778: Make -fmodules-codegen and -fmodules-debuginfo work also with precompiled headers

[Luboš Luňák via Phabricator via cfe-commits]

llunak updated this revision to Diff 263839.
llunak edited the summary of this revision.

Repository:
  rC Clang

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

https://reviews.llvm.org/D69778

Files:
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/Modules/Inputs/codegen-flags/foo.h
  clang/test/PCH/codegen.cpp

Index: clang/test/PCH/codegen.cpp
===
--- /dev/null
+++ clang/test/PCH/codegen.cpp
@@ -0,0 +1,30 @@
+// This test is the PCH version of Modules/codegen-flags.test . It uses its inputs.
+// The purpose of this test is just verify that the codegen options work with PCH as well.
+// All the codegen functionality should be tested in Modules/.
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// REQUIRES: x86-registered-target
+
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules-codegen -x c++-header -building-pch-with-obj -emit-pch %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-cg.pch
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules-debuginfo -x c++-header -building-pch-with-obj -emit-pch %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-di.pch
+
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - %s -include-pch %t/foo-cg.pch -building-pch-with-obj -fmodules-codegen | FileCheck --check-prefix=CG %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - %s -include-pch %t/foo-di.pch -building-pch-with-obj -fmodules-debuginfo | FileCheck --check-prefix=DI %s
+
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -include-pch %t/foo-cg.pch %S/../Modules/Inputs/codegen-flags/use.cpp | FileCheck --check-prefix=CG-USE %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -include-pch %t/foo-di.pch %S/../Modules/Inputs/codegen-flags/use.cpp | FileCheck --check-prefix=DI-USE %s
+
+// CG: define weak_odr void @_Z2f1v
+// CG: DICompileUnit
+// CG-NOT: DICompositeType
+
+// CG-USE: declare void @_Z2f1v
+// CG-USE: DICompileUnit
+// CG-USE: distinct !DICompositeType(tag: DW_TAG_structure_type, name: "foo"
+
+// DI-NOT: define
+// DI: distinct !DICompositeType(tag: DW_TAG_structure_type, name: "foo"
+
+// DI-USE: define linkonce_odr void @_Z2f1v
+// DI-USE: = !DICompositeType(tag: DW_TAG_structure_type, name: "foo", {{.*}}, flags: DIFlagFwdDecl
Index: clang/test/Modules/Inputs/codegen-flags/foo.h
===
--- clang/test/Modules/Inputs/codegen-flags/foo.h
+++ clang/test/Modules/Inputs/codegen-flags/foo.h
@@ -1,4 +1,7 @@
+#ifndef FOO_H
+#define FOO_H
 struct foo {
 };
 inline void f1() {
 }
+#endif
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -2460,9 +2460,10 @@
 
   assert(FD->doesThisDeclarationHaveABody());
   bool ModulesCodegen = false;
-  if (Writer->WritingModule && !FD->isDependentContext()) {
+  if (!FD->isDependentContext()) {
 Optional Linkage;
-if (Writer->WritingModule->Kind == Module::ModuleInterfaceUnit) {
+if (Writer->WritingModule &&
+Writer->WritingModule->Kind == Module::ModuleInterfaceUnit) {
   // When building a C++ Modules TS module interface unit, a strong
   // definition in the module interface is provided by the compilation of
   // that module interface unit, not by its users. (Inline functions are
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -5631,8 +5631,8 @@
 
   // getODRHash will compute the ODRHash if it has not been previously computed.
   Record->push_back(D->getODRHash());
-  bool ModulesDebugInfo = Writer->Context->getLangOpts().ModulesDebugInfo &&
-  Writer->WritingModule && !D->isDependentType();
+  bool ModulesDebugInfo =
+  Writer->Context->getLangOpts().ModulesDebugInfo && !D->isDependentType();
   Record->push_back(ModulesDebugInfo);
   if (ModulesDebugInfo)
 Writer->ModularCodegenDecls.push_back(Writer->GetDeclRef(D));
Index: clang/lib/Serialization/ASTReaderDecl.cpp
===
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -503,8 +503,12 @@
 }
 
 void ASTDeclReader::ReadFunctionDefinition(FunctionDecl *FD) {
-  if (Record.readInt())
+  if (Record.readInt()) {
 Reader.DefinitionSource[FD] = Loc.F->Kind == ModuleKind::MK_MainFile;
+if (Reader.getContext().getLangOpts().BuildingPCHWithObjectFile &&
+Reader.DeclIsFromPCHWithObjectFile(FD))
+  Reader.DefinitionSource

[PATCH] D69778: Make -fmodules-codegen and -fmodules-debuginfo work also with precompiled headers

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

In D69778#2032363 , @dblaikie wrote:

> So the original commit ( cbc9d22e49b4 
>  ) was 
> reverted at some point, and now you're proposing recommitting it with a 
> slight change?


Yes.

> Could you summarize (in the summary (which should hopefully be included in 
> the commit message eventually)) the commit (include the hash), the revert 
> (including the hash), reasons for revert and what's changed in this patch 
> compared to the original commit that addresses the reasons for reverting?

Done and see D74846  for details and the exact 
change.

(& ideally what extra testing was done on the newer version of the patch to 
ensure the original issue or another like it would now be caught before 
committing)

There's no testing besides checking that PR44953 no longer crashes. As said in 
D74846 , I don't see how to do a test for 
this, and if there's a problem with this patch then the same problem should 
also exist with modules.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69778



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

[PATCH] D69778: Make -fmodules-codegen and -fmodules-debuginfo work also with precompiled headers

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a subscriber: aganea.
llunak added a comment.

In D69778#2035318 , @dblaikie wrote:

> Do you have a sense of the larger testing that PR44953 was reduced from? Have 
> you tried compiling a non-trivial codebase (I assume you might've tested it 
> with Open Office, for instance) - wonder why PR44953 didn't show up there? 
> (perhaps just the construct needed to tickle the issue isn't common/isn't 
> common in Open Office code, etc)


I've been building LibreOffice and some of its 3rd-party dependencies such as 
Skia for months with this patch, without problems. PR44953 seems to be a corner 
case, we do not use D3DX12 headers in LO, and (I presume) __declspec(selectany) 
is a niche feature. And while I do not understand why PR44953 was happening, I 
do understand why my original patch was triggering it, and it will not happen 
with this patch (or, if it will happen, then it'll happen with modules as 
well). And I've tested with the testcase in PR44953.

(It's LibreOffice BTW, OpenOffice is anything but dead.)

> and/or maybe see if the person who filed PR44953 might be able to test this 
> version of the patch on the original codebase that bug was reduced from?

@aganea Have you tried how this version of the patch affects PR44953? If not, 
could you please try?


Repository:
  rC Clang

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

https://reviews.llvm.org/D69778



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

[PATCH] D63508: make -frewrite-includes also rewrite conditions in #if/#elif

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Ping?


Repository:
  rC Clang

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

https://reviews.llvm.org/D63508



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

[PATCH] D65371: do not emit -Wunused-macros warnings in -frewrite-includes mode (PR15614)

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Ping?


Repository:
  rC Clang

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

https://reviews.llvm.org/D65371



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

[PATCH] D65371: do not emit -Wunused-macros warnings in -frewrite-includes mode (PR15614)

[Luboš Luňák via Phabricator via cfe-commits]

llunak updated this revision to Diff 219370.
llunak added a comment.

Added a test.


Repository:
  rC Clang

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

https://reviews.llvm.org/D65371

Files:
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Frontend/rewrite-includes-warnings.c


Index: clang/test/Frontend/rewrite-includes-warnings.c
===
--- clang/test/Frontend/rewrite-includes-warnings.c
+++ clang/test/Frontend/rewrite-includes-warnings.c
@@ -1,4 +1,6 @@
-// RUN: %clang_cc1 -verify -Wall -Wextra -E -frewrite-includes %s
+// RUN: %clang_cc1 -verify -Wall -Wextra -Wunused-macros -E -frewrite-includes 
%s
 // expected-no-diagnostics
 
 #pragma GCC visibility push (default)
+
+#define UNUSED
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -2723,7 +2723,8 @@
   // If we need warning for not using the macro, add its location in the
   // warn-because-unused-macro set. If it gets used it will be removed from 
set.
   if (getSourceManager().isInMainFile(MI->getDefinitionLoc()) &&
-  !Diags->isIgnored(diag::pp_macro_not_used, MI->getDefinitionLoc())) {
+  !Diags->isIgnored(diag::pp_macro_not_used, MI->getDefinitionLoc()) &&
+  !MacroExpansionInDirectivesOverride) {
 MI->setIsWarnIfUnused(true);
 WarnUnusedMacroLocs.insert(MI->getDefinitionLoc());
   }


Index: clang/test/Frontend/rewrite-includes-warnings.c
===
--- clang/test/Frontend/rewrite-includes-warnings.c
+++ clang/test/Frontend/rewrite-includes-warnings.c
@@ -1,4 +1,6 @@
-// RUN: %clang_cc1 -verify -Wall -Wextra -E -frewrite-includes %s
+// RUN: %clang_cc1 -verify -Wall -Wextra -Wunused-macros -E -frewrite-includes %s
 // expected-no-diagnostics
 
 #pragma GCC visibility push (default)
+
+#define UNUSED
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -2723,7 +2723,8 @@
   // If we need warning for not using the macro, add its location in the
   // warn-because-unused-macro set. If it gets used it will be removed from set.
   if (getSourceManager().isInMainFile(MI->getDefinitionLoc()) &&
-  !Diags->isIgnored(diag::pp_macro_not_used, MI->getDefinitionLoc())) {
+  !Diags->isIgnored(diag::pp_macro_not_used, MI->getDefinitionLoc()) &&
+  !MacroExpansionInDirectivesOverride) {
 MI->setIsWarnIfUnused(true);
 WarnUnusedMacroLocs.insert(MI->getDefinitionLoc());
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65371: do not emit -Wunused-macros warnings in -frewrite-includes mode (PR15614)

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

In D65371#1659929 , @dblaikie wrote:

> A test case would be good (in the clang/test directory - probably near/in the 
> other tests for -frewrite-includes)


Done.

> And does the same bug occur for other preprocessor-related warnings? Maybe 
> it's not practical to disable them all this way & there should be a different 
> solution? (or maybe we shouldn't fix these and users can pass -w to disable 
> warnings when using -frewrite-includes?)

I've never seen any other warning from -frewrite-includes besides 
-Wunused-macros. Given that I use and more or less maintain Icecream, which 
uses -frewrite-includes for distributed builds, I'd say any other warnings 
there either don't exist or are very rare (which rather makes sense, given that 
-frewrite-includes only does limited macro expansion when analysing the input 
and that's about it). Given that, I'd prefer not to disable warnings globally - 
they are unlikely to show up, if they do, they can hopefully be handled 
individually, but on the other hand maybe Clang one day gets warnings about 
#include or similar that would be a pity to miss when using -rewrite-includes.


Repository:
  rC Clang

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

https://reviews.llvm.org/D65371



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

[PATCH] D69779: -fmodules-codegen should not emit extern templates

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

In D69779#1801771 , @dblaikie wrote:

> Ah, if I mark the standalone function template 'inline' (the implicit linkage 
> of member functions) then I get the same failure for both. Haven't tested 
> whether the fix is the same fix for both yet.


With my patch there are no errors with your testcase, even with the 'inline' 
added.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69779



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

[PATCH] D69585: PerformPendingInstatiations() already in the PCH

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Ping.


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

https://reviews.llvm.org/D69585



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

[PATCH] D69779: -fmodules-codegen should not emit extern templates

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Do you need some more information about the patch? It'd be nice if this could 
make it into 10.0.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69779



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

[PATCH] D69585: PerformPendingInstatiations() already in the PCH

[Luboš Luňák via Phabricator via cfe-commits]

llunak marked 2 inline comments as done.
llunak added a comment.

In D69585#1818205 , @rnk wrote:

> I'm interested in making clang do this, but I think this needs significantly 
> more work until this is ready to land. It needs in-tree tests.


What tests specifically? I can add one test that just uses the option and then 
checks the PCH already contains what it should, but besides that this would 
need to repeat all PCH tests with this enabled. Should I just do that?

> I assumed we'd want to hook it up to `clang-cl /Yc` and `/Yu`.

If you mean automatically, then probably not. As said in the description, this 
leads to an error if a PCH'd template has a specialization that's not at least 
mentioned in the PCH. That's not a big deal and it's easy to handle, but it's 
still technically a regression. I myself wouldn't mind and would be fine with 
making this the default, but I assume (plural) you wouldn't.




Comment at: clang/include/clang/Basic/LangOptions.def:163
 BENIGN_LANGOPT(CacheGeneratedPCH, 1, 0, "cache generated PCH files in memory")
+BENIGN_LANGOPT(PCHInstantiateTemplates, 1, 0, "performing pending template 
instantiations already while building a pch")
 COMPATIBLE_LANGOPT(ModulesDeclUse, 1, 0, "require declaration of module 
uses")

rnk wrote:
> I think both sides of a PCH user and creator will need to agree on this, so 
> this isn't a "benign" langopt, is it? This is a regular one, and we should 
> diagnose mismatches between the PCH creation and use.
The flag needs to be used only when creating the PCH. Users don't care, they'll 
run the instantiation too, the flag will just make users do less repeated work.





Comment at: clang/lib/Sema/Sema.cpp:985-986
+
+// FIXME: Instantiating implicit templates already in the PCH breaks some
+// OpenMP-specific code paths, see https://reviews.llvm.org/D69585 .
+if(LangOpts.PCHInstantiateTemplates && !LangOpts.OpenMP) {

rnk wrote:
> This really deserves to be debugged before we land it.
I debugged this more than 2 months ago, that's why the OpenMP code owner is 
added as a reviewer.  The initial diff (that I have no idea how to display 
here) included a suggested fix and the description said this was a WIP and 
included my findings about it. As far as I can tell the only effect that had 
was that this patch was sitting here all the time without a single reaction, so 
if nobody OpenMP related cares then I do not either.




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

https://reviews.llvm.org/D69585



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

[PATCH] D69779: -fmodules-codegen should not emit extern templates

[Luboš Luňák via Phabricator via cfe-commits]

llunak updated this revision to Diff 238086.
llunak added a comment.

I've updated the test as requested.

However I've noticed that a PCH-based test for this relies on 
https://reviews.llvm.org/D69585 . The fix works (of course), but it requires a 
PCH/module with the instantiation.


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

https://reviews.llvm.org/D69779

Files:
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/PCH/codegen-extern-template.cpp
  clang/test/PCH/codegen-extern-template.h


Index: clang/test/PCH/codegen-extern-template.h
===
--- /dev/null
+++ clang/test/PCH/codegen-extern-template.h
@@ -0,0 +1,12 @@
+// header for codegen-extern-template.cpp
+#ifndef CODEGEN_EXTERN_TEMPLATE_H
+#define CODEGEN_EXTERN_TEMPLATE_H
+
+template 
+inline T foo() { return 10; }
+
+extern template int foo();
+
+inline int bar() { return foo(); }
+
+#endif
Index: clang/test/PCH/codegen-extern-template.cpp
===
--- /dev/null
+++ clang/test/PCH/codegen-extern-template.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -x c++-header 
-building-pch-with-obj -pch-instantiate-templates -fmodules-codegen -emit-pch 
%S/codegen-extern-template.h -o %t.pch
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -o - %s -include-pch 
%t.pch | FileCheck %s
+// expected-no-diagnostics
+
+#include "codegen-extern-template.h"
+
+template int foo();
+
+// CHECK: define weak_odr i32 @_Z3fooIiET_v
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -2412,11 +2412,12 @@
 }
 if (Writer->Context->getLangOpts().ModulesCodegen) {
   // Under -fmodules-codegen, codegen is performed for all non-internal,
-  // non-always_inline functions.
+  // non-always_inline functions, unless they are available elsewhere.
   if (!FD->hasAttr()) {
 if (!Linkage)
   Linkage = Writer->Context->GetGVALinkageForFunction(FD);
-ModulesCodegen = *Linkage != GVA_Internal;
+ModulesCodegen =
+*Linkage != GVA_Internal && *Linkage != GVA_AvailableExternally;
   }
 }
   }


Index: clang/test/PCH/codegen-extern-template.h
===
--- /dev/null
+++ clang/test/PCH/codegen-extern-template.h
@@ -0,0 +1,12 @@
+// header for codegen-extern-template.cpp
+#ifndef CODEGEN_EXTERN_TEMPLATE_H
+#define CODEGEN_EXTERN_TEMPLATE_H
+
+template 
+inline T foo() { return 10; }
+
+extern template int foo();
+
+inline int bar() { return foo(); }
+
+#endif
Index: clang/test/PCH/codegen-extern-template.cpp
===
--- /dev/null
+++ clang/test/PCH/codegen-extern-template.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -x c++-header -building-pch-with-obj -pch-instantiate-templates -fmodules-codegen -emit-pch %S/codegen-extern-template.h -o %t.pch
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -o - %s -include-pch %t.pch | FileCheck %s
+// expected-no-diagnostics
+
+#include "codegen-extern-template.h"
+
+template int foo();
+
+// CHECK: define weak_odr i32 @_Z3fooIiET_v
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -2412,11 +2412,12 @@
 }
 if (Writer->Context->getLangOpts().ModulesCodegen) {
   // Under -fmodules-codegen, codegen is performed for all non-internal,
-  // non-always_inline functions.
+  // non-always_inline functions, unless they are available elsewhere.
   if (!FD->hasAttr()) {
 if (!Linkage)
   Linkage = Writer->Context->GetGVALinkageForFunction(FD);
-ModulesCodegen = *Linkage != GVA_Internal;
+ModulesCodegen =
+*Linkage != GVA_Internal && *Linkage != GVA_AvailableExternally;
   }
 }
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69779: -fmodules-codegen should not emit extern templates

[Luboš Luňák via Phabricator via cfe-commits]

llunak updated this revision to Diff 238089.
llunak added a comment.

This version uses a module based on the code posted above.


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

https://reviews.llvm.org/D69779

Files:
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/Modules/codegen-extern-template.cpp
  clang/test/Modules/codegen-extern-template.h
  clang/test/Modules/codegen-extern-template.modulemap


Index: clang/test/Modules/codegen-extern-template.modulemap
===
--- /dev/null
+++ clang/test/Modules/codegen-extern-template.modulemap
@@ -0,0 +1 @@
+module foo { header "codegen-extern-template.h" }
Index: clang/test/Modules/codegen-extern-template.h
===
--- /dev/null
+++ clang/test/Modules/codegen-extern-template.h
@@ -0,0 +1,12 @@
+// header for codegen-extern-template.cpp
+#ifndef CODEGEN_EXTERN_TEMPLATE_H
+#define CODEGEN_EXTERN_TEMPLATE_H
+
+template 
+inline T foo() { return 10; }
+
+extern template int foo();
+
+inline int bar() { return foo(); }
+
+#endif
Index: clang/test/Modules/codegen-extern-template.cpp
===
--- /dev/null
+++ clang/test/Modules/codegen-extern-template.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules -fmodules-codegen 
-emit-module -fmodule-name=foo %S/codegen-extern-template.modulemap -x c++ -o 
%t.pcm
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fmodules -fmodule-file=%t.pcm %s 
-emit-llvm -o - | FileCheck %s
+// expected-no-diagnostics
+
+#include "codegen-extern-template.h"
+
+template int foo();
+
+// CHECK: define weak_odr i32 @_Z3fooIiET_v
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -2412,11 +2412,12 @@
 }
 if (Writer->Context->getLangOpts().ModulesCodegen) {
   // Under -fmodules-codegen, codegen is performed for all non-internal,
-  // non-always_inline functions.
+  // non-always_inline functions, unless they are available elsewhere.
   if (!FD->hasAttr()) {
 if (!Linkage)
   Linkage = Writer->Context->GetGVALinkageForFunction(FD);
-ModulesCodegen = *Linkage != GVA_Internal;
+ModulesCodegen =
+*Linkage != GVA_Internal && *Linkage != GVA_AvailableExternally;
   }
 }
   }


Index: clang/test/Modules/codegen-extern-template.modulemap
===
--- /dev/null
+++ clang/test/Modules/codegen-extern-template.modulemap
@@ -0,0 +1 @@
+module foo { header "codegen-extern-template.h" }
Index: clang/test/Modules/codegen-extern-template.h
===
--- /dev/null
+++ clang/test/Modules/codegen-extern-template.h
@@ -0,0 +1,12 @@
+// header for codegen-extern-template.cpp
+#ifndef CODEGEN_EXTERN_TEMPLATE_H
+#define CODEGEN_EXTERN_TEMPLATE_H
+
+template 
+inline T foo() { return 10; }
+
+extern template int foo();
+
+inline int bar() { return foo(); }
+
+#endif
Index: clang/test/Modules/codegen-extern-template.cpp
===
--- /dev/null
+++ clang/test/Modules/codegen-extern-template.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules -fmodules-codegen -emit-module -fmodule-name=foo %S/codegen-extern-template.modulemap -x c++ -o %t.pcm
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fmodules -fmodule-file=%t.pcm %s -emit-llvm -o - | FileCheck %s
+// expected-no-diagnostics
+
+#include "codegen-extern-template.h"
+
+template int foo();
+
+// CHECK: define weak_odr i32 @_Z3fooIiET_v
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -2412,11 +2412,12 @@
 }
 if (Writer->Context->getLangOpts().ModulesCodegen) {
   // Under -fmodules-codegen, codegen is performed for all non-internal,
-  // non-always_inline functions.
+  // non-always_inline functions, unless they are available elsewhere.
   if (!FD->hasAttr()) {
 if (!Linkage)
   Linkage = Writer->Context->GetGVALinkageForFunction(FD);
-ModulesCodegen = *Linkage != GVA_Internal;
+ModulesCodegen =
+*Linkage != GVA_Internal && *Linkage != GVA_AvailableExternally;
   }
 }
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69779: -fmodules-codegen should not emit extern templates

[Luboš Luňák via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG729530f68fe1: -fmodules-codegen should not emit extern 
templates (authored by llunak).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69779

Files:
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/Modules/codegen-extern-template.cpp
  clang/test/Modules/codegen-extern-template.h
  clang/test/Modules/codegen-extern-template.modulemap


Index: clang/test/Modules/codegen-extern-template.modulemap
===
--- /dev/null
+++ clang/test/Modules/codegen-extern-template.modulemap
@@ -0,0 +1 @@
+module foo { header "codegen-extern-template.h" }
Index: clang/test/Modules/codegen-extern-template.h
===
--- /dev/null
+++ clang/test/Modules/codegen-extern-template.h
@@ -0,0 +1,12 @@
+// header for codegen-extern-template.cpp
+#ifndef CODEGEN_EXTERN_TEMPLATE_H
+#define CODEGEN_EXTERN_TEMPLATE_H
+
+template 
+inline T foo() { return 10; }
+
+extern template int foo();
+
+inline int bar() { return foo(); }
+
+#endif
Index: clang/test/Modules/codegen-extern-template.cpp
===
--- /dev/null
+++ clang/test/Modules/codegen-extern-template.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules -fmodules-codegen 
-emit-module -fmodule-name=foo %S/codegen-extern-template.modulemap -x c++ -o 
%t.pcm
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fmodules -fmodule-file=%t.pcm %s 
-emit-llvm -o - | FileCheck %s
+// expected-no-diagnostics
+
+#include "codegen-extern-template.h"
+
+template int foo();
+
+// CHECK: define weak_odr i32 @_Z3fooIiET_v
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -2437,11 +2437,12 @@
 }
 if (Writer->Context->getLangOpts().ModulesCodegen) {
   // Under -fmodules-codegen, codegen is performed for all non-internal,
-  // non-always_inline functions.
+  // non-always_inline functions, unless they are available elsewhere.
   if (!FD->hasAttr()) {
 if (!Linkage)
   Linkage = Writer->Context->GetGVALinkageForFunction(FD);
-ModulesCodegen = *Linkage != GVA_Internal;
+ModulesCodegen =
+*Linkage != GVA_Internal && *Linkage != GVA_AvailableExternally;
   }
 }
   }


Index: clang/test/Modules/codegen-extern-template.modulemap
===
--- /dev/null
+++ clang/test/Modules/codegen-extern-template.modulemap
@@ -0,0 +1 @@
+module foo { header "codegen-extern-template.h" }
Index: clang/test/Modules/codegen-extern-template.h
===
--- /dev/null
+++ clang/test/Modules/codegen-extern-template.h
@@ -0,0 +1,12 @@
+// header for codegen-extern-template.cpp
+#ifndef CODEGEN_EXTERN_TEMPLATE_H
+#define CODEGEN_EXTERN_TEMPLATE_H
+
+template 
+inline T foo() { return 10; }
+
+extern template int foo();
+
+inline int bar() { return foo(); }
+
+#endif
Index: clang/test/Modules/codegen-extern-template.cpp
===
--- /dev/null
+++ clang/test/Modules/codegen-extern-template.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules -fmodules-codegen -emit-module -fmodule-name=foo %S/codegen-extern-template.modulemap -x c++ -o %t.pcm
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fmodules -fmodule-file=%t.pcm %s -emit-llvm -o - | FileCheck %s
+// expected-no-diagnostics
+
+#include "codegen-extern-template.h"
+
+template int foo();
+
+// CHECK: define weak_odr i32 @_Z3fooIiET_v
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -2437,11 +2437,12 @@
 }
 if (Writer->Context->getLangOpts().ModulesCodegen) {
   // Under -fmodules-codegen, codegen is performed for all non-internal,
-  // non-always_inline functions.
+  // non-always_inline functions, unless they are available elsewhere.
   if (!FD->hasAttr()) {
 if (!Linkage)
   Linkage = Writer->Context->GetGVALinkageForFunction(FD);
-ModulesCodegen = *Linkage != GVA_Internal;
+ModulesCodegen =
+*Linkage != GVA_Internal && *Linkage != GVA_AvailableExternally;
   }
 }
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69778: Make -fmodules-codegen and -fmodules-debuginfo work also with precompiled headers

[Luboš Luňák via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGcbc9d22e49b4: make -fmodules-codegen and -fmodules-debuginfo 
work also with PCHs (authored by llunak).

Changed prior to commit:
  https://reviews.llvm.org/D69778?vs=227634&id=238115#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69778

Files:
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/Modules/Inputs/codegen-flags/foo.h
  clang/test/PCH/codegen.cpp

Index: clang/test/PCH/codegen.cpp
===
--- /dev/null
+++ clang/test/PCH/codegen.cpp
@@ -0,0 +1,30 @@
+// This test is the PCH version of Modules/codegen-flags.test . It uses its inputs.
+// The purpose of this test is just verify that the codegen options work with PCH as well.
+// All the codegen functionality should be tested in Modules/.
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// REQUIRES: x86-registered-target
+
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules-codegen -x c++-header -building-pch-with-obj -emit-pch %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-cg.pch
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules-debuginfo -x c++-header -building-pch-with-obj -emit-pch %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-di.pch
+
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - %s -include-pch %t/foo-cg.pch -building-pch-with-obj -fmodules-codegen | FileCheck --check-prefix=CG %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - %s -include-pch %t/foo-di.pch -building-pch-with-obj -fmodules-debuginfo | FileCheck --check-prefix=DI %s
+
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -include-pch %t/foo-cg.pch %S/../Modules/Inputs/codegen-flags/use.cpp | FileCheck --check-prefix=CG-USE %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -include-pch %t/foo-di.pch %S/../Modules/Inputs/codegen-flags/use.cpp | FileCheck --check-prefix=DI-USE %s
+
+// CG: define weak_odr void @_Z2f1v
+// CG: DICompileUnit
+// CG-NOT: DICompositeType
+
+// CG-USE: declare void @_Z2f1v
+// CG-USE: DICompileUnit
+// CG-USE: distinct !DICompositeType(tag: DW_TAG_structure_type, name: "foo"
+
+// DI-NOT: define
+// DI: distinct !DICompositeType(tag: DW_TAG_structure_type, name: "foo"
+
+// DI-USE: define linkonce_odr void @_Z2f1v
+// DI-USE: = !DICompositeType(tag: DW_TAG_structure_type, name: "foo", {{.*}}, flags: DIFlagFwdDecl
Index: clang/test/Modules/Inputs/codegen-flags/foo.h
===
--- clang/test/Modules/Inputs/codegen-flags/foo.h
+++ clang/test/Modules/Inputs/codegen-flags/foo.h
@@ -1,4 +1,7 @@
+#ifndef FOO_H
+#define FOO_H
 struct foo {
 };
 inline void f1() {
 }
+#endif
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -1010,15 +1010,16 @@
 
   if (D->getStorageDuration() == SD_Static) {
 bool ModulesCodegen = false;
-if (Writer.WritingModule &&
-!D->getDescribedVarTemplate() && !D->getMemberSpecializationInfo() &&
+if (!D->getDescribedVarTemplate() && !D->getMemberSpecializationInfo() &&
 !isa(D)) {
   // When building a C++ Modules TS module interface unit, a strong
   // definition in the module interface is provided by the compilation of
   // that module interface unit, not by its users. (Inline variables are
   // still emitted in module users.)
   ModulesCodegen =
-  (Writer.WritingModule->Kind == Module::ModuleInterfaceUnit &&
+  (((Writer.WritingModule &&
+ Writer.WritingModule->Kind == Module::ModuleInterfaceUnit) ||
+Writer.Context->getLangOpts().BuildingPCHWithObjectFile) &&
Writer.Context->GetGVALinkageForVariable(D) == GVA_StrongExternal);
 }
 Record.push_back(ModulesCodegen);
@@ -2425,9 +2426,11 @@
 
   assert(FD->doesThisDeclarationHaveABody());
   bool ModulesCodegen = false;
-  if (Writer->WritingModule && !FD->isDependentContext()) {
+  if (!FD->isDependentContext()) {
 Optional Linkage;
-if (Writer->WritingModule->Kind == Module::ModuleInterfaceUnit) {
+if ((Writer->WritingModule &&
+ Writer->WritingModule->Kind == Module::ModuleInterfaceUnit) ||
+Writer->Context->getLangOpts().BuildingPCHWithObjectFile) {
   // When building a C++ Modules TS module interface unit, a strong
   // definition in the module interface is provided by the compilation of
   // that module interface unit, not by its users. (Inline functions are
Index: clang/lib/Serialization/AS

[PATCH] D69585: PerformPendingInstatiations() already in the PCH

[Luboš Luňák via Phabricator via cfe-commits]

llunak updated this revision to Diff 238217.
llunak edited the summary of this revision.
llunak removed a project: OpenMP.
llunak added a comment.

In order to simplify this, I've updated the patch to remove any reference to 
OpenMP and I've moved that part to https://reviews.llvm.org/D72759 .

This means that the only thing missing here should be adding tests. Which, as 
said above, I'm unsure how to do exactly given that it'd probably need 
duplicating all PCH tests to use this option, so please advise how to do this 
so that the patch can be accepted.

In D69585#1820849 , @ABataev wrote:

> I don't see any crashes in OpenMP tests, other tests just must be updated by 
> the author, if this patch will ever going to be landed. I don't think it is a 
> good idea to have a not fully functional implementation, even guarded by the 
> option.


Please see https://reviews.llvm.org/D72759 .


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

https://reviews.llvm.org/D69585

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/CC1Options.td
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/Sema.cpp


Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -983,6 +983,12 @@
  LateParsedInstantiations.begin(),
  LateParsedInstantiations.end());
 LateParsedInstantiations.clear();
+
+if(LangOpts.PCHInstantiateTemplates) {
+  llvm::TimeTraceScope TimeScope("PerformPendingInstantiations",
+ StringRef(""));
+  PerformPendingInstantiations();
+}
   }
 
   DiagnoseUnterminatedPragmaPack();
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3294,6 +3294,7 @@
 
   Opts.CompleteMemberPointers = Args.hasArg(OPT_fcomplete_member_pointers);
   Opts.BuildingPCHWithObjectFile = Args.hasArg(OPT_building_pch_with_obj);
+  Opts.PCHInstantiateTemplates = Args.hasArg(OPT_pch_instantiate_templates);
 }
 
 static bool isStrictlyPreprocessorAction(frontend::ActionKind Action) {
Index: clang/include/clang/Driver/CC1Options.td
===
--- clang/include/clang/Driver/CC1Options.td
+++ clang/include/clang/Driver/CC1Options.td
@@ -676,6 +676,8 @@
   HelpText<"Disable inclusion of timestamp in precompiled headers">;
 def building_pch_with_obj : Flag<["-"], "building-pch-with-obj">,
   HelpText<"This compilation is part of building a PCH with corresponding 
object file.">;
+def pch_instantiate_templates : Flag<["-"], "pch-instantiate-templates">,
+  HelpText<"Perform pending template instantiations already while building the 
PCH.">;
 
 def aligned_alloc_unavailable : Flag<["-"], "faligned-alloc-unavailable">,
   HelpText<"Aligned allocation/deallocation functions are unavailable">;
Index: clang/include/clang/Basic/LangOptions.def
===
--- clang/include/clang/Basic/LangOptions.def
+++ clang/include/clang/Basic/LangOptions.def
@@ -161,6 +161,7 @@
 BENIGN_LANGOPT(CompilingPCH, 1, 0, "building a pch")
 BENIGN_LANGOPT(BuildingPCHWithObjectFile, 1, 0, "building a pch which has a 
corresponding object file")
 BENIGN_LANGOPT(CacheGeneratedPCH, 1, 0, "cache generated PCH files in memory")
+BENIGN_LANGOPT(PCHInstantiateTemplates, 1, 0, "performing pending template 
instantiations already while building a pch")
 COMPATIBLE_LANGOPT(ModulesDeclUse, 1, 0, "require declaration of module 
uses")
 BENIGN_LANGOPT(ModulesSearchAll  , 1, 1, "searching even non-imported modules 
to find unresolved references")
 COMPATIBLE_LANGOPT(ModulesStrictDeclUse, 1, 0, "requiring declaration of 
module uses and all headers to be in modules")


Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -983,6 +983,12 @@
  LateParsedInstantiations.begin(),
  LateParsedInstantiations.end());
 LateParsedInstantiations.clear();
+
+if(LangOpts.PCHInstantiateTemplates) {
+  llvm::TimeTraceScope TimeScope("PerformPendingInstantiations",
+ StringRef(""));
+  PerformPendingInstantiations();
+}
   }
 
   DiagnoseUnterminatedPragmaPack();
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3294,6 +3294,7 @@
 
   Opts.CompleteMemberPointers = Args.hasArg(OPT_fcomplete_member_pointers);
   Opts.BuildingPCHWithObjectFile = Ar

[PATCH] D88680: Add ability to turn off -fpch-instantiate-templates in clang-cl

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

> I tried pretty hard to get a small repro for these failures, but couldn't.

Can you get at least some testcase, even if not small? You can use -E 
-frewrite-includes to create a single large file from all the input. Although 
the patch looks fine to me as such, I consider it a workaround. The reason for 
-fpch-instantiate-templates being the default for clang-cl is that MSVC does 
some kind of template instantiation for PCHs too (although seeing your email 
you probably know more than I do). So preferably the core problem should be 
fixed.

In D88680#2307564 , @rnk wrote:

> I think the flag was originally intended to be an internal -cc1 flag not 
> exposed to users.

No, it's intentionally exposed in the gcc-compatible driver, because there the 
option is not the default because of corner cases if the PCH is not 
self-contained. But since MSVC requires PCHs to be self-contained, it was 
deemed safe to always enable it in clang-cl. And, as said above, I still think 
it generally should.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88680

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

[PATCH] D88680: Add ability to turn off -fpch-instantiate-templates in clang-cl

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

LGTM, although I'm not sure if me saying that is enough. I'll commit this in 
few days if nobody says anything else.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88680

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

[PATCH] D73846: [PCH] make sure to not warn about unused macros from -D

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Ping.


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

https://reviews.llvm.org/D73846



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

[PATCH] D74846: fix -fcodegen-modules code when used with PCH (PR44953)

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Ping..


Repository:
  rC Clang

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

https://reviews.llvm.org/D74846



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

[PATCH] D69585: PerformPendingInstatiations() already in the PCH

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Ping


Repository:
  rC Clang

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

https://reviews.llvm.org/D69585



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

[PATCH] D69585: PerformPendingInstatiations() already in the PCH

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

In D69585#1942431 , @rsmith wrote:

> This needs to be done behind a flag. It's an explicit design goal that 
> compilation behavior using a PCH or precompiled preamble behaves identically 
> to compilation not using a PCH / precompiled preamble. The behavior of this 
> patch is also non-conforming: we're only allowed to perform function template 
> instantiations at their points of instantiation (essentially, at points of 
> use + at the end of the translation unit).


How is that different from -fmodules? Given that AFAICT this makes PCHs work 
the same way as modules, this should mean -fmodules also both fails the 
identical behaviour goal and is non-conforming.

And to make sure I understand correctly, by behaving identically you mean that 
all the compiler output should be identical without even benign differences?


Repository:
  rC Clang

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

https://reviews.llvm.org/D69585



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

[PATCH] D74846: fix -fcodegen-modules code when used with PCH (PR44958)

[Luboš Luňák via Phabricator via cfe-commits]

llunak created this revision.
llunak added reviewers: hans, dblaikie.
llunak added a project: clang.
Herald added a subscriber: cfe-commits.

In D69778  I incorrectly handled two cases 
(checking for -building-pch-with-obj without also checking -fmodules-codegen).


Repository:
  rC Clang

https://reviews.llvm.org/D74846

Files:
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/PCH/Inputs/declspec-selectany-pch.cpp
  clang/test/PCH/Inputs/declspec-selectany-pch.h
  clang/test/PCH/declspec-selectany.cpp


Index: clang/test/PCH/declspec-selectany.cpp
===
--- /dev/null
+++ clang/test/PCH/declspec-selectany.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cl /Yc%S/Inputs/declspec-selectany-pch.h 
%S/Inputs/declspec-selectany-pch.cpp /c /Fo%t.obj /Fp%t.pch
+// RUN: %clang_cl /Yu%S/Inputs/declspec-selectany-pch.h %s /I%S/Inputs /c 
/Fo%t.obj /Fp%t.pch
+
+#include "declspec-selectany-pch.h"
+
+int main() {
+  CD3DX12_CPU_DESCRIPTOR_HANDLE desc = 
CD3DX12_CPU_DESCRIPTOR_HANDLE(D3D12_DEFAULT);
+  return desc.ptr;
+}
Index: clang/test/PCH/Inputs/declspec-selectany-pch.h
===
--- /dev/null
+++ clang/test/PCH/Inputs/declspec-selectany-pch.h
@@ -0,0 +1,7 @@
+struct CD3DX12_DEFAULT {};
+extern const __declspec(selectany) CD3DX12_DEFAULT D3D12_DEFAULT;
+
+struct CD3DX12_CPU_DESCRIPTOR_HANDLE {
+  CD3DX12_CPU_DESCRIPTOR_HANDLE(CD3DX12_DEFAULT) { ptr = 0; }
+  size_t ptr;
+};
Index: clang/test/PCH/Inputs/declspec-selectany-pch.cpp
===
--- /dev/null
+++ clang/test/PCH/Inputs/declspec-selectany-pch.cpp
@@ -0,0 +1 @@
+#include "declspec-selectany-pch.h"
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -1017,10 +1017,12 @@
   // definition in the module interface is provided by the compilation of
   // that module interface unit, not by its users. (Inline variables are
   // still emitted in module users.)
+  // Similarly if a PCH is built with codegen and its own object file.
   ModulesCodegen =
   (((Writer.WritingModule &&
  Writer.WritingModule->Kind == Module::ModuleInterfaceUnit) ||
-Writer.Context->getLangOpts().BuildingPCHWithObjectFile) &&
+(Writer.Context->getLangOpts().BuildingPCHWithObjectFile &&
+ Writer.Context->getLangOpts().ModulesCodegen)) &&
Writer.Context->GetGVALinkageForVariable(D) == GVA_StrongExternal);
 }
 Record.push_back(ModulesCodegen);
@@ -2451,9 +2453,8 @@
   bool ModulesCodegen = false;
   if (!FD->isDependentContext()) {
 Optional Linkage;
-if ((Writer->WritingModule &&
- Writer->WritingModule->Kind == Module::ModuleInterfaceUnit) ||
-Writer->Context->getLangOpts().BuildingPCHWithObjectFile) {
+if (Writer->WritingModule &&
+Writer->WritingModule->Kind == Module::ModuleInterfaceUnit) {
   // When building a C++ Modules TS module interface unit, a strong
   // definition in the module interface is provided by the compilation of
   // that module interface unit, not by its users. (Inline functions are


Index: clang/test/PCH/declspec-selectany.cpp
===
--- /dev/null
+++ clang/test/PCH/declspec-selectany.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cl /Yc%S/Inputs/declspec-selectany-pch.h %S/Inputs/declspec-selectany-pch.cpp /c /Fo%t.obj /Fp%t.pch
+// RUN: %clang_cl /Yu%S/Inputs/declspec-selectany-pch.h %s /I%S/Inputs /c /Fo%t.obj /Fp%t.pch
+
+#include "declspec-selectany-pch.h"
+
+int main() {
+  CD3DX12_CPU_DESCRIPTOR_HANDLE desc = CD3DX12_CPU_DESCRIPTOR_HANDLE(D3D12_DEFAULT);
+  return desc.ptr;
+}
Index: clang/test/PCH/Inputs/declspec-selectany-pch.h
===
--- /dev/null
+++ clang/test/PCH/Inputs/declspec-selectany-pch.h
@@ -0,0 +1,7 @@
+struct CD3DX12_DEFAULT {};
+extern const __declspec(selectany) CD3DX12_DEFAULT D3D12_DEFAULT;
+
+struct CD3DX12_CPU_DESCRIPTOR_HANDLE {
+  CD3DX12_CPU_DESCRIPTOR_HANDLE(CD3DX12_DEFAULT) { ptr = 0; }
+  size_t ptr;
+};
Index: clang/test/PCH/Inputs/declspec-selectany-pch.cpp
===
--- /dev/null
+++ clang/test/PCH/Inputs/declspec-selectany-pch.cpp
@@ -0,0 +1 @@
+#include "declspec-selectany-pch.h"
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -1017,10 +1017,12 @@
   // definition in the module interface is provided by the compilation of
   // that module interface unit, not by its users. (Inline variables a

[PATCH] D74846: fix -fcodegen-modules code when used with PCH (PR44953)

[Luboš Luňák via Phabricator via cfe-commits]

llunak updated this revision to Diff 246122.
llunak retitled this revision from "fix -fcodegen-modules code when used with 
PCH (PR44958)" to "fix -fcodegen-modules code when used with PCH (PR44953)".
llunak edited the summary of this revision.
llunak added a comment.

Upon further investigation it turns out I probably should not have enabled 
those two places for PCH in D69778  at all, 
since they do not deal with -fmodules-codegen. So reverting those two places 
should be the right fix. That also makes a test irrelevant (not much point in 
testing a rare corner-case for a code path never taken).


Repository:
  rC Clang

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

https://reviews.llvm.org/D74846

Files:
  clang/lib/Serialization/ASTWriterDecl.cpp


Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -1011,16 +1011,15 @@
 
   if (D->getStorageDuration() == SD_Static) {
 bool ModulesCodegen = false;
-if (!D->getDescribedVarTemplate() && !D->getMemberSpecializationInfo() &&
+if (Writer.WritingModule &&
+!D->getDescribedVarTemplate() && !D->getMemberSpecializationInfo() &&
 !isa(D)) {
   // When building a C++ Modules TS module interface unit, a strong
   // definition in the module interface is provided by the compilation of
   // that module interface unit, not by its users. (Inline variables are
   // still emitted in module users.)
   ModulesCodegen =
-  (((Writer.WritingModule &&
- Writer.WritingModule->Kind == Module::ModuleInterfaceUnit) ||
-Writer.Context->getLangOpts().BuildingPCHWithObjectFile) &&
+  (Writer.WritingModule->Kind == Module::ModuleInterfaceUnit &&
Writer.Context->GetGVALinkageForVariable(D) == GVA_StrongExternal);
 }
 Record.push_back(ModulesCodegen);
@@ -2451,9 +2450,8 @@
   bool ModulesCodegen = false;
   if (!FD->isDependentContext()) {
 Optional Linkage;
-if ((Writer->WritingModule &&
- Writer->WritingModule->Kind == Module::ModuleInterfaceUnit) ||
-Writer->Context->getLangOpts().BuildingPCHWithObjectFile) {
+if (Writer->WritingModule &&
+Writer->WritingModule->Kind == Module::ModuleInterfaceUnit) {
   // When building a C++ Modules TS module interface unit, a strong
   // definition in the module interface is provided by the compilation of
   // that module interface unit, not by its users. (Inline functions are


Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -1011,16 +1011,15 @@
 
   if (D->getStorageDuration() == SD_Static) {
 bool ModulesCodegen = false;
-if (!D->getDescribedVarTemplate() && !D->getMemberSpecializationInfo() &&
+if (Writer.WritingModule &&
+!D->getDescribedVarTemplate() && !D->getMemberSpecializationInfo() &&
 !isa(D)) {
   // When building a C++ Modules TS module interface unit, a strong
   // definition in the module interface is provided by the compilation of
   // that module interface unit, not by its users. (Inline variables are
   // still emitted in module users.)
   ModulesCodegen =
-  (((Writer.WritingModule &&
- Writer.WritingModule->Kind == Module::ModuleInterfaceUnit) ||
-Writer.Context->getLangOpts().BuildingPCHWithObjectFile) &&
+  (Writer.WritingModule->Kind == Module::ModuleInterfaceUnit &&
Writer.Context->GetGVALinkageForVariable(D) == GVA_StrongExternal);
 }
 Record.push_back(ModulesCodegen);
@@ -2451,9 +2450,8 @@
   bool ModulesCodegen = false;
   if (!FD->isDependentContext()) {
 Optional Linkage;
-if ((Writer->WritingModule &&
- Writer->WritingModule->Kind == Module::ModuleInterfaceUnit) ||
-Writer->Context->getLangOpts().BuildingPCHWithObjectFile) {
+if (Writer->WritingModule &&
+Writer->WritingModule->Kind == Module::ModuleInterfaceUnit) {
   // When building a C++ Modules TS module interface unit, a strong
   // definition in the module interface is provided by the compilation of
   // that module interface unit, not by its users. (Inline functions are
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74846: fix -fcodegen-modules code when used with PCH (PR44953)

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

In D74846#1889826 , @dblaikie wrote:

> I know it's a bit of an awkward situation to test - but please include one to 
> demonstrate why the original code was inappropriate. At least useful for 
> reviewing/validating the patch, but also might help someone else not to make 
> the same change again in the future.


I don't know how to do that, except by doing the does-not-crash test that you 
refused above. The only way I can trigger the change in 
ASTDeclWriter::VisitVarDecl() to make any difference is by using the 
_declspec(selectany), but that crashes without the fix and does nothing with 
it. I've tried static variables, static const variables, templated statics, 
inline variables, statics in functions and I don't know what the code actually 
affects other than _declspec(selectany) crashing.

Also, since I apparently don't know what the code in fact causes or why 
_declspec(selectany) crashes, I actually also don't know why the original code 
was inappropriate, other than that it crashes for an unknown reason. Since I 
simply reused the modules code for PCH, maybe _declspec(selectany) would crash 
with modules too? I don't know. But before we get to this, can we please first 
fix my patch for 10.0? I didn't check properly for -fmodules-codegen in D69778 
, that's for certain. That can be fixed before 
finding out why exactly it causes the trouble it causes.

In D74846#1891208 , @aganea wrote:

> @llunak Do you have time to address this patch this week? If not, I can 
> finish it and land it. Please let me know. We'd like to see it merged into 
> 10.0.


Feel free to do whatever you think would help you.


Repository:
  rC Clang

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

https://reviews.llvm.org/D74846



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

[PATCH] D74846: fix -fcodegen-modules code when used with PCH (PR44953)

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

In D74846#1891580 , @dblaikie wrote:

> In D74846#1891486 , @llunak wrote:
>
> > But before we get to this, can we please first fix my patch for 10.0? I 
> > didn't check properly for -fmodules-codegen in D69778 
> > , that's for certain. That can be fixed 
> > before finding out why exactly it causes the trouble it causes.
>
>
> Generally I'm not in favor of committing fixes/changes that are not 
> understood (that leaves traps in the codebase & potentially other bugs, if 
> we're committing code we don't understand). I'd rather revert the original 
> patch until it's better understood.


I think it'll help to split this into smaller parts:

1. In D69778  I made it possible to use 
-fmodules-codegen also for PCHs by simply enabling the ModulesCodegen code also 
when -fmodules-codegen is used together with -building-pch-with-obj. Looking at 
ASTDeclWriter::VisitVarDecl() in current git it can be clearly seen that I 
messed up, as the code around line 1020 depends on -building-pch-with-obj but 
not -fmodules-codegen, so the ModulesCodegen code gets activated also when 
clang-cl /Yc is used, without -fmodules-codegen. That's clearly wrong and 
that's why aganea runs into a crash caused by ModulesCodegen code despite not 
asking for it. I find that part very clear and understood, both versions of my 
patch fixed that in some way, and that's what should be fixed for 10.0.
2. In  D69778  I included all the 
ModulesCodegen code, even the one checking for Module::ModuleInterfaceUnit 
instead of -fmodules-codegen (and that's how I made the mistake). I assumed 
Module::ModuleInterfaceUnit was some kind of equivalent to the PCH's own object 
file. That might have been a mistake, I don't know. Given that I'm unable to 
find a variable for which that piece of code would make a difference, I can't 
even check. Regardless of what it is, it's unimportant for 10.0.
3. There's the crash about the _declspec(selectany) variable from the 
bugreport. It was triggered by my mistake, but I don't know what the actual 
cause of it is. And as said before, it may not be related to my patch at all 
other than that it got triggered by it. Again, it's presumably not important 
enough for 10.0.

>>> I know it's a bit of an awkward situation to test - but please include one 
>>> to demonstrate why the original code was inappropriate. At least useful for 
>>> reviewing/validating the patch, but also might help someone else not to 
>>> make the same change again in the future.
>> 
>> I don't know how to do that, except by doing the does-not-crash test that 
>> you refused above. The only way I can trigger the change in 
>> ASTDeclWriter::VisitVarDecl() to make any difference is by using the 
>> _declspec(selectany), but that crashes without the fix and does nothing with 
>> it.
> 
> Could you explain more what you mean by "does nothing"? I would've assumed it 
> would go down a path to generate IR that was otherwise/previously 
> untested/unreachable (due to the crash) - so testing that the resulting IR is 
> as expected (that the IR contains a selectany (or however that's lowered to 
> IR) declaration and that declaration is used to initialize the 'desc' 
> variable in main?) is what I would think would be appropriate here.

"Does nothing" means that my current patch disables the ModulesCodegen code for 
it, so it "does nothing" other than taking the normal non-ModulesCodegen paths, 
so there's nothing to test compared to the normal case. At least for the 1) 
case I don't see what to test and how to do it. And for case 2) I also don't 
know what to test, because I can't find what difference it makes in the PCH 
case, except for triggering the crash. And I don't know how to test the crash 
either (except by crashing/not-crashing), because it crashes before it 
generates any code.

>> Also, since I apparently don't know what the code in fact causes or why 
>> _declspec(selectany) crashes, I actually also don't know why the original 
>> code was inappropriate, other than that it crashes for an unknown reason. 
>> Since I simply reused the modules code for PCH, maybe _declspec(selectany) 
>> would crash with modules too? I don't know.
> 
> Could you verify this? My attempt to do so doesn't seem to have reproduced 
> the failure with modular codegen:

Your testcase is not equivalent. In your case ASTDeclWriter::VisitVarDecl() has 
ModulesCodegen set to false for 'D3D12_DEFAULT', because 
'Writer.WritingModule->Kind == Module::ModuleInterfaceUnit' is false. Due to my 
mistake, ModulesCodegen was true in the PCH case.


Repository:
  rC Clang

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

https://reviews.llvm.org/D74846



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
ht

[PATCH] D73846: make sure to not warn about unused macros from -D

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Ping...


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

https://reviews.llvm.org/D73846



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

[PATCH] D69585: PerformPendingInstatiations() already in the PCH

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Ping..


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

https://reviews.llvm.org/D69585



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

[PATCH] D73852: [clang] detect switch fallthrough marked by a comment (PR43465)

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

In D73852#1901186 , @rsmith wrote:

> We shouldn't enable the warning under -Wextra in language modes where there's 
> no standard way to suppress it.


That may be true, but that is not what the bugreport is about, it explicitly 
mentions flex. There are codebases that explicitly use -Wimplicit-fallthrough, 
and flex generates the same code even for C++. So either Clang needs to handle 
it, or flex needs to change (which may be non-trivial if 
https://bugs.llvm.org/show_bug.cgi?id=43465#c24 is right), or codebases using 
flex will need to either special-case Clang or live with the warning.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73852



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

[PATCH] D73852: [clang] detect switch fallthrough marked by a comment (PR43465)

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

In D73852#1901895 , @rsmith wrote:

> Thank you, Luboš, and sorry for the process problems here.


FWIW, the Clang repository here in Phabricator is still 'Inactive', so even 
though 
https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface
 does say to use it, reviews may again not get sent to the correct commits list 
if somebody else gets confused by that too (especially for projects other than 
LLVM itself or Clang, as the page doesn't even mention what the mailing lists 
for those are).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73852



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

[PATCH] D69585: PerformPendingInstatiations() already in the PCH

[Luboš Luňák via Phabricator via cfe-commits]

llunak created this revision.
llunak added reviewers: ABataev, rsmith, dblaikie.
llunak added projects: clang, OpenMP.
Herald added a reviewer: jdoerfert.
Herald added a subscriber: cfe-commits.

This patch makes template instantiations be already performed in the PCH 
instead of it being done in every single file that uses the PCH. I can see 
20-30% build time saved with the few tests I've tried.

The delaying of template instantiations until the PCH is used comes from 
7f76d11dcc9196e1fc9d1308da9ed2330a6b06c2 , which doesn't really give any useful 
reasoning for it, except for extending a unittest, which however now passes 
even with the instantiation moved back into the PCH. Since modules do not delay 
instantiation either, presumably whatever was wrong there has been fixed 
already.

I've built a complete debug build of LibreOffice with the patch and everything 
seems to work fine.

All Clang tests pass fine as well, with the exception of 23 tests, 22 of which 
are in OpenMP:

- OpenMP/declare_target_codegen.cpp fails because some virtuals of template 
classes are not emitted. This is fixed by the second change in the patch. I do 
not understand the purpose of that code (neither I have any clue about OpenMP), 
but it was introduced in d2c1dd5902782fb773c64dbf4e0b529aa4044b05 , which also 
changed this very test, and the change makes the test pass again.
- The remaining 22 tests, CodeGenCXX/vla-lambda-capturing.cpp and 21 in 
OpenMP/, all use the same FileCheck for normal compilation and using the source 
file as PCH input. Instantiating already in the PCH reorders some things, which 
makes the tests fail. Looking at the differences in the generated output, they 
all seem to be functionally equivalent to me, they just do not match exactly 
anymore.

That obviously makes the patch not ready, but I'd like to get feedback on this, 
before spending the time on adjusting the tests (which I expect will be quite a 
hassle[*]). So, is there any problem with this patch, besides adjusting the 
tests? And is there some easier way to handle those 21 OpenMP tests besides 
possibly (up to) doubling the checks in them?

[X] BTW, https://pastebin.com/Dg2L7K27 helped quite a lot with reviewing the 
differences.


Repository:
  rC Clang

https://reviews.llvm.org/D69585

Files:
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaDeclCXX.cpp


Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -15569,7 +15569,7 @@
 return;
   // Do not mark as used if compiling for the device outside of the target
   // region.
-  if (LangOpts.OpenMP && LangOpts.OpenMPIsDevice &&
+  if (TUKind != TU_Prefix && LangOpts.OpenMP && LangOpts.OpenMPIsDevice &&
   !isInOpenMPDeclareTargetContext() &&
   !isInOpenMPTargetExecutionDirective()) {
 if (!DefinitionRequired)
Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -981,6 +981,12 @@
  LateParsedInstantiations.begin(),
  LateParsedInstantiations.end());
 LateParsedInstantiations.clear();
+
+{
+  llvm::TimeTraceScope TimeScope("PerformPendingInstantiations",
+ StringRef(""));
+  PerformPendingInstantiations();
+}
   }
 
   DiagnoseUnterminatedPragmaPack();


Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -15569,7 +15569,7 @@
 return;
   // Do not mark as used if compiling for the device outside of the target
   // region.
-  if (LangOpts.OpenMP && LangOpts.OpenMPIsDevice &&
+  if (TUKind != TU_Prefix && LangOpts.OpenMP && LangOpts.OpenMPIsDevice &&
   !isInOpenMPDeclareTargetContext() &&
   !isInOpenMPTargetExecutionDirective()) {
 if (!DefinitionRequired)
Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -981,6 +981,12 @@
  LateParsedInstantiations.begin(),
  LateParsedInstantiations.end());
 LateParsedInstantiations.clear();
+
+{
+  llvm::TimeTraceScope TimeScope("PerformPendingInstantiations",
+ StringRef(""));
+  PerformPendingInstantiations();
+}
   }
 
   DiagnoseUnterminatedPragmaPack();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64284: (WIP) share template instantiations from PCH in one .o if -building-pch-with-obj

[Luboš Luňák via Phabricator via cfe-commits]

llunak abandoned this revision.
llunak added a comment.

Due to some technical problems with this approach and lack of feedback, I'm 
scratching this one. A new approach is at https://reviews.llvm.org/D69585 .


Repository:
  rC Clang

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

https://reviews.llvm.org/D64284



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

[PATCH] D69585: PerformPendingInstatiations() already in the PCH

[Luboš Luňák via Phabricator via cfe-commits]

llunak updated this revision to Diff 227575.
llunak added a comment.

Let's go a different route. This patch fully passes all tests and so should be 
ready to be committed.

It does so by opting out for OpenMP, which can be handled later whenever 
somebody who know about OpenMP looks at it.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69585

Files:
  clang/lib/Sema/Sema.cpp
  clang/test/CodeGenCXX/vla-lambda-capturing.cpp


Index: clang/test/CodeGenCXX/vla-lambda-capturing.cpp
===
--- clang/test/CodeGenCXX/vla-lambda-capturing.cpp
+++ clang/test/CodeGenCXX/vla-lambda-capturing.cpp
@@ -1,6 +1,6 @@
-// RUN: %clang_cc1 %s -std=c++11 -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 %s -std=c++11 -emit-llvm -o - | FileCheck --check-prefixes 
CHECK,CHECK-NOPCH %s
 // RUN: %clang_cc1 %s -std=c++11 -emit-pch -o %t
-// RUN: %clang_cc1 %s -std=c++11 -include-pch %t -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 %s -std=c++11 -include-pch %t -emit-llvm -o - | FileCheck 
--check-prefixes CHECK,CHECK-PCH %s
 
 #ifndef HEADER
 #define HEADER
@@ -111,14 +111,14 @@
 // CHECK: call void @llvm.stackrestore(
 // CHECK: ret void
 
-// CHECK: define linkonce_odr{{.*}} void [[F_INT_LAMBDA]]([[CAP_TYPE2]]*
-// CHECK: [[THIS:%.+]] = load [[CAP_TYPE2]]*, [[CAP_TYPE2]]**
-// CHECK: [[SIZE_REF:%.+]] = getelementptr inbounds [[CAP_TYPE2]], 
[[CAP_TYPE2]]* [[THIS]], i{{.+}} 0, i{{.+}} 0
-// CHECK: [[SIZE:%.+]] = load [[INTPTR_T]], [[INTPTR_T]]* [[SIZE_REF]]
-// CHECK: call i{{.+}}* @llvm.stacksave()
-// CHECK: alloca [[INTPTR_T]], [[INTPTR_T]] [[SIZE]]
-// CHECK: call void @llvm.stackrestore(
-// CHECK: ret void
+// CHECK-NOPCH: define linkonce_odr{{.*}} void [[F_INT_LAMBDA]]([[CAP_TYPE2]]*
+// CHECK-NOPCH: [[THIS:%.+]] = load [[CAP_TYPE2]]*, [[CAP_TYPE2]]**
+// CHECK-NOPCH: [[SIZE_REF:%.+]] = getelementptr inbounds [[CAP_TYPE2]], 
[[CAP_TYPE2]]* [[THIS]], i{{.+}} 0, i{{.+}} 0
+// CHECK-NOPCH: [[SIZE:%.+]] = load [[INTPTR_T]], [[INTPTR_T]]* [[SIZE_REF]]
+// CHECK-NOPCH: call i{{.+}}* @llvm.stacksave()
+// CHECK-NOPCH: alloca [[INTPTR_T]], [[INTPTR_T]] [[SIZE]]
+// CHECK-NOPCH: call void @llvm.stackrestore(
+// CHECK-NOPCH: ret void
 
 // CHECK: define linkonce_odr{{.*}} void [[B_INT_LAMBDA]]([[CAP_TYPE3]]*
 // CHECK: [[SIZE2_REF:%.+]] = getelementptr inbounds [[CAP_TYPE3]], 
[[CAP_TYPE3]]* [[THIS:%.+]], i{{[0-9]+}} 0, i{{[0-9]+}} 1
@@ -168,4 +168,14 @@
 // CHECK: [[MUL:%.+]] = mul {{.*}} i{{[0-9]+}} [[SIZE2]], [[SIZE1]]
 // CHECK: mul {{.*}} i{{[0-9]+}} {{[0-9]+}}, [[MUL]]
 // CHECK: ret void
+
+// CHECK-PCH: define linkonce_odr{{.*}} void [[F_INT_LAMBDA]]([[CAP_TYPE2]]*
+// CHECK-PCH: [[THIS:%.+]] = load [[CAP_TYPE2]]*, [[CAP_TYPE2]]**
+// CHECK-PCH: [[SIZE_REF:%.+]] = getelementptr inbounds [[CAP_TYPE2]], 
[[CAP_TYPE2]]* [[THIS]], i{{.+}} 0, i{{.+}} 0
+// CHECK-PCH: [[SIZE:%.+]] = load [[INTPTR_T]], [[INTPTR_T]]* [[SIZE_REF]]
+// CHECK-PCH: call i{{.+}}* @llvm.stacksave()
+// CHECK-PCH: alloca [[INTPTR_T]], [[INTPTR_T]] [[SIZE]]
+// CHECK-PCH: call void @llvm.stackrestore(
+// CHECK-PCH: ret void
+
 #endif
Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -981,6 +981,14 @@
  LateParsedInstantiations.begin(),
  LateParsedInstantiations.end());
 LateParsedInstantiations.clear();
+
+// FIXME: Instantiating implicit templates already in the PCH breaks some
+// OpenMP-specific code paths, see https://reviews.llvm.org/D69585 .
+if (!LangOpts.OpenMP) {
+  llvm::TimeTraceScope TimeScope("PerformPendingInstantiations",
+ StringRef(""));
+  PerformPendingInstantiations();
+}
   }
 
   DiagnoseUnterminatedPragmaPack();


Index: clang/test/CodeGenCXX/vla-lambda-capturing.cpp
===
--- clang/test/CodeGenCXX/vla-lambda-capturing.cpp
+++ clang/test/CodeGenCXX/vla-lambda-capturing.cpp
@@ -1,6 +1,6 @@
-// RUN: %clang_cc1 %s -std=c++11 -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 %s -std=c++11 -emit-llvm -o - | FileCheck --check-prefixes CHECK,CHECK-NOPCH %s
 // RUN: %clang_cc1 %s -std=c++11 -emit-pch -o %t
-// RUN: %clang_cc1 %s -std=c++11 -include-pch %t -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 %s -std=c++11 -include-pch %t -emit-llvm -o - | FileCheck --check-prefixes CHECK,CHECK-PCH %s
 
 #ifndef HEADER
 #define HEADER
@@ -111,14 +111,14 @@
 // CHECK: call void @llvm.stackrestore(
 // CHECK: ret void
 
-// CHECK: define linkonce_odr{{.*}} void [[F_INT_LAMBDA]]([[CAP_TYPE2]]*
-// CHECK: [[THIS:%.+]] = load [[CAP_TYPE2]]*, [[CAP_TYPE2]]**
-// CHECK: [[SIZE_REF:%.+]] = getelementptr inbounds [[CAP_TYPE2]], [[CAP_TYPE2]]* [[THIS]], i{{.+}} 0, i{{.+}} 0
-// CHECK: [[SIZE:%.+]] = load [[INTPTR_T]], [[INTP

[PATCH] D69750: make -ftime-trace also trace time spent creating debug info

[Luboš Luňák via Phabricator via cfe-commits]

llunak created this revision.
llunak added a reviewer: anton-afanasyev.
llunak added a project: clang.
Herald added subscribers: cfe-commits, aprantl.

In debug builds a noticeable time can be spent generating debug info, so make 
-ftime-trace track that too.


Repository:
  rC Clang

https://reviews.llvm.org/D69750

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp


Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -46,6 +46,7 @@
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MD5.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/TimeProfiler.h"
 using namespace clang;
 using namespace clang::CodeGen;
 
@@ -2946,6 +2947,13 @@
   if (Ty.isNull())
 return nullptr;
 
+  llvm::TimeTraceScope TimeScope("DebugType", [&]() {
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+Ty.print(OS, getPrintingPolicy());
+return Name;
+  });
+
   // Unwrap the type as needed for debug information.
   Ty = UnwrapTypeForDebugInfo(Ty, CGM.getContext());
 
@@ -3664,6 +3672,15 @@
   if (!D)
 return;
 
+  llvm::TimeTraceScope TimeScope("DebugFunction", [&]() {
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+if (const NamedDecl *ND = dyn_cast(D))
+  ND->getNameForDiagnostic(OS, getPrintingPolicy(),
+   /*Qualified=*/true);
+return Name;
+  });
+
   llvm::DINode::DIFlags Flags = llvm::DINode::FlagZero;
   llvm::DIFile *Unit = getOrCreateFile(Loc);
   bool IsDeclForCallSite = Fn ? true : false;
@@ -4384,6 +4401,14 @@
   if (D->hasAttr())
 return;
 
+  llvm::TimeTraceScope TimeScope("DebugGlobalVariable", [&]() {
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+D->getNameForDiagnostic(OS, getPrintingPolicy(),
+/*Qualified=*/true);
+return Name;
+  });
+
   // If we already created a DIGlobalVariable for this declaration, just attach
   // it to the llvm::GlobalVariable.
   auto Cached = DeclCache.find(D->getCanonicalDecl());
@@ -,6 +4469,14 @@
   assert(DebugKind >= codegenoptions::LimitedDebugInfo);
   if (VD->hasAttr())
 return;
+  llvm::TimeTraceScope TimeScope("DebugConstGlobalVariable", [&]() {
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+VD->getNameForDiagnostic(OS, getPrintingPolicy(),
+ /*Qualified=*/true);
+return Name;
+  });
+
   auto Align = getDeclAlignIfRequired(VD, CGM.getContext());
   // Create the descriptor for the variable.
   llvm::DIFile *Unit = getOrCreateFile(VD->getLocation());


Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -46,6 +46,7 @@
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MD5.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/TimeProfiler.h"
 using namespace clang;
 using namespace clang::CodeGen;
 
@@ -2946,6 +2947,13 @@
   if (Ty.isNull())
 return nullptr;
 
+  llvm::TimeTraceScope TimeScope("DebugType", [&]() {
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+Ty.print(OS, getPrintingPolicy());
+return Name;
+  });
+
   // Unwrap the type as needed for debug information.
   Ty = UnwrapTypeForDebugInfo(Ty, CGM.getContext());
 
@@ -3664,6 +3672,15 @@
   if (!D)
 return;
 
+  llvm::TimeTraceScope TimeScope("DebugFunction", [&]() {
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+if (const NamedDecl *ND = dyn_cast(D))
+  ND->getNameForDiagnostic(OS, getPrintingPolicy(),
+   /*Qualified=*/true);
+return Name;
+  });
+
   llvm::DINode::DIFlags Flags = llvm::DINode::FlagZero;
   llvm::DIFile *Unit = getOrCreateFile(Loc);
   bool IsDeclForCallSite = Fn ? true : false;
@@ -4384,6 +4401,14 @@
   if (D->hasAttr())
 return;
 
+  llvm::TimeTraceScope TimeScope("DebugGlobalVariable", [&]() {
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+D->getNameForDiagnostic(OS, getPrintingPolicy(),
+/*Qualified=*/true);
+return Name;
+  });
+
   // If we already created a DIGlobalVariable for this declaration, just attach
   // it to the llvm::GlobalVariable.
   auto Cached = DeclCache.find(D->getCanonicalDecl());
@@ -,6 +4469,14 @@
   assert(DebugKind >= codegenoptions::LimitedDebugInfo);
   if (VD->hasAttr())
 return;
+  llvm::TimeTraceScope TimeScope("DebugConstGlobalVariable", [&]() {
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+VD->getNameForDiagnostic(OS, getPrintingPolicy(),
+ /*Qualified=*/true);
+return Name;
+  });
+
   auto Align = getDeclAlignIfRequired(VD, CGM.getContext());
   // Create the descriptor for the variable.
   llvm::DIFile *Unit = getOrCreateFile(VD->getLocation());
_

[PATCH] D69750: make -ftime-trace also trace time spent creating debug info

[Luboš Luňák via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4f2104c5adbc: make -ftime-trace also trace time spent 
creating debug info (authored by llunak).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69750

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp


Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -46,6 +46,7 @@
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MD5.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/TimeProfiler.h"
 using namespace clang;
 using namespace clang::CodeGen;
 
@@ -2968,6 +2969,13 @@
   if (Ty.isNull())
 return nullptr;
 
+  llvm::TimeTraceScope TimeScope("DebugType", [&]() {
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+Ty.print(OS, getPrintingPolicy());
+return Name;
+  });
+
   // Unwrap the type as needed for debug information.
   Ty = UnwrapTypeForDebugInfo(Ty, CGM.getContext());
 
@@ -3686,6 +3694,15 @@
   if (!D)
 return;
 
+  llvm::TimeTraceScope TimeScope("DebugFunction", [&]() {
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+if (const NamedDecl *ND = dyn_cast(D))
+  ND->getNameForDiagnostic(OS, getPrintingPolicy(),
+   /*Qualified=*/true);
+return Name;
+  });
+
   llvm::DINode::DIFlags Flags = llvm::DINode::FlagZero;
   llvm::DIFile *Unit = getOrCreateFile(Loc);
   bool IsDeclForCallSite = Fn ? true : false;
@@ -4406,6 +4423,14 @@
   if (D->hasAttr())
 return;
 
+  llvm::TimeTraceScope TimeScope("DebugGlobalVariable", [&]() {
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+D->getNameForDiagnostic(OS, getPrintingPolicy(),
+/*Qualified=*/true);
+return Name;
+  });
+
   // If we already created a DIGlobalVariable for this declaration, just attach
   // it to the llvm::GlobalVariable.
   auto Cached = DeclCache.find(D->getCanonicalDecl());
@@ -4466,6 +4491,14 @@
   assert(DebugKind >= codegenoptions::LimitedDebugInfo);
   if (VD->hasAttr())
 return;
+  llvm::TimeTraceScope TimeScope("DebugConstGlobalVariable", [&]() {
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+VD->getNameForDiagnostic(OS, getPrintingPolicy(),
+ /*Qualified=*/true);
+return Name;
+  });
+
   auto Align = getDeclAlignIfRequired(VD, CGM.getContext());
   // Create the descriptor for the variable.
   llvm::DIFile *Unit = getOrCreateFile(VD->getLocation());


Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -46,6 +46,7 @@
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MD5.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/TimeProfiler.h"
 using namespace clang;
 using namespace clang::CodeGen;
 
@@ -2968,6 +2969,13 @@
   if (Ty.isNull())
 return nullptr;
 
+  llvm::TimeTraceScope TimeScope("DebugType", [&]() {
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+Ty.print(OS, getPrintingPolicy());
+return Name;
+  });
+
   // Unwrap the type as needed for debug information.
   Ty = UnwrapTypeForDebugInfo(Ty, CGM.getContext());
 
@@ -3686,6 +3694,15 @@
   if (!D)
 return;
 
+  llvm::TimeTraceScope TimeScope("DebugFunction", [&]() {
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+if (const NamedDecl *ND = dyn_cast(D))
+  ND->getNameForDiagnostic(OS, getPrintingPolicy(),
+   /*Qualified=*/true);
+return Name;
+  });
+
   llvm::DINode::DIFlags Flags = llvm::DINode::FlagZero;
   llvm::DIFile *Unit = getOrCreateFile(Loc);
   bool IsDeclForCallSite = Fn ? true : false;
@@ -4406,6 +4423,14 @@
   if (D->hasAttr())
 return;
 
+  llvm::TimeTraceScope TimeScope("DebugGlobalVariable", [&]() {
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+D->getNameForDiagnostic(OS, getPrintingPolicy(),
+/*Qualified=*/true);
+return Name;
+  });
+
   // If we already created a DIGlobalVariable for this declaration, just attach
   // it to the llvm::GlobalVariable.
   auto Cached = DeclCache.find(D->getCanonicalDecl());
@@ -4466,6 +4491,14 @@
   assert(DebugKind >= codegenoptions::LimitedDebugInfo);
   if (VD->hasAttr())
 return;
+  llvm::TimeTraceScope TimeScope("DebugConstGlobalVariable", [&]() {
+std::string Name;
+llvm::raw_string_ostream OS(Name);
+VD->getNameForDiagnostic(OS, getPrintingPolicy(),
+ /*Qualified=*/true);
+return Name;
+  });
+
   auto Align = getDeclAlignIfRequired(VD, CGM.getContext());
   // Create the descriptor for the variable.
   llvm::DIFile *Unit = getOrCreateFile(VD->getLocation());
___

[PATCH] D69778: Make -fmodules-codegen and -fmodules-debuginfo work also with precompiled headers

[Luboš Luňák via Phabricator via cfe-commits]

llunak created this revision.
llunak added a reviewer: dblaikie.
llunak added a project: clang.
Herald added a subscriber: cfe-commits.

This patch allows to build PCH's (with -building-pch-with-obj and the extra .o 
file) with -fmodules-codegen -fmodules-debuginfo to allow emitting shared code 
into the extra .o file (presumably similarly how modules emit that to wherever 
they emit it). This saves up to 20% of build time here. I can build pretty much 
a complete debug build of LibreOffice with this, with just two minor problems 
(to be sorted out later).

The patch is fairly simple, it basically just duplicates -fmodules checks to 
also alternatively check -building-pch-with-obj.


Repository:
  rC Clang

https://reviews.llvm.org/D69778

Files:
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/Modules/Inputs/codegen-flags/foo.h
  clang/test/PCH/codegen.cpp

Index: clang/test/PCH/codegen.cpp
===
--- /dev/null
+++ clang/test/PCH/codegen.cpp
@@ -0,0 +1,30 @@
+// This test is the PCH version of Modules/codegen-flags.test . It uses its inputs.
+// The purpose of this test is just verify that the codegen options work with PCH as well.
+// All the codegen functionality should be tested in Modules/.
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// REQUIRES: x86-registered-target
+
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules-codegen -x c++-header -building-pch-with-obj -emit-pch %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-cg.pch
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules-debuginfo -x c++-header -building-pch-with-obj -emit-pch %S/../Modules/Inputs/codegen-flags/foo.h -o %t/foo-di.pch
+
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - %s -include-pch %t/foo-cg.pch -building-pch-with-obj -fmodules-codegen | FileCheck --check-prefix=CG %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - %s -include-pch %t/foo-di.pch -building-pch-with-obj -fmodules-debuginfo | FileCheck --check-prefix=DI %s
+
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -include-pch %t/foo-cg.pch %S/../Modules/Inputs/codegen-flags/use.cpp | FileCheck --check-prefix=CG-USE %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -debug-info-kind=limited -o - -include-pch %t/foo-di.pch %S/../Modules/Inputs/codegen-flags/use.cpp | FileCheck --check-prefix=DI-USE %s
+
+// CG: define weak_odr void @_Z2f1v
+// CG: DICompileUnit
+// CG-NOT: DICompositeType
+
+// CG-USE: declare void @_Z2f1v
+// CG-USE: DICompileUnit
+// CG-USE: distinct !DICompositeType(tag: DW_TAG_structure_type, name: "foo"
+
+// DI-NOT: define
+// DI: distinct !DICompositeType(tag: DW_TAG_structure_type, name: "foo"
+
+// DI-USE: define linkonce_odr void @_Z2f1v
+// DI-USE: = !DICompositeType(tag: DW_TAG_structure_type, name: "foo", {{.*}}, flags: DIFlagFwdDecl
Index: clang/test/Modules/Inputs/codegen-flags/foo.h
===
--- clang/test/Modules/Inputs/codegen-flags/foo.h
+++ clang/test/Modules/Inputs/codegen-flags/foo.h
@@ -1,3 +1,4 @@
+#pragma once
 struct foo {
 };
 inline void f1() {
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -990,15 +990,16 @@
 
   if (D->getStorageDuration() == SD_Static) {
 bool ModulesCodegen = false;
-if (Writer.WritingModule &&
-!D->getDescribedVarTemplate() && !D->getMemberSpecializationInfo() &&
+if (!D->getDescribedVarTemplate() && !D->getMemberSpecializationInfo() &&
 !isa(D)) {
   // When building a C++ Modules TS module interface unit, a strong
   // definition in the module interface is provided by the compilation of
   // that module interface unit, not by its users. (Inline variables are
   // still emitted in module users.)
   ModulesCodegen =
-  (Writer.WritingModule->Kind == Module::ModuleInterfaceUnit &&
+  (((Writer.WritingModule &&
+ Writer.WritingModule->Kind == Module::ModuleInterfaceUnit) ||
+Writer.Context->getLangOpts().BuildingPCHWithObjectFile) &&
Writer.Context->GetGVALinkageForVariable(D) == GVA_StrongExternal);
 }
 Record.push_back(ModulesCodegen);
@@ -2397,9 +2398,11 @@
 
   assert(FD->doesThisDeclarationHaveABody());
   bool ModulesCodegen = false;
-  if (Writer->WritingModule && !FD->isDependentContext()) {
+  if (!FD->isDependentContext()) {
 Optional Linkage;
-if (Writer->WritingModule->Kind == Module::ModuleInterfaceUnit) {
+if ((Writer->WritingModule &&
+ Writer->WritingModule->Kind == Module::ModuleInterfaceUnit) ||
+Writer->Context->getLa

[PATCH] D69779: -fmodules-codegen should not emit extern templates

[Luboš Luňák via Phabricator via cfe-commits]

llunak created this revision.
llunak added a reviewer: dblaikie.
llunak added a project: clang.
Herald added a subscriber: cfe-commits.

See the test for a testcase. If a header contains 'extern template', then the 
template should be provided somewhere by an explicit instantiation, so it is 
not necessary to generate a copy. Worse (at least with PCH 
-building-pch-with-obj), the current code actually leads to an unresolved 
symbol, because the PCH's object file will not actually contain functions from 
such a template (because of the GVA_AvailableExternally?), but the object file 
for the explicit instantiation will not contain them either because it will be 
blocked by the information provided by the PCH. I don't know if the same 
problem exists with modules (nor I know how to check for sure), all tests pass. 
If the problem is not there for modules, I can make the change PCH-specific.


Repository:
  rC Clang

https://reviews.llvm.org/D69779

Files:
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/PCH/codegen-extern-template.cpp
  clang/test/PCH/codegen-extern-template.h


Index: clang/test/PCH/codegen-extern-template.h
===
--- /dev/null
+++ clang/test/PCH/codegen-extern-template.h
@@ -0,0 +1,16 @@
+// header for codegen-extern-template.cpp
+#pragma once
+
+template< typename T >
+struct A
+{
+   T foo() { return 10;}
+};
+
+extern template struct A< int >;
+
+inline
+int bar(A* p)
+{
+return p->foo();
+}
Index: clang/test/PCH/codegen-extern-template.cpp
===
--- /dev/null
+++ clang/test/PCH/codegen-extern-template.cpp
@@ -0,0 +1,21 @@
+// Build the PCH with extern template.
+// RUN: %clang -x c++-header %S/codegen-extern-template.h -o %t.pch -Xclang 
-building-pch-with-obj -Xclang -fmodules-codegen
+// Build the PCH's object file.
+// RUN: %clang -c %s -include-pch %t.pch -Xclang -building-pch-with-obj 
-Xclang -fmodules-codegen -o %t.1.o
+// Build source with explicit template instantiation.
+// RUN: %clang -c %s -DMAIN -include-pch %t.pch -o %t.2.o
+// There should be no unresolved symbol.
+// RUN: %clang %t.1.o %t.2.o
+// expected-no-diagnostics
+
+#include "codegen-extern-template.h"
+
+#ifdef MAIN
+template struct A< int >;
+int main()
+{
+A< int > a;
+return bar(&a);
+}
+
+#endif
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -2412,11 +2412,12 @@
 }
 if (Writer->Context->getLangOpts().ModulesCodegen) {
   // Under -fmodules-codegen, codegen is performed for all non-internal,
-  // non-always_inline functions.
+  // non-always_inline functions, unless they are available elsewhere.
   if (!FD->hasAttr()) {
 if (!Linkage)
   Linkage = Writer->Context->GetGVALinkageForFunction(FD);
-ModulesCodegen = *Linkage != GVA_Internal;
+ModulesCodegen =
+*Linkage != GVA_Internal && *Linkage != GVA_AvailableExternally;
   }
 }
   }


Index: clang/test/PCH/codegen-extern-template.h
===
--- /dev/null
+++ clang/test/PCH/codegen-extern-template.h
@@ -0,0 +1,16 @@
+// header for codegen-extern-template.cpp
+#pragma once
+
+template< typename T >
+struct A
+{
+   T foo() { return 10;}
+};
+
+extern template struct A< int >;
+
+inline
+int bar(A* p)
+{
+return p->foo();
+}
Index: clang/test/PCH/codegen-extern-template.cpp
===
--- /dev/null
+++ clang/test/PCH/codegen-extern-template.cpp
@@ -0,0 +1,21 @@
+// Build the PCH with extern template.
+// RUN: %clang -x c++-header %S/codegen-extern-template.h -o %t.pch -Xclang -building-pch-with-obj -Xclang -fmodules-codegen
+// Build the PCH's object file.
+// RUN: %clang -c %s -include-pch %t.pch -Xclang -building-pch-with-obj -Xclang -fmodules-codegen -o %t.1.o
+// Build source with explicit template instantiation.
+// RUN: %clang -c %s -DMAIN -include-pch %t.pch -o %t.2.o
+// There should be no unresolved symbol.
+// RUN: %clang %t.1.o %t.2.o
+// expected-no-diagnostics
+
+#include "codegen-extern-template.h"
+
+#ifdef MAIN
+template struct A< int >;
+int main()
+{
+A< int > a;
+return bar(&a);
+}
+
+#endif
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -2412,11 +2412,12 @@
 }
 if (Writer->Context->getLangOpts().ModulesCodegen) {
   // Under -fmodules-codegen, codegen is performed for all non-internal,
-  // non-always_inline functions.
+  // non-always_inline functions, unless they are available elsewhere.
   if (!FD->hasAttr()) {
 if (!Linkage)
   Linkag

[PATCH] D69585: PerformPendingInstatiations() already in the PCH

[Luboš Luňák via Phabricator via cfe-commits]

llunak updated this revision to Diff 229722.
llunak added a comment.

It turns out that this patch may introduce unwanted changes, specifically it 
can cause err_specialization_after_instantiation if the specialization is done 
in a source file but needed already by code in the PCH. But this seems to be 
rare (I've encountered it only after building the Skia library with 
put-everything-in-the-PCH), is easy to avoid (PCHs often need little tweaks 
anyway) and the performance gain is very much worth it.

Still, this means there should be an option to enable this. Done in this 
version of the patch. I don't know how to handle tests for it, pretty much all 
the tests pass as said above, but it seems a bit silly to duplicate all the 
PCH-related ones to also use the flag.


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

https://reviews.llvm.org/D69585

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/CC1Options.td
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/Sema.cpp


Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -981,6 +981,14 @@
  LateParsedInstantiations.begin(),
  LateParsedInstantiations.end());
 LateParsedInstantiations.clear();
+
+// FIXME: Instantiating implicit templates already in the PCH breaks some
+// OpenMP-specific code paths, see https://reviews.llvm.org/D69585 .
+if(LangOpts.PCHInstantiateTemplates && !LangOpts.OpenMP) {
+  llvm::TimeTraceScope TimeScope("PerformPendingInstantiations",
+ StringRef(""));
+  PerformPendingInstantiations();
+}
   }
 
   DiagnoseUnterminatedPragmaPack();
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3223,6 +3223,7 @@
 
   Opts.CompleteMemberPointers = Args.hasArg(OPT_fcomplete_member_pointers);
   Opts.BuildingPCHWithObjectFile = Args.hasArg(OPT_building_pch_with_obj);
+  Opts.PCHInstantiateTemplates = Args.hasArg(OPT_pch_instantiate_templates);
 }
 
 static bool isStrictlyPreprocessorAction(frontend::ActionKind Action) {
Index: clang/include/clang/Driver/CC1Options.td
===
--- clang/include/clang/Driver/CC1Options.td
+++ clang/include/clang/Driver/CC1Options.td
@@ -672,6 +672,8 @@
   HelpText<"Disable inclusion of timestamp in precompiled headers">;
 def building_pch_with_obj : Flag<["-"], "building-pch-with-obj">,
   HelpText<"This compilation is part of building a PCH with corresponding 
object file.">;
+def pch_instantiate_templates : Flag<["-"], "pch-instantiate-templates">,
+  HelpText<"Perform pending template instantiations already while building the 
PCH.">;
 
 def aligned_alloc_unavailable : Flag<["-"], "faligned-alloc-unavailable">,
   HelpText<"Aligned allocation/deallocation functions are unavailable">;
Index: clang/include/clang/Basic/LangOptions.def
===
--- clang/include/clang/Basic/LangOptions.def
+++ clang/include/clang/Basic/LangOptions.def
@@ -160,6 +160,7 @@
 BENIGN_LANGOPT(CompilingPCH, 1, 0, "building a pch")
 BENIGN_LANGOPT(BuildingPCHWithObjectFile, 1, 0, "building a pch which has a 
corresponding object file")
 BENIGN_LANGOPT(CacheGeneratedPCH, 1, 0, "cache generated PCH files in memory")
+BENIGN_LANGOPT(PCHInstantiateTemplates, 1, 0, "performing pending template 
instantiations already while building a pch")
 COMPATIBLE_LANGOPT(ModulesDeclUse, 1, 0, "require declaration of module 
uses")
 BENIGN_LANGOPT(ModulesSearchAll  , 1, 1, "searching even non-imported modules 
to find unresolved references")
 COMPATIBLE_LANGOPT(ModulesStrictDeclUse, 1, 0, "requiring declaration of 
module uses and all headers to be in modules")


Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -981,6 +981,14 @@
  LateParsedInstantiations.begin(),
  LateParsedInstantiations.end());
 LateParsedInstantiations.clear();
+
+// FIXME: Instantiating implicit templates already in the PCH breaks some
+// OpenMP-specific code paths, see https://reviews.llvm.org/D69585 .
+if(LangOpts.PCHInstantiateTemplates && !LangOpts.OpenMP) {
+  llvm::TimeTraceScope TimeScope("PerformPendingInstantiations",
+ StringRef(""));
+  PerformPendingInstantiations();
+}
   }
 
   DiagnoseUnterminatedPragmaPack();
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cp

[PATCH] D69585: PerformPendingInstatiations() already in the PCH

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Ping.


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

https://reviews.llvm.org/D69585



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

[PATCH] D69778: Make -fmodules-codegen and -fmodules-debuginfo work also with precompiled headers

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Ping.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69778



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

[PATCH] D69779: -fmodules-codegen should not emit extern templates

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Ping.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69779



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

[PATCH] D69778: Make -fmodules-codegen and -fmodules-debuginfo work also with precompiled headers

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

In D69778#1771799 , @rsmith wrote:

> It's a bit weird for this to be controlled by a `-fmodules` flag, but it's 
> only a `-cc1` flag, so I'm OK with that; we can rename it if/when we expose 
> it from the driver.


It's a bit weird, but I didn't want to add -fpch-codegen and then duplicate the 
checks everywhere. And the -building-pch-with-obj part requires a non-trivial 
build setup anyway. But I can create another patch that maps it all to some 
common flag, if wanted.

In D69778#1772125 , @dblaikie wrote:

> I was/am still wondering whether there's a way to coalesce these codepaths 
> between PCH and the existing modular code generation support?


In what way could this be united more? The way I understand it, the code paths 
are pretty much the same, they just need to account for being invoked in 
different contexts. This patch is tiny compared to what it was originally 
before I made it reuse all the codegen code.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69778



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

[PATCH] D69778: Make -fmodules-codegen and -fmodules-debuginfo work also with precompiled headers

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

In D69778#1776472 , @dblaikie wrote:

> I guess one aspect is that -building-pch-with-obj seems like it duplicates 
> the fmodules-codegen concept (both basically are a flag passed during pcm/pch 
> build time that says "I promise to build an object file from this pcm/pch, so 
> rely on that assumption when building other things that depend on the 
> pcm/pch) - if I'd noticed the -building-pch-with-obj going in I would've made 
> the point then.


Is that described somewhere? There's no mention of modules codegen anywhere 
under clang/docs. I was under the impression that modules created the codegen 
part directly in the .pcm in ELF (or whatever) format.

> But yeah, that's out of scope for this patch, but might be something that'd 
> be good to look into to try to merge these features/codepaths more.

Note that AFAICT the -building-pch-with-obj flag is pretty much internal, used 
only by the MSVC-like PCH creation, so presumably this could be still unified 
more without any real harm. And I think I could do the work if it'd be agreed 
that this would be a good thing.

As for this patch itself, could you please also review 
https://reviews.llvm.org/D69585 and https://reviews.llvm.org/D69779 ? I have 
not really tried this patch without the first one, so I'm a bit hesitant to 
merge it alone, and I quickly ran into the problem fixed by the second patch, 
so I'd prefer to merge them all three together if possible.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69778



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

[PATCH] D73846: make sure to not warn about unused macros from -D

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Ping


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

https://reviews.llvm.org/D73846



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

[PATCH] D69585: PerformPendingInstatiations() already in the PCH

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Ping...


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

https://reviews.llvm.org/D69585



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

[PATCH] D74846: fix -fcodegen-modules code when used with PCH (PR44953)

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Ping. I don't particularly care about the declspec(selectany) corner case, but 
at least the mistake from  D69778  should be 
fixed (and it's a simple fix), so that it can be committed again.


Repository:
  rC Clang

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

https://reviews.llvm.org/D74846



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

[PATCH] D69585: PerformPendingInstatiations() already in the PCH

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

In D69585#1922108 , @dblaikie wrote:

> Does this still have an OpenMP special case? The production code changes 
> don't seem to mention OpenMP anymore, but there's still a lot of test updates 
> for OpenMP - what are they for?


It's been handled by b841b9e96e605bed5a1f9b846a07aae88c65ce02 
 . The 
tests need updates because they test compilation both with and without PCH use, 
and this patch reorders templates in the output, without any functional change.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69585



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