[PATCH] D158414: [LexerTest] Use LexTokensUntilEOF() in StringifyArgs

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

ping!


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

https://reviews.llvm.org/D158414

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

[PATCH] D158415: [Lex] Handle repl_input_end in Preprocessor::LexTokensUntilEOF()

[Jonas Hahnfeld via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGabb9eb2778dc: [Lex] Handle repl_input_end in 
Preprocessor::LexTokensUntilEOF() (authored by Hahnfeld).

Changed prior to commit:
  https://reviews.llvm.org/D158415?vs=556849=557607#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158415

Files:
  clang/lib/Lex/Preprocessor.cpp


Index: clang/lib/Lex/Preprocessor.cpp
===
--- clang/lib/Lex/Preprocessor.cpp
+++ clang/lib/Lex/Preprocessor.cpp
@@ -1002,7 +1002,8 @@
   while (1) {
 Token Tok;
 Lex(Tok);
-if (Tok.isOneOf(tok::unknown, tok::eof, tok::eod))
+if (Tok.isOneOf(tok::unknown, tok::eof, tok::eod,
+tok::annot_repl_input_end))
   break;
 if (Tokens != nullptr)
   Tokens->push_back(Tok);


Index: clang/lib/Lex/Preprocessor.cpp
===
--- clang/lib/Lex/Preprocessor.cpp
+++ clang/lib/Lex/Preprocessor.cpp
@@ -1002,7 +1002,8 @@
   while (1) {
 Token Tok;
 Lex(Tok);
-if (Tok.isOneOf(tok::unknown, tok::eof, tok::eod))
+if (Tok.isOneOf(tok::unknown, tok::eof, tok::eod,
+tok::annot_repl_input_end))
   break;
 if (Tokens != nullptr)
   Tokens->push_back(Tok);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158413: [Lex] Introduce Preprocessor::LexTokensUntilEOF()

[Jonas Hahnfeld via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3116d60494f2: [Lex] Introduce 
Preprocessor::LexTokensUntilEOF() (authored by Hahnfeld).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158413

Files:
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/Lex/Preprocessor.cpp
  clang/unittests/Analysis/MacroExpansionContextTest.cpp
  clang/unittests/Basic/SourceManagerTest.cpp
  clang/unittests/Lex/LexerTest.cpp
  clang/unittests/Lex/ModuleDeclStateTest.cpp
  clang/unittests/Lex/PPCallbacksTest.cpp
  clang/unittests/Lex/PPConditionalDirectiveRecordTest.cpp
  clang/unittests/Lex/PPDependencyDirectivesTest.cpp
  clang/unittests/Lex/PPMemoryAllocationsTest.cpp

Index: clang/unittests/Lex/PPMemoryAllocationsTest.cpp
===
--- clang/unittests/Lex/PPMemoryAllocationsTest.cpp
+++ clang/unittests/Lex/PPMemoryAllocationsTest.cpp
@@ -75,12 +75,7 @@
   PP.Initialize(*Target);
   PP.EnterMainSourceFile();
 
-  while (1) {
-Token tok;
-PP.Lex(tok);
-if (tok.is(tok::eof))
-  break;
-  }
+  PP.LexTokensUntilEOF();
 
   size_t NumAllocated = PP.getPreprocessorAllocator().getBytesAllocated();
   float BytesPerDefine = float(NumAllocated) / float(NumMacros);
Index: clang/unittests/Lex/PPDependencyDirectivesTest.cpp
===
--- clang/unittests/Lex/PPDependencyDirectivesTest.cpp
+++ clang/unittests/Lex/PPDependencyDirectivesTest.cpp
@@ -133,12 +133,7 @@
   SmallVector IncludedFiles;
   PP.addPPCallbacks(std::make_unique(PP, IncludedFiles));
   PP.EnterMainSourceFile();
-  while (true) {
-Token tok;
-PP.Lex(tok);
-if (tok.is(tok::eof))
-  break;
-  }
+  PP.LexTokensUntilEOF();
 
   SmallVector IncludedFilesSlash;
   for (StringRef IncludedFile : IncludedFiles)
Index: clang/unittests/Lex/PPConditionalDirectiveRecordTest.cpp
===
--- clang/unittests/Lex/PPConditionalDirectiveRecordTest.cpp
+++ clang/unittests/Lex/PPConditionalDirectiveRecordTest.cpp
@@ -87,13 +87,7 @@
   PP.EnterMainSourceFile();
 
   std::vector toks;
-  while (1) {
-Token tok;
-PP.Lex(tok);
-if (tok.is(tok::eof))
-  break;
-toks.push_back(tok);
-  }
+  PP.LexTokensUntilEOF();
 
   // Make sure we got the tokens that we expected.
   ASSERT_EQ(10U, toks.size());
Index: clang/unittests/Lex/PPCallbacksTest.cpp
===
--- clang/unittests/Lex/PPCallbacksTest.cpp
+++ clang/unittests/Lex/PPCallbacksTest.cpp
@@ -229,13 +229,7 @@
 
 // Lex source text.
 PP.EnterMainSourceFile();
-
-while (true) {
-  Token Tok;
-  PP.Lex(Tok);
-  if (Tok.is(tok::eof))
-break;
-}
+PP.LexTokensUntilEOF();
 
 // Callbacks have been executed at this point -- return filename range.
 return Callbacks;
@@ -259,13 +253,7 @@
 
 // Lex source text.
 PP.EnterMainSourceFile();
-
-while (true) {
-  Token Tok;
-  PP.Lex(Tok);
-  if (Tok.is(tok::eof))
-break;
-}
+PP.LexTokensUntilEOF();
 
 return Callbacks->Results;
   }
@@ -290,12 +278,7 @@
 
 // Lex source text.
 PP.EnterMainSourceFile();
-while (true) {
-  Token Tok;
-  PP.Lex(Tok);
-  if (Tok.is(tok::eof))
-break;
-}
+PP.LexTokensUntilEOF();
 
 return Callbacks->Marks;
   }
@@ -334,12 +317,7 @@
 
 // Lex source text.
 PP.EnterMainSourceFile();
-while (true) {
-  Token Tok;
-  PP.Lex(Tok);
-  if (Tok.is(tok::eof))
-break;
-}
+PP.LexTokensUntilEOF();
 
 PragmaOpenCLExtensionCallbacks::CallbackParameters RetVal = {
   Callbacks->Name,
@@ -477,12 +455,7 @@
 
   // Lex source text.
   PP.EnterMainSourceFile();
-  while (true) {
-Token Tok;
-PP.Lex(Tok);
-if (Tok.is(tok::eof))
-  break;
-  }
+  PP.LexTokensUntilEOF();
 
   ASSERT_EQ(1u, Callbacks->NumCalls);
   ASSERT_EQ(0u, DiagConsumer->getNumErrors());
Index: clang/unittests/Lex/ModuleDeclStateTest.cpp
===
--- clang/unittests/Lex/ModuleDeclStateTest.cpp
+++ clang/unittests/Lex/ModuleDeclStateTest.cpp
@@ -90,12 +90,7 @@
 PP.addPPCallbacks(std::move(C));
 PP.EnterMainSourceFile();
 
-while (1) {
-  Token tok;
-  PP.Lex(tok);
-  if (tok.is(tok::eof))
-break;
-}
+PP.LexTokensUntilEOF();
   }
 
   FileSystemOptions FileMgrOpts;
Index: clang/unittests/Lex/LexerTest.cpp
===
--- clang/unittests/Lex/LexerTest.cpp
+++ clang/unittests/Lex/LexerTest.cpp
@@ -74,13 +74,7 @@
 PP = CreatePP(Source, ModLoader);
 
 std::vector toks;
-while (1) {
-  Token tok;
-  

[PATCH] D158413: [Lex] Introduce Preprocessor::LexTokensUntilEOF()

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added inline comments.



Comment at: clang/lib/Lex/Preprocessor.cpp:1000
+  std::vector toks;
+  while (1) {
+Token tok;

v.g.vassilev wrote:
> Hahnfeld wrote:
> > aaron.ballman wrote:
> > > v.g.vassilev wrote:
> > > > aaron.ballman wrote:
> > > > > Hahnfeld wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > I'd prefer not to assume the token stream has an EOF token 
> > > > > > > (perhaps the stream is one only being used to parse until the 
> > > > > > > `eod` token instead), so if we can turn this into a non-infinite 
> > > > > > > loop, that would make me more comfortable.
> > > > > > I'm not sure I understand entirely. Do you want something like
> > > > > > ```
> > > > > > tok.isOneOf(tok::unknown, tok::eof, tok::eod)
> > > > > > ```
> > > > > > instead of `tok.is(tok::eof)`? Can this happen at the level of the 
> > > > > > `Preprocessor`?
> > > > > I was thinking something more along the lines of:
> > > > > ```
> > > > > if (Tokens) {
> > > > >   for (Token Tok; !Tok.isOneOf(tok::eof, tok::eod); Lex(Tok))
> > > > > Tokens->push_back(Tok);
> > > > > }
> > > > > ```
> > > > > but I hadn't thought about `tok::unknown`; that might be a good one 
> > > > > to also include given that clangd operates on partial sources.
> > > > > 
> > > > I was wondering if we could somehow merge this routine with 
> > > > `Parser::SkipUntil` since they seem to be doing a very similar tasks.
> > > That could perhaps end up looking reasonable (they do similar tasks aside 
> > > from collecting the tokens that are being skipped). Do you need the 
> > > interface to be on `Preprocessor` or `Parser` though (or does it not 
> > > really matter for you)?
> > > `tok.isOneOf(tok::unknown, tok::eof, tok::eod)`
> > 
> > I implemented this check, let me know if this looks reasonable. The code 
> > you posted doesn't do what we need because we also want to lex if `Tokens` 
> > is `nullptr`, so the hierarchy must be an `if` inside the loop.
> > Given these additional token kinds, does `UntilEOF` still make sense or do 
> > we want another name? Note that I'll leave `repl_input_end` to 
> > https://reviews.llvm.org/D158415.
> > 
> > > I was wondering if we could somehow merge this routine with 
> > > `Parser::SkipUntil` since they seem to be doing a very similar tasks.
> > 
> > I'm not sure this makes sense, given that `Parser::SkipUntil` requires some 
> > knowledge about the structural input. At the very least, I'd prefer not to 
> > go into that direction for this change.
> I am not sure I understand the reasoning but I somewhat see that having 
> Parser's `SkipUntil` be implemented with our new `Preprocessor::LexUntil...` 
> would require a lot more work. How about adding a fixme note capturing this 
> as a future possible refactoring?
@v.g.vassilev `Parser::SkipUntil` has a long `switch` statement looking at the 
token and taking special actions depending on their kind and the context it 
appears in. I don't see how to generalize this in the way `LexTokensUntilEOF` 
works.


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

https://reviews.llvm.org/D158413

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

[PATCH] D158413: [Lex] Introduce Preprocessor::LexTokensUntilEOF()

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld updated this revision to Diff 557603.
Hahnfeld marked an inline comment as done.
Hahnfeld added a comment.

Address minor naming convention nit.


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

https://reviews.llvm.org/D158413

Files:
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/Lex/Preprocessor.cpp
  clang/unittests/Analysis/MacroExpansionContextTest.cpp
  clang/unittests/Basic/SourceManagerTest.cpp
  clang/unittests/Lex/LexerTest.cpp
  clang/unittests/Lex/ModuleDeclStateTest.cpp
  clang/unittests/Lex/PPCallbacksTest.cpp
  clang/unittests/Lex/PPConditionalDirectiveRecordTest.cpp
  clang/unittests/Lex/PPDependencyDirectivesTest.cpp
  clang/unittests/Lex/PPMemoryAllocationsTest.cpp

Index: clang/unittests/Lex/PPMemoryAllocationsTest.cpp
===
--- clang/unittests/Lex/PPMemoryAllocationsTest.cpp
+++ clang/unittests/Lex/PPMemoryAllocationsTest.cpp
@@ -75,12 +75,7 @@
   PP.Initialize(*Target);
   PP.EnterMainSourceFile();
 
-  while (1) {
-Token tok;
-PP.Lex(tok);
-if (tok.is(tok::eof))
-  break;
-  }
+  PP.LexTokensUntilEOF();
 
   size_t NumAllocated = PP.getPreprocessorAllocator().getBytesAllocated();
   float BytesPerDefine = float(NumAllocated) / float(NumMacros);
Index: clang/unittests/Lex/PPDependencyDirectivesTest.cpp
===
--- clang/unittests/Lex/PPDependencyDirectivesTest.cpp
+++ clang/unittests/Lex/PPDependencyDirectivesTest.cpp
@@ -133,12 +133,7 @@
   SmallVector IncludedFiles;
   PP.addPPCallbacks(std::make_unique(PP, IncludedFiles));
   PP.EnterMainSourceFile();
-  while (true) {
-Token tok;
-PP.Lex(tok);
-if (tok.is(tok::eof))
-  break;
-  }
+  PP.LexTokensUntilEOF();
 
   SmallVector IncludedFilesSlash;
   for (StringRef IncludedFile : IncludedFiles)
Index: clang/unittests/Lex/PPConditionalDirectiveRecordTest.cpp
===
--- clang/unittests/Lex/PPConditionalDirectiveRecordTest.cpp
+++ clang/unittests/Lex/PPConditionalDirectiveRecordTest.cpp
@@ -87,13 +87,7 @@
   PP.EnterMainSourceFile();
 
   std::vector toks;
-  while (1) {
-Token tok;
-PP.Lex(tok);
-if (tok.is(tok::eof))
-  break;
-toks.push_back(tok);
-  }
+  PP.LexTokensUntilEOF();
 
   // Make sure we got the tokens that we expected.
   ASSERT_EQ(10U, toks.size());
Index: clang/unittests/Lex/PPCallbacksTest.cpp
===
--- clang/unittests/Lex/PPCallbacksTest.cpp
+++ clang/unittests/Lex/PPCallbacksTest.cpp
@@ -229,13 +229,7 @@
 
 // Lex source text.
 PP.EnterMainSourceFile();
-
-while (true) {
-  Token Tok;
-  PP.Lex(Tok);
-  if (Tok.is(tok::eof))
-break;
-}
+PP.LexTokensUntilEOF();
 
 // Callbacks have been executed at this point -- return filename range.
 return Callbacks;
@@ -259,13 +253,7 @@
 
 // Lex source text.
 PP.EnterMainSourceFile();
-
-while (true) {
-  Token Tok;
-  PP.Lex(Tok);
-  if (Tok.is(tok::eof))
-break;
-}
+PP.LexTokensUntilEOF();
 
 return Callbacks->Results;
   }
@@ -290,12 +278,7 @@
 
 // Lex source text.
 PP.EnterMainSourceFile();
-while (true) {
-  Token Tok;
-  PP.Lex(Tok);
-  if (Tok.is(tok::eof))
-break;
-}
+PP.LexTokensUntilEOF();
 
 return Callbacks->Marks;
   }
@@ -334,12 +317,7 @@
 
 // Lex source text.
 PP.EnterMainSourceFile();
-while (true) {
-  Token Tok;
-  PP.Lex(Tok);
-  if (Tok.is(tok::eof))
-break;
-}
+PP.LexTokensUntilEOF();
 
 PragmaOpenCLExtensionCallbacks::CallbackParameters RetVal = {
   Callbacks->Name,
@@ -477,12 +455,7 @@
 
   // Lex source text.
   PP.EnterMainSourceFile();
-  while (true) {
-Token Tok;
-PP.Lex(Tok);
-if (Tok.is(tok::eof))
-  break;
-  }
+  PP.LexTokensUntilEOF();
 
   ASSERT_EQ(1u, Callbacks->NumCalls);
   ASSERT_EQ(0u, DiagConsumer->getNumErrors());
Index: clang/unittests/Lex/ModuleDeclStateTest.cpp
===
--- clang/unittests/Lex/ModuleDeclStateTest.cpp
+++ clang/unittests/Lex/ModuleDeclStateTest.cpp
@@ -90,12 +90,7 @@
 PP.addPPCallbacks(std::move(C));
 PP.EnterMainSourceFile();
 
-while (1) {
-  Token tok;
-  PP.Lex(tok);
-  if (tok.is(tok::eof))
-break;
-}
+PP.LexTokensUntilEOF();
   }
 
   FileSystemOptions FileMgrOpts;
Index: clang/unittests/Lex/LexerTest.cpp
===
--- clang/unittests/Lex/LexerTest.cpp
+++ clang/unittests/Lex/LexerTest.cpp
@@ -74,13 +74,7 @@
 PP = CreatePP(Source, ModLoader);
 
 std::vector toks;
-while (1) {
-  Token tok;
-  PP->Lex(tok);
-  if (tok.is(tok::eof))
-break;
-  toks.push_back(tok);
-}
+PP->LexTokensUntilEOF();
 

[PATCH] D158414: [LexerTest] Use LexTokensUntilEOF() in StringifyArgs

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

ping


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

https://reviews.llvm.org/D158414

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

[PATCH] D158413: [Lex] Introduce Preprocessor::LexTokensUntilEOF()

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

ping


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

https://reviews.llvm.org/D158413

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

[PATCH] D158415: [Lex] Handle repl_input_end in Preprocessor::LexTokensUntilEOF()

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld updated this revision to Diff 556849.
Hahnfeld retitled this revision from "[Lex] Handle repl_input_end in 
Preprocessor::LexAll()" to "[Lex] Handle repl_input_end in 
Preprocessor::LexTokensUntilEOF()".
Hahnfeld added a comment.

Rebase on D158413 


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

https://reviews.llvm.org/D158415

Files:
  clang/lib/Lex/Preprocessor.cpp


Index: clang/lib/Lex/Preprocessor.cpp
===
--- clang/lib/Lex/Preprocessor.cpp
+++ clang/lib/Lex/Preprocessor.cpp
@@ -999,7 +999,8 @@
   while (1) {
 Token tok;
 Lex(tok);
-if (tok.isOneOf(tok::unknown, tok::eof, tok::eod))
+if (tok.isOneOf(tok::unknown, tok::eof, tok::eod,
+tok::annot_repl_input_end))
   break;
 if (Tokens != nullptr)
   Tokens->push_back(tok);


Index: clang/lib/Lex/Preprocessor.cpp
===
--- clang/lib/Lex/Preprocessor.cpp
+++ clang/lib/Lex/Preprocessor.cpp
@@ -999,7 +999,8 @@
   while (1) {
 Token tok;
 Lex(tok);
-if (tok.isOneOf(tok::unknown, tok::eof, tok::eod))
+if (tok.isOneOf(tok::unknown, tok::eof, tok::eod,
+tok::annot_repl_input_end))
   break;
 if (Tokens != nullptr)
   Tokens->push_back(tok);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158414: [LexerTest] Use LexTokensUntilEOF() in StringifyArgs

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld updated this revision to Diff 556848.
Hahnfeld retitled this revision from "[LexerTest] Use LexAll() in 
StringifyArgs" to "[LexerTest] Use LexTokensUntilEOF() in StringifyArgs".
Hahnfeld edited the summary of this revision.

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

https://reviews.llvm.org/D158414

Files:
  clang/unittests/Lex/LexerTest.cpp


Index: clang/unittests/Lex/LexerTest.cpp
===
--- clang/unittests/Lex/LexerTest.cpp
+++ clang/unittests/Lex/LexerTest.cpp
@@ -451,18 +451,15 @@
   Token Eof;
   Eof.setKind(tok::eof);
   std::vector ArgTokens;
-  while (1) {
-Token tok;
-PP->Lex(tok);
-if (tok.is(tok::eof)) {
-  ArgTokens.push_back(Eof);
-  break;
+  PP->LexTokensUntilEOF();
+  // Replace all tok::comma with tok::eof for stringification.
+  for (auto  : ArgTokens) {
+if (tok.is(tok::comma)) {
+  tok = Eof;
 }
-if (tok.is(tok::comma))
-  ArgTokens.push_back(Eof);
-else
-  ArgTokens.push_back(tok);
   }
+  // Push a tok::eof as the last element.
+  ArgTokens.push_back(Eof);
 
   auto MacroArgsDeleter = [](MacroArgs *M) { M->destroy(*PP); };
   std::unique_ptr MA(


Index: clang/unittests/Lex/LexerTest.cpp
===
--- clang/unittests/Lex/LexerTest.cpp
+++ clang/unittests/Lex/LexerTest.cpp
@@ -451,18 +451,15 @@
   Token Eof;
   Eof.setKind(tok::eof);
   std::vector ArgTokens;
-  while (1) {
-Token tok;
-PP->Lex(tok);
-if (tok.is(tok::eof)) {
-  ArgTokens.push_back(Eof);
-  break;
+  PP->LexTokensUntilEOF();
+  // Replace all tok::comma with tok::eof for stringification.
+  for (auto  : ArgTokens) {
+if (tok.is(tok::comma)) {
+  tok = Eof;
 }
-if (tok.is(tok::comma))
-  ArgTokens.push_back(Eof);
-else
-  ArgTokens.push_back(tok);
   }
+  // Push a tok::eof as the last element.
+  ArgTokens.push_back(Eof);
 
   auto MacroArgsDeleter = [](MacroArgs *M) { M->destroy(*PP); };
   std::unique_ptr MA(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158413: [Lex] Introduce Preprocessor::LexTokensUntilEOF()

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added inline comments.



Comment at: clang/lib/Lex/Preprocessor.cpp:1000
+  std::vector toks;
+  while (1) {
+Token tok;

aaron.ballman wrote:
> v.g.vassilev wrote:
> > aaron.ballman wrote:
> > > Hahnfeld wrote:
> > > > aaron.ballman wrote:
> > > > > I'd prefer not to assume the token stream has an EOF token (perhaps 
> > > > > the stream is one only being used to parse until the `eod` token 
> > > > > instead), so if we can turn this into a non-infinite loop, that would 
> > > > > make me more comfortable.
> > > > I'm not sure I understand entirely. Do you want something like
> > > > ```
> > > > tok.isOneOf(tok::unknown, tok::eof, tok::eod)
> > > > ```
> > > > instead of `tok.is(tok::eof)`? Can this happen at the level of the 
> > > > `Preprocessor`?
> > > I was thinking something more along the lines of:
> > > ```
> > > if (Tokens) {
> > >   for (Token Tok; !Tok.isOneOf(tok::eof, tok::eod); Lex(Tok))
> > > Tokens->push_back(Tok);
> > > }
> > > ```
> > > but I hadn't thought about `tok::unknown`; that might be a good one to 
> > > also include given that clangd operates on partial sources.
> > > 
> > I was wondering if we could somehow merge this routine with 
> > `Parser::SkipUntil` since they seem to be doing a very similar tasks.
> That could perhaps end up looking reasonable (they do similar tasks aside 
> from collecting the tokens that are being skipped). Do you need the interface 
> to be on `Preprocessor` or `Parser` though (or does it not really matter for 
> you)?
> `tok.isOneOf(tok::unknown, tok::eof, tok::eod)`

I implemented this check, let me know if this looks reasonable. The code you 
posted doesn't do what we need because we also want to lex if `Tokens` is 
`nullptr`, so the hierarchy must be an `if` inside the loop.
Given these additional token kinds, does `UntilEOF` still make sense or do we 
want another name? Note that I'll leave `repl_input_end` to 
https://reviews.llvm.org/D158415.

> I was wondering if we could somehow merge this routine with 
> `Parser::SkipUntil` since they seem to be doing a very similar tasks.

I'm not sure this makes sense, given that `Parser::SkipUntil` requires some 
knowledge about the structural input. At the very least, I'd prefer not to go 
into that direction for this change.


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

https://reviews.llvm.org/D158413

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

[PATCH] D158413: [Lex] Introduce Preprocessor::LexTokensUntilEOF()

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld updated this revision to Diff 556834.
Hahnfeld marked an inline comment as done.
Hahnfeld added a comment.

Handle all of `tok::unknown, tok::eof, tok::eod`.


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

https://reviews.llvm.org/D158413

Files:
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/Lex/Preprocessor.cpp
  clang/unittests/Analysis/MacroExpansionContextTest.cpp
  clang/unittests/Basic/SourceManagerTest.cpp
  clang/unittests/Lex/LexerTest.cpp
  clang/unittests/Lex/ModuleDeclStateTest.cpp
  clang/unittests/Lex/PPCallbacksTest.cpp
  clang/unittests/Lex/PPConditionalDirectiveRecordTest.cpp
  clang/unittests/Lex/PPDependencyDirectivesTest.cpp
  clang/unittests/Lex/PPMemoryAllocationsTest.cpp

Index: clang/unittests/Lex/PPMemoryAllocationsTest.cpp
===
--- clang/unittests/Lex/PPMemoryAllocationsTest.cpp
+++ clang/unittests/Lex/PPMemoryAllocationsTest.cpp
@@ -75,12 +75,7 @@
   PP.Initialize(*Target);
   PP.EnterMainSourceFile();
 
-  while (1) {
-Token tok;
-PP.Lex(tok);
-if (tok.is(tok::eof))
-  break;
-  }
+  PP.LexTokensUntilEOF();
 
   size_t NumAllocated = PP.getPreprocessorAllocator().getBytesAllocated();
   float BytesPerDefine = float(NumAllocated) / float(NumMacros);
Index: clang/unittests/Lex/PPDependencyDirectivesTest.cpp
===
--- clang/unittests/Lex/PPDependencyDirectivesTest.cpp
+++ clang/unittests/Lex/PPDependencyDirectivesTest.cpp
@@ -133,12 +133,7 @@
   SmallVector IncludedFiles;
   PP.addPPCallbacks(std::make_unique(PP, IncludedFiles));
   PP.EnterMainSourceFile();
-  while (true) {
-Token tok;
-PP.Lex(tok);
-if (tok.is(tok::eof))
-  break;
-  }
+  PP.LexTokensUntilEOF();
 
   SmallVector IncludedFilesSlash;
   for (StringRef IncludedFile : IncludedFiles)
Index: clang/unittests/Lex/PPConditionalDirectiveRecordTest.cpp
===
--- clang/unittests/Lex/PPConditionalDirectiveRecordTest.cpp
+++ clang/unittests/Lex/PPConditionalDirectiveRecordTest.cpp
@@ -87,13 +87,7 @@
   PP.EnterMainSourceFile();
 
   std::vector toks;
-  while (1) {
-Token tok;
-PP.Lex(tok);
-if (tok.is(tok::eof))
-  break;
-toks.push_back(tok);
-  }
+  PP.LexTokensUntilEOF();
 
   // Make sure we got the tokens that we expected.
   ASSERT_EQ(10U, toks.size());
Index: clang/unittests/Lex/PPCallbacksTest.cpp
===
--- clang/unittests/Lex/PPCallbacksTest.cpp
+++ clang/unittests/Lex/PPCallbacksTest.cpp
@@ -229,13 +229,7 @@
 
 // Lex source text.
 PP.EnterMainSourceFile();
-
-while (true) {
-  Token Tok;
-  PP.Lex(Tok);
-  if (Tok.is(tok::eof))
-break;
-}
+PP.LexTokensUntilEOF();
 
 // Callbacks have been executed at this point -- return filename range.
 return Callbacks;
@@ -259,13 +253,7 @@
 
 // Lex source text.
 PP.EnterMainSourceFile();
-
-while (true) {
-  Token Tok;
-  PP.Lex(Tok);
-  if (Tok.is(tok::eof))
-break;
-}
+PP.LexTokensUntilEOF();
 
 return Callbacks->Results;
   }
@@ -290,12 +278,7 @@
 
 // Lex source text.
 PP.EnterMainSourceFile();
-while (true) {
-  Token Tok;
-  PP.Lex(Tok);
-  if (Tok.is(tok::eof))
-break;
-}
+PP.LexTokensUntilEOF();
 
 return Callbacks->Marks;
   }
@@ -334,12 +317,7 @@
 
 // Lex source text.
 PP.EnterMainSourceFile();
-while (true) {
-  Token Tok;
-  PP.Lex(Tok);
-  if (Tok.is(tok::eof))
-break;
-}
+PP.LexTokensUntilEOF();
 
 PragmaOpenCLExtensionCallbacks::CallbackParameters RetVal = {
   Callbacks->Name,
@@ -477,12 +455,7 @@
 
   // Lex source text.
   PP.EnterMainSourceFile();
-  while (true) {
-Token Tok;
-PP.Lex(Tok);
-if (Tok.is(tok::eof))
-  break;
-  }
+  PP.LexTokensUntilEOF();
 
   ASSERT_EQ(1u, Callbacks->NumCalls);
   ASSERT_EQ(0u, DiagConsumer->getNumErrors());
Index: clang/unittests/Lex/ModuleDeclStateTest.cpp
===
--- clang/unittests/Lex/ModuleDeclStateTest.cpp
+++ clang/unittests/Lex/ModuleDeclStateTest.cpp
@@ -90,12 +90,7 @@
 PP.addPPCallbacks(std::move(C));
 PP.EnterMainSourceFile();
 
-while (1) {
-  Token tok;
-  PP.Lex(tok);
-  if (tok.is(tok::eof))
-break;
-}
+PP.LexTokensUntilEOF();
   }
 
   FileSystemOptions FileMgrOpts;
Index: clang/unittests/Lex/LexerTest.cpp
===
--- clang/unittests/Lex/LexerTest.cpp
+++ clang/unittests/Lex/LexerTest.cpp
@@ -74,13 +74,7 @@
 PP = CreatePP(Source, ModLoader);
 
 std::vector toks;
-while (1) {
-  Token tok;
-  PP->Lex(tok);
-  if (tok.is(tok::eof))
-break;
-  toks.push_back(tok);
-}
+

[PATCH] D158413: [Lex] Introduce Preprocessor::LexTokensUntilEOF()

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld marked 2 inline comments as done.
Hahnfeld added inline comments.



Comment at: clang/lib/Lex/Preprocessor.cpp:998
 
+std::vector Preprocessor::LexAll() {
+  std::vector toks;

aaron.ballman wrote:
> v.g.vassilev wrote:
> > Shouldn't we take the results as a `LexAll(std::vector )`?
> > 
> > Perhaps we should rename this interface to `LexTokensUntilEOF`?
> I think both suggestions make sense, especially given this doesn't return the 
> EOF token (so it doesn't really lex *all*). Though, given how many callers do 
> not want to store the tokens, perhaps it should take a `std::optional` or a 
> pointer to the vector so we can skip the storage work if it's unnecessary?
The new `LexTokensUntilEOF` now takes an optional `std::vector *Tokens = 
nullptr`.



Comment at: clang/lib/Lex/Preprocessor.cpp:1000
+  std::vector toks;
+  while (1) {
+Token tok;

aaron.ballman wrote:
> I'd prefer not to assume the token stream has an EOF token (perhaps the 
> stream is one only being used to parse until the `eod` token instead), so if 
> we can turn this into a non-infinite loop, that would make me more 
> comfortable.
I'm not sure I understand entirely. Do you want something like
```
tok.isOneOf(tok::unknown, tok::eof, tok::eod)
```
instead of `tok.is(tok::eof)`? Can this happen at the level of the 
`Preprocessor`?


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

https://reviews.llvm.org/D158413

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

[PATCH] D158413: [Lex] Introduce Preprocessor::LexTokensUntilEOF()

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld updated this revision to Diff 556791.
Hahnfeld retitled this revision from "[Lex] Introduce Preprocessor::LexAll()" 
to "[Lex] Introduce Preprocessor::LexTokensUntilEOF()".
Hahnfeld edited the summary of this revision.
Hahnfeld added a comment.

Rename to `Preprocessor::LexTokensUntilEOF()`


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

https://reviews.llvm.org/D158413

Files:
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/Lex/Preprocessor.cpp
  clang/unittests/Analysis/MacroExpansionContextTest.cpp
  clang/unittests/Basic/SourceManagerTest.cpp
  clang/unittests/Lex/LexerTest.cpp
  clang/unittests/Lex/ModuleDeclStateTest.cpp
  clang/unittests/Lex/PPCallbacksTest.cpp
  clang/unittests/Lex/PPConditionalDirectiveRecordTest.cpp
  clang/unittests/Lex/PPDependencyDirectivesTest.cpp
  clang/unittests/Lex/PPMemoryAllocationsTest.cpp

Index: clang/unittests/Lex/PPMemoryAllocationsTest.cpp
===
--- clang/unittests/Lex/PPMemoryAllocationsTest.cpp
+++ clang/unittests/Lex/PPMemoryAllocationsTest.cpp
@@ -75,12 +75,7 @@
   PP.Initialize(*Target);
   PP.EnterMainSourceFile();
 
-  while (1) {
-Token tok;
-PP.Lex(tok);
-if (tok.is(tok::eof))
-  break;
-  }
+  PP.LexTokensUntilEOF();
 
   size_t NumAllocated = PP.getPreprocessorAllocator().getBytesAllocated();
   float BytesPerDefine = float(NumAllocated) / float(NumMacros);
Index: clang/unittests/Lex/PPDependencyDirectivesTest.cpp
===
--- clang/unittests/Lex/PPDependencyDirectivesTest.cpp
+++ clang/unittests/Lex/PPDependencyDirectivesTest.cpp
@@ -133,12 +133,7 @@
   SmallVector IncludedFiles;
   PP.addPPCallbacks(std::make_unique(PP, IncludedFiles));
   PP.EnterMainSourceFile();
-  while (true) {
-Token tok;
-PP.Lex(tok);
-if (tok.is(tok::eof))
-  break;
-  }
+  PP.LexTokensUntilEOF();
 
   SmallVector IncludedFilesSlash;
   for (StringRef IncludedFile : IncludedFiles)
Index: clang/unittests/Lex/PPConditionalDirectiveRecordTest.cpp
===
--- clang/unittests/Lex/PPConditionalDirectiveRecordTest.cpp
+++ clang/unittests/Lex/PPConditionalDirectiveRecordTest.cpp
@@ -87,13 +87,7 @@
   PP.EnterMainSourceFile();
 
   std::vector toks;
-  while (1) {
-Token tok;
-PP.Lex(tok);
-if (tok.is(tok::eof))
-  break;
-toks.push_back(tok);
-  }
+  PP.LexTokensUntilEOF();
 
   // Make sure we got the tokens that we expected.
   ASSERT_EQ(10U, toks.size());
Index: clang/unittests/Lex/PPCallbacksTest.cpp
===
--- clang/unittests/Lex/PPCallbacksTest.cpp
+++ clang/unittests/Lex/PPCallbacksTest.cpp
@@ -229,13 +229,7 @@
 
 // Lex source text.
 PP.EnterMainSourceFile();
-
-while (true) {
-  Token Tok;
-  PP.Lex(Tok);
-  if (Tok.is(tok::eof))
-break;
-}
+PP.LexTokensUntilEOF();
 
 // Callbacks have been executed at this point -- return filename range.
 return Callbacks;
@@ -259,13 +253,7 @@
 
 // Lex source text.
 PP.EnterMainSourceFile();
-
-while (true) {
-  Token Tok;
-  PP.Lex(Tok);
-  if (Tok.is(tok::eof))
-break;
-}
+PP.LexTokensUntilEOF();
 
 return Callbacks->Results;
   }
@@ -290,12 +278,7 @@
 
 // Lex source text.
 PP.EnterMainSourceFile();
-while (true) {
-  Token Tok;
-  PP.Lex(Tok);
-  if (Tok.is(tok::eof))
-break;
-}
+PP.LexTokensUntilEOF();
 
 return Callbacks->Marks;
   }
@@ -334,12 +317,7 @@
 
 // Lex source text.
 PP.EnterMainSourceFile();
-while (true) {
-  Token Tok;
-  PP.Lex(Tok);
-  if (Tok.is(tok::eof))
-break;
-}
+PP.LexTokensUntilEOF();
 
 PragmaOpenCLExtensionCallbacks::CallbackParameters RetVal = {
   Callbacks->Name,
@@ -477,12 +455,7 @@
 
   // Lex source text.
   PP.EnterMainSourceFile();
-  while (true) {
-Token Tok;
-PP.Lex(Tok);
-if (Tok.is(tok::eof))
-  break;
-  }
+  PP.LexTokensUntilEOF();
 
   ASSERT_EQ(1u, Callbacks->NumCalls);
   ASSERT_EQ(0u, DiagConsumer->getNumErrors());
Index: clang/unittests/Lex/ModuleDeclStateTest.cpp
===
--- clang/unittests/Lex/ModuleDeclStateTest.cpp
+++ clang/unittests/Lex/ModuleDeclStateTest.cpp
@@ -90,12 +90,7 @@
 PP.addPPCallbacks(std::move(C));
 PP.EnterMainSourceFile();
 
-while (1) {
-  Token tok;
-  PP.Lex(tok);
-  if (tok.is(tok::eof))
-break;
-}
+PP.LexTokensUntilEOF();
   }
 
   FileSystemOptions FileMgrOpts;
Index: clang/unittests/Lex/LexerTest.cpp
===
--- clang/unittests/Lex/LexerTest.cpp
+++ clang/unittests/Lex/LexerTest.cpp
@@ -74,13 +74,7 @@
 PP = CreatePP(Source, ModLoader);
 
 std::vector toks;
-while (1) {
-   

[PATCH] D158415: [Lex] Handle repl_input_end in Preprocessor::LexAll()

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

This revision cannot land without https://reviews.llvm.org/D158413 and for that 
one I really want to have feedback from somebody working in that area...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158415

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

[PATCH] D158415: [Lex] Handle repl_input_end in Preprocessor::LexAll()

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

ping!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158415

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

[PATCH] D158414: [LexerTest] Use LexAll() in StringifyArgs

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

ping!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158414

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

[PATCH] D158413: [Lex] Introduce Preprocessor::LexAll()

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

ping!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158413

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

[PATCH] D158415: [Lex] Handle repl_input_end in Preprocessor::LexAll()

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

ping :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158415

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

[PATCH] D158414: [LexerTest] Use LexAll() in StringifyArgs

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

ping :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158414

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

[PATCH] D158413: [Lex] Introduce Preprocessor::LexAll()

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

ping :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158413

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

[PATCH] D157838: [clang-repl] Disambiguate declarations with private typedefs

[Jonas Hahnfeld via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6c274ba4108b: [clang-repl] Disambiguate declarations with 
private typedefs (authored by Hahnfeld).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157838

Files:
  clang/lib/Parse/ParseTentative.cpp
  clang/test/Interpreter/disambiguate-decl-stmt.cpp


Index: clang/test/Interpreter/disambiguate-decl-stmt.cpp
===
--- clang/test/Interpreter/disambiguate-decl-stmt.cpp
+++ clang/test/Interpreter/disambiguate-decl-stmt.cpp
@@ -1,8 +1,6 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -fincremental-extensions -std=c++20 %s
 // RUN: %clang_cc1 -fsyntax-only -DMS -fms-extensions -verify 
-fincremental-extensions -std=c++20 %s
 
-// expected-no-diagnostics
-
 extern "C" int printf(const char*,...);
 
 // Decls which are hard to disambiguate
@@ -47,6 +45,37 @@
 
 // Ctors
 
+// Private typedefs / using declarations
+class PrivateUsingMember { using T = int; T f(); };
+PrivateUsingMember::T PrivateUsingMember::f() { return 0; }
+
+class PrivateUsingVar { using T = int; static T i; };
+PrivateUsingVar::T PrivateUsingVar::i = 42;
+
+// The same with namespaces
+namespace PrivateUsingNamespace { class Member { using T = int; T f(); }; }
+PrivateUsingNamespace::Member::T PrivateUsingNamespace::Member::f() { return 
0; }
+
+namespace PrivateUsingNamespace { class Var { using T = int; static T i; }; }
+PrivateUsingNamespace::Var::T PrivateUsingNamespace::Var::i = 42;
+
+// The same with friend declarations
+class PrivateUsingFriendMember;
+class PrivateUsingFriendVar;
+class PrivateUsingFriend { friend class PrivateUsingFriendMember; friend class 
PrivateUsingFriendVar; using T = int; };
+class PrivateUsingFriendMember { PrivateUsingFriend::T f(); };
+PrivateUsingFriend::T PrivateUsingFriendMember::f() { return 0; }
+
+class PrivateUsingFriendVar { static PrivateUsingFriend::T i; };
+PrivateUsingFriend::T PrivateUsingFriendVar::i = 42;
+
+// The following should still diagnose (inspired by PR13642)
+// FIXME: Should not be diagnosed twice!
+class PR13642 { class Inner { public: static int i; }; };
+// expected-note@-1 2 {{implicitly declared private here}}
+PR13642::Inner::i = 5;
+// expected-error@-1 2 {{'Inner' is a private member of 'PR13642'}}
+
 // Deduction guide
 template struct A { A(); A(T); };
 A() -> A;
Index: clang/lib/Parse/ParseTentative.cpp
===
--- clang/lib/Parse/ParseTentative.cpp
+++ clang/lib/Parse/ParseTentative.cpp
@@ -83,6 +83,17 @@
   isDeductionGuide,
   DeclSpec::FriendSpecified::No))
 return true;
+} else if (SS.isNotEmpty()) {
+  // If the scope is not empty, it could alternatively be something 
like
+  // a typedef or using declaration. That declaration might be private
+  // in the global context, which would be diagnosed by calling into
+  // isCXXSimpleDeclaration, but may actually be fine in the context of
+  // member functions and static variable definitions. Check if the 
next
+  // token is also an identifier and assume a declaration.
+  // We cannot check if the scopes match because the declarations could
+  // involve namespaces and friend declarations.
+  if (NextToken().is(tok::identifier))
+return true;
 }
 break;
   }


Index: clang/test/Interpreter/disambiguate-decl-stmt.cpp
===
--- clang/test/Interpreter/disambiguate-decl-stmt.cpp
+++ clang/test/Interpreter/disambiguate-decl-stmt.cpp
@@ -1,8 +1,6 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -fincremental-extensions -std=c++20 %s
 // RUN: %clang_cc1 -fsyntax-only -DMS -fms-extensions -verify -fincremental-extensions -std=c++20 %s
 
-// expected-no-diagnostics
-
 extern "C" int printf(const char*,...);
 
 // Decls which are hard to disambiguate
@@ -47,6 +45,37 @@
 
 // Ctors
 
+// Private typedefs / using declarations
+class PrivateUsingMember { using T = int; T f(); };
+PrivateUsingMember::T PrivateUsingMember::f() { return 0; }
+
+class PrivateUsingVar { using T = int; static T i; };
+PrivateUsingVar::T PrivateUsingVar::i = 42;
+
+// The same with namespaces
+namespace PrivateUsingNamespace { class Member { using T = int; T f(); }; }
+PrivateUsingNamespace::Member::T PrivateUsingNamespace::Member::f() { return 0; }
+
+namespace PrivateUsingNamespace { class Var { using T = int; static T i; }; }
+PrivateUsingNamespace::Var::T PrivateUsingNamespace::Var::i = 42;
+
+// The same with friend declarations
+class PrivateUsingFriendMember;
+class PrivateUsingFriendVar;
+class PrivateUsingFriend { friend class PrivateUsingFriendMember; friend class PrivateUsingFriendVar; using T = int; 

[PATCH] D158415: [Lex] Handle repl_input_end in Preprocessor::LexAll()

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld created this revision.
Hahnfeld added reviewers: aaron.ballman, v.g.vassilev.
Herald added a project: All.
Hahnfeld requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This fixes many unit tests when trying to enable `IncrementalExtensions` by 
default for testing purposes.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158415

Files:
  clang/lib/Lex/Preprocessor.cpp


Index: clang/lib/Lex/Preprocessor.cpp
===
--- clang/lib/Lex/Preprocessor.cpp
+++ clang/lib/Lex/Preprocessor.cpp
@@ -1000,7 +1000,7 @@
   while (1) {
 Token tok;
 Lex(tok);
-if (tok.is(tok::eof))
+if (tok.isOneOf(tok::eof, tok::annot_repl_input_end))
   break;
 toks.push_back(tok);
   }


Index: clang/lib/Lex/Preprocessor.cpp
===
--- clang/lib/Lex/Preprocessor.cpp
+++ clang/lib/Lex/Preprocessor.cpp
@@ -1000,7 +1000,7 @@
   while (1) {
 Token tok;
 Lex(tok);
-if (tok.is(tok::eof))
+if (tok.isOneOf(tok::eof, tok::annot_repl_input_end))
   break;
 toks.push_back(tok);
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158414: [LexerTest] Use LexAll() in StringifyArgs

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld created this revision.
Hahnfeld added a reviewer: erichkeane.
Herald added a project: All.
Hahnfeld requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Rely on `Preprocessor::LexAll()` and just replace `tok::comma` by `tok::eof` 
and push a `tok::eof` as the last element.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158414

Files:
  clang/unittests/Lex/LexerTest.cpp


Index: clang/unittests/Lex/LexerTest.cpp
===
--- clang/unittests/Lex/LexerTest.cpp
+++ clang/unittests/Lex/LexerTest.cpp
@@ -447,19 +447,15 @@
 
   Token Eof;
   Eof.setKind(tok::eof);
-  std::vector ArgTokens;
-  while (1) {
-Token tok;
-PP->Lex(tok);
-if (tok.is(tok::eof)) {
-  ArgTokens.push_back(Eof);
-  break;
+  std::vector ArgTokens = PP->LexAll();
+  // Replace all tok::comma with tok::eof for stringification.
+  for (auto  : ArgTokens) {
+if (tok.is(tok::comma)) {
+  tok = Eof;
 }
-if (tok.is(tok::comma))
-  ArgTokens.push_back(Eof);
-else
-  ArgTokens.push_back(tok);
   }
+  // Push a tok::eof as the last element.
+  ArgTokens.push_back(Eof);
 
   auto MacroArgsDeleter = [](MacroArgs *M) { M->destroy(*PP); };
   std::unique_ptr MA(


Index: clang/unittests/Lex/LexerTest.cpp
===
--- clang/unittests/Lex/LexerTest.cpp
+++ clang/unittests/Lex/LexerTest.cpp
@@ -447,19 +447,15 @@
 
   Token Eof;
   Eof.setKind(tok::eof);
-  std::vector ArgTokens;
-  while (1) {
-Token tok;
-PP->Lex(tok);
-if (tok.is(tok::eof)) {
-  ArgTokens.push_back(Eof);
-  break;
+  std::vector ArgTokens = PP->LexAll();
+  // Replace all tok::comma with tok::eof for stringification.
+  for (auto  : ArgTokens) {
+if (tok.is(tok::comma)) {
+  tok = Eof;
 }
-if (tok.is(tok::comma))
-  ArgTokens.push_back(Eof);
-else
-  ArgTokens.push_back(tok);
   }
+  // Push a tok::eof as the last element.
+  ArgTokens.push_back(Eof);
 
   auto MacroArgsDeleter = [](MacroArgs *M) { M->destroy(*PP); };
   std::unique_ptr MA(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158413: [Lex] Introduce Preprocessor::LexAll()

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld created this revision.
Hahnfeld added reviewers: hokein, aaron.ballman.
Herald added subscribers: kbarton, nemanjai.
Herald added a project: All.
Hahnfeld requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This new method repeatedly calls `Lex()` until end of file is reached and 
returns a `std::vector` of `Token`s. Use it in Clang's unit tests to avoid 
quite some code duplication.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158413

Files:
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/Lex/Preprocessor.cpp
  clang/unittests/Analysis/MacroExpansionContextTest.cpp
  clang/unittests/Basic/SourceManagerTest.cpp
  clang/unittests/Lex/LexerTest.cpp
  clang/unittests/Lex/ModuleDeclStateTest.cpp
  clang/unittests/Lex/PPCallbacksTest.cpp
  clang/unittests/Lex/PPConditionalDirectiveRecordTest.cpp
  clang/unittests/Lex/PPDependencyDirectivesTest.cpp
  clang/unittests/Lex/PPMemoryAllocationsTest.cpp

Index: clang/unittests/Lex/PPMemoryAllocationsTest.cpp
===
--- clang/unittests/Lex/PPMemoryAllocationsTest.cpp
+++ clang/unittests/Lex/PPMemoryAllocationsTest.cpp
@@ -75,12 +75,7 @@
   PP.Initialize(*Target);
   PP.EnterMainSourceFile();
 
-  while (1) {
-Token tok;
-PP.Lex(tok);
-if (tok.is(tok::eof))
-  break;
-  }
+  (void)PP.LexAll();
 
   size_t NumAllocated = PP.getPreprocessorAllocator().getBytesAllocated();
   float BytesPerDefine = float(NumAllocated) / float(NumMacros);
Index: clang/unittests/Lex/PPDependencyDirectivesTest.cpp
===
--- clang/unittests/Lex/PPDependencyDirectivesTest.cpp
+++ clang/unittests/Lex/PPDependencyDirectivesTest.cpp
@@ -133,12 +133,7 @@
   SmallVector IncludedFiles;
   PP.addPPCallbacks(std::make_unique(PP, IncludedFiles));
   PP.EnterMainSourceFile();
-  while (true) {
-Token tok;
-PP.Lex(tok);
-if (tok.is(tok::eof))
-  break;
-  }
+  (void)PP.LexAll();
 
   SmallVector ExpectedIncludes{
   "main.c", "./head1.h", "./head2.h", "./head2.h", "./head3.h", "./head3.h",
Index: clang/unittests/Lex/PPConditionalDirectiveRecordTest.cpp
===
--- clang/unittests/Lex/PPConditionalDirectiveRecordTest.cpp
+++ clang/unittests/Lex/PPConditionalDirectiveRecordTest.cpp
@@ -86,14 +86,7 @@
   PP.addPPCallbacks(std::unique_ptr(PPRec));
   PP.EnterMainSourceFile();
 
-  std::vector toks;
-  while (1) {
-Token tok;
-PP.Lex(tok);
-if (tok.is(tok::eof))
-  break;
-toks.push_back(tok);
-  }
+  std::vector toks = PP.LexAll();
 
   // Make sure we got the tokens that we expected.
   ASSERT_EQ(10U, toks.size());
Index: clang/unittests/Lex/PPCallbacksTest.cpp
===
--- clang/unittests/Lex/PPCallbacksTest.cpp
+++ clang/unittests/Lex/PPCallbacksTest.cpp
@@ -229,13 +229,7 @@
 
 // Lex source text.
 PP.EnterMainSourceFile();
-
-while (true) {
-  Token Tok;
-  PP.Lex(Tok);
-  if (Tok.is(tok::eof))
-break;
-}
+(void)PP.LexAll();
 
 // Callbacks have been executed at this point -- return filename range.
 return Callbacks;
@@ -259,13 +253,7 @@
 
 // Lex source text.
 PP.EnterMainSourceFile();
-
-while (true) {
-  Token Tok;
-  PP.Lex(Tok);
-  if (Tok.is(tok::eof))
-break;
-}
+(void)PP.LexAll();
 
 return Callbacks->Results;
   }
@@ -290,12 +278,7 @@
 
 // Lex source text.
 PP.EnterMainSourceFile();
-while (true) {
-  Token Tok;
-  PP.Lex(Tok);
-  if (Tok.is(tok::eof))
-break;
-}
+(void)PP.LexAll();
 
 return Callbacks->Marks;
   }
@@ -334,12 +317,7 @@
 
 // Lex source text.
 PP.EnterMainSourceFile();
-while (true) {
-  Token Tok;
-  PP.Lex(Tok);
-  if (Tok.is(tok::eof))
-break;
-}
+(void)PP.LexAll();
 
 PragmaOpenCLExtensionCallbacks::CallbackParameters RetVal = {
   Callbacks->Name,
@@ -477,12 +455,7 @@
 
   // Lex source text.
   PP.EnterMainSourceFile();
-  while (true) {
-Token Tok;
-PP.Lex(Tok);
-if (Tok.is(tok::eof))
-  break;
-  }
+  (void)PP.LexAll();
 
   ASSERT_EQ(1u, Callbacks->NumCalls);
   ASSERT_EQ(0u, DiagConsumer->getNumErrors());
Index: clang/unittests/Lex/ModuleDeclStateTest.cpp
===
--- clang/unittests/Lex/ModuleDeclStateTest.cpp
+++ clang/unittests/Lex/ModuleDeclStateTest.cpp
@@ -90,12 +90,7 @@
 PP.addPPCallbacks(std::move(C));
 PP.EnterMainSourceFile();
 
-while (1) {
-  Token tok;
-  PP.Lex(tok);
-  if (tok.is(tok::eof))
-break;
-}
+(void)PP.LexAll();
   }
 
   FileSystemOptions FileMgrOpts;
Index: clang/unittests/Lex/LexerTest.cpp
===
--- 

[PATCH] D157838: [clang-repl] Disambiguate declarations with private typedefs

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added inline comments.



Comment at: clang/lib/Parse/ParseTentative.cpp:94
+  // involve namespaces and friend declarations.
+  if (NextToken().is(tok::identifier))
+return true;

v.g.vassilev wrote:
> I am wondering what is the false positive rate of this change? That is, if we 
> enable incremental parsing by default (in a local build) and then run all 
> tests which do not produce diagnostics. 
With this patch and enabling `IncrementalExtensions` by default, it's at least 
not worse than current `main`: For all of `check-clang`, I see a number of 
failing unit tests that start hoarding some memory and are killed with 
`std::bad_alloc` or are stuck using 100% CPU. Those we probably want to debug 
independently of this change...



Comment at: clang/test/Interpreter/disambiguate-decl-stmt.cpp:74
+// expected-error@-1 2 {{'Inner' is a private member of 'PR13642'}}
+
 // Deduction guide

v.g.vassilev wrote:
> Could we move the diagnostic-producing cases in a separate file?
We could, but I'm actually not a big fan because it moves the expected failing 
test away from the related disambiguation tests above...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157838

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

[PATCH] D156537: [CodeGen] Keep track of eagerly emitted globals

[Jonas Hahnfeld via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc861d32d7c27: [CodeGen] Keep track of eagerly emitted 
globals (authored by Hahnfeld).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156537

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/Interpreter/inline-virtual.cpp


Index: clang/test/Interpreter/inline-virtual.cpp
===
--- /dev/null
+++ clang/test/Interpreter/inline-virtual.cpp
@@ -0,0 +1,24 @@
+// REQUIRES: host-supports-jit
+// UNSUPPORTED: system-aix
+//
+// We disable RTTI to avoid problems on Windows for non-RTTI builds of LLVM
+// where the JIT cannot find ??_7type_info@@6B@.
+// RUN: cat %s | clang-repl -Xcc -fno-rtti | FileCheck %s
+// RUN: cat %s | clang-repl -Xcc -fno-rtti -Xcc -O2 | FileCheck %s
+
+extern "C" int printf(const char *, ...);
+
+struct A { int a; A(int a) : a(a) {} virtual ~A(); };
+
+// Then define the virtual destructor as inline out-of-line, in a separate
+// PartialTranslationUnit.
+inline A::~A() { printf("~A(%d)\n", a); }
+
+// Create one instance with new and delete it.
+A *a1 = new A(1);
+delete a1;
+// CHECK: ~A(1)
+
+// Also create one global that will be auto-destructed.
+A a2(2);
+// CHECK: ~A(2)
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -3658,6 +3658,7 @@
   if (MustBeEmitted(Global) && MayBeEmittedEagerly(Global)) {
 // Emit the definition if it can't be deferred.
 EmitGlobalDefinition(GD);
+addEmittedDeferredDecl(GD);
 return;
   }
 


Index: clang/test/Interpreter/inline-virtual.cpp
===
--- /dev/null
+++ clang/test/Interpreter/inline-virtual.cpp
@@ -0,0 +1,24 @@
+// REQUIRES: host-supports-jit
+// UNSUPPORTED: system-aix
+//
+// We disable RTTI to avoid problems on Windows for non-RTTI builds of LLVM
+// where the JIT cannot find ??_7type_info@@6B@.
+// RUN: cat %s | clang-repl -Xcc -fno-rtti | FileCheck %s
+// RUN: cat %s | clang-repl -Xcc -fno-rtti -Xcc -O2 | FileCheck %s
+
+extern "C" int printf(const char *, ...);
+
+struct A { int a; A(int a) : a(a) {} virtual ~A(); };
+
+// Then define the virtual destructor as inline out-of-line, in a separate
+// PartialTranslationUnit.
+inline A::~A() { printf("~A(%d)\n", a); }
+
+// Create one instance with new and delete it.
+A *a1 = new A(1);
+delete a1;
+// CHECK: ~A(1)
+
+// Also create one global that will be auto-destructed.
+A a2(2);
+// CHECK: ~A(2)
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -3658,6 +3658,7 @@
   if (MustBeEmitted(Global) && MayBeEmittedEagerly(Global)) {
 // Emit the definition if it can't be deferred.
 EmitGlobalDefinition(GD);
+addEmittedDeferredDecl(GD);
 return;
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156537: [CodeGen] Keep track of eagerly emitted globals

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

I reverted the change yesterday because the test wasn't passing on Windows, for 
example 
https://lab.llvm.org/buildbot/#/builders/216/builds/25769/steps/7/logs/FAIL__Clang__inline-virtual_cpp.
 The problem is that the JIT cannot find `??_7type_info@@6B@` related to RTTI. 
I see that we have some special setup in ROOT/Cling to make this work, but it's 
not fully clear to me how this could be applied to `clang-repl` if we are 
building LLVM without RTTI. For now, I propose to disable RTTI for this test 
(it's not needed) and will re-commit once the build bots are back to a more 
healthy green state :)


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

https://reviews.llvm.org/D156537

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

[PATCH] D158252: Fix regression of D157680

[Jonas Hahnfeld via Phabricator via cfe-commits]

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

LG. IMHO such fixes to the tests can be committed without review :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158252

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

[PATCH] D156537: [CodeGen] Keep track of eagerly emitted globals

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld updated this revision to Diff 551401.
Hahnfeld added a comment.

Disable RTTI to (hopefully) avoid problems on Windows.


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

https://reviews.llvm.org/D156537

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/Interpreter/inline-virtual.cpp


Index: clang/test/Interpreter/inline-virtual.cpp
===
--- /dev/null
+++ clang/test/Interpreter/inline-virtual.cpp
@@ -0,0 +1,24 @@
+// REQUIRES: host-supports-jit
+// UNSUPPORTED: system-aix
+//
+// We disable RTTI to avoid problems on Windows for non-RTTI builds of LLVM
+// where the JIT cannot find ??_7type_info@@6B@.
+// RUN: cat %s | clang-repl -Xcc -fno-rtti | FileCheck %s
+// RUN: cat %s | clang-repl -Xcc -fno-rtti -Xcc -O2 | FileCheck %s
+
+extern "C" int printf(const char *, ...);
+
+struct A { int a; A(int a) : a(a) {} virtual ~A(); };
+
+// Then define the virtual destructor as inline out-of-line, in a separate
+// PartialTranslationUnit.
+inline A::~A() { printf("~A(%d)\n", a); }
+
+// Create one instance with new and delete it.
+A *a1 = new A(1);
+delete a1;
+// CHECK: ~A(1)
+
+// Also create one global that will be auto-destructed.
+A a2(2);
+// CHECK: ~A(2)
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -3658,6 +3658,7 @@
   if (MustBeEmitted(Global) && MayBeEmittedEagerly(Global)) {
 // Emit the definition if it can't be deferred.
 EmitGlobalDefinition(GD);
+addEmittedDeferredDecl(GD);
 return;
   }
 


Index: clang/test/Interpreter/inline-virtual.cpp
===
--- /dev/null
+++ clang/test/Interpreter/inline-virtual.cpp
@@ -0,0 +1,24 @@
+// REQUIRES: host-supports-jit
+// UNSUPPORTED: system-aix
+//
+// We disable RTTI to avoid problems on Windows for non-RTTI builds of LLVM
+// where the JIT cannot find ??_7type_info@@6B@.
+// RUN: cat %s | clang-repl -Xcc -fno-rtti | FileCheck %s
+// RUN: cat %s | clang-repl -Xcc -fno-rtti -Xcc -O2 | FileCheck %s
+
+extern "C" int printf(const char *, ...);
+
+struct A { int a; A(int a) : a(a) {} virtual ~A(); };
+
+// Then define the virtual destructor as inline out-of-line, in a separate
+// PartialTranslationUnit.
+inline A::~A() { printf("~A(%d)\n", a); }
+
+// Create one instance with new and delete it.
+A *a1 = new A(1);
+delete a1;
+// CHECK: ~A(1)
+
+// Also create one global that will be auto-destructed.
+A a2(2);
+// CHECK: ~A(2)
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -3658,6 +3658,7 @@
   if (MustBeEmitted(Global) && MayBeEmittedEagerly(Global)) {
 // Emit the definition if it can't be deferred.
 EmitGlobalDefinition(GD);
+addEmittedDeferredDecl(GD);
 return;
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157750: Properly handle -fsplit-machine-functions for fatbinary compilation

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added inline comments.



Comment at: clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c:16
+// causes a warning.
+// RUN:   %clang --target=x86_64-unknown-linux-gnu -nogpulib -nogpuinc \
+// RUN: --cuda-gpu-arch=sm_70 -x cuda -fsplit-machine-functions -S %s 2>&1 
\

steelannelida wrote:
> Unfortunately these commands fail in our sandbox due to writing files to 
> readonly directories:
> 
>  `unable to open output file 'fsplit-machine-functions-with-cuda-nvptx.s': 
> 'Permission denied'`
> 
> Could you please specify the output files via `%t` substitutions? I'm not 
> sure how to do this for cuda compilation.
IIRC the file names are generated based on what you specify with `-o`. Did you 
try this already?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157750

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

[PATCH] D156537: [CodeGen] Keep track of eagerly emitted globals

[Jonas Hahnfeld via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf8dadefd4afc: [CodeGen] Keep track of eagerly emitted 
globals (authored by Hahnfeld).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156537

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/Interpreter/inline-virtual.cpp


Index: clang/test/Interpreter/inline-virtual.cpp
===
--- /dev/null
+++ clang/test/Interpreter/inline-virtual.cpp
@@ -0,0 +1,21 @@
+// REQUIRES: host-supports-jit
+// UNSUPPORTED: system-aix
+// RUN: cat %s | clang-repl | FileCheck %s
+// RUN: cat %s | clang-repl -Xcc -O2 | FileCheck %s
+
+extern "C" int printf(const char *, ...);
+
+struct A { int a; A(int a) : a(a) {} virtual ~A(); };
+
+// Then define the virtual destructor as inline out-of-line, in a separate
+// PartialTranslationUnit.
+inline A::~A() { printf("~A(%d)\n", a); }
+
+// Create one instance with new and delete it.
+A *a1 = new A(1);
+delete a1;
+// CHECK: ~A(1)
+
+// Also create one global that will be auto-destructed.
+A a2(2);
+// CHECK: ~A(2)
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -3658,6 +3658,7 @@
   if (MustBeEmitted(Global) && MayBeEmittedEagerly(Global)) {
 // Emit the definition if it can't be deferred.
 EmitGlobalDefinition(GD);
+addEmittedDeferredDecl(GD);
 return;
   }
 


Index: clang/test/Interpreter/inline-virtual.cpp
===
--- /dev/null
+++ clang/test/Interpreter/inline-virtual.cpp
@@ -0,0 +1,21 @@
+// REQUIRES: host-supports-jit
+// UNSUPPORTED: system-aix
+// RUN: cat %s | clang-repl | FileCheck %s
+// RUN: cat %s | clang-repl -Xcc -O2 | FileCheck %s
+
+extern "C" int printf(const char *, ...);
+
+struct A { int a; A(int a) : a(a) {} virtual ~A(); };
+
+// Then define the virtual destructor as inline out-of-line, in a separate
+// PartialTranslationUnit.
+inline A::~A() { printf("~A(%d)\n", a); }
+
+// Create one instance with new and delete it.
+A *a1 = new A(1);
+delete a1;
+// CHECK: ~A(1)
+
+// Also create one global that will be auto-destructed.
+A a2(2);
+// CHECK: ~A(2)
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -3658,6 +3658,7 @@
   if (MustBeEmitted(Global) && MayBeEmittedEagerly(Global)) {
 // Emit the definition if it can't be deferred.
 EmitGlobalDefinition(GD);
+addEmittedDeferredDecl(GD);
 return;
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157379: [CodeGen] Restrict addEmittedDeferredDecl to incremental extensions

[Jonas Hahnfeld via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd43a3d634696: [CodeGen] Restrict addEmittedDeferredDecl to 
incremental extensions (authored by Hahnfeld).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157379

Files:
  clang/lib/CodeGen/CodeGenModule.h


Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -361,6 +361,10 @@
   llvm::DenseMap EmittedDeferredDecls;
 
   void addEmittedDeferredDecl(GlobalDecl GD) {
+// Reemission is only needed in incremental mode.
+if (!Context.getLangOpts().IncrementalExtensions)
+  return;
+
 // Assume a linkage by default that does not need reemission.
 auto L = llvm::GlobalValue::ExternalLinkage;
 if (llvm::isa(GD.getDecl()))


Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -361,6 +361,10 @@
   llvm::DenseMap EmittedDeferredDecls;
 
   void addEmittedDeferredDecl(GlobalDecl GD) {
+// Reemission is only needed in incremental mode.
+if (!Context.getLangOpts().IncrementalExtensions)
+  return;
+
 // Assume a linkage by default that does not need reemission.
 auto L = llvm::GlobalValue::ExternalLinkage;
 if (llvm::isa(GD.getDecl()))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156897: [CodeGen] Clean up access to EmittedDeferredDecls, NFCI.

[Jonas Hahnfeld via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb719e410781c: [CodeGen] Clean up access to 
EmittedDeferredDecls, NFCI. (authored by Hahnfeld).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156897

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h


Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -361,10 +361,15 @@
   llvm::DenseMap EmittedDeferredDecls;
 
   void addEmittedDeferredDecl(GlobalDecl GD) {
-if (!llvm::isa(GD.getDecl()))
-  return;
-llvm::GlobalVariable::LinkageTypes L = getFunctionLinkage(GD);
-if (llvm::GlobalValue::isLinkOnceLinkage(L) ||
+// Assume a linkage by default that does not need reemission.
+auto L = llvm::GlobalValue::ExternalLinkage;
+if (llvm::isa(GD.getDecl()))
+  L = getFunctionLinkage(GD);
+else if (auto *VD = llvm::dyn_cast(GD.getDecl()))
+  L = getLLVMLinkageVarDefinition(VD);
+
+if (llvm::GlobalValue::isInternalLinkage(L) ||
+llvm::GlobalValue::isLinkOnceLinkage(L) ||
 llvm::GlobalValue::isWeakLinkage(L)) {
   EmittedDeferredDecls[getMangledName(GD)] = GD;
 }
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -3677,7 +3677,6 @@
 // The value must be emitted, but cannot be emitted eagerly.
 assert(!MayBeEmittedEagerly(Global));
 addDeferredDeclToEmit(GD);
-EmittedDeferredDecls[MangledName] = GD;
   } else {
 // Otherwise, remember that we saw a deferred decl with this name.  The
 // first use of the mangled name will cause it to move into
@@ -4417,7 +4416,6 @@
   // DeferredDeclsToEmit list, and remove it from DeferredDecls (since we
   // don't need it anymore).
   addDeferredDeclToEmit(DDI->second);
-  EmittedDeferredDecls[DDI->first] = DDI->second;
   DeferredDecls.erase(DDI);
 
   // Otherwise, there are cases we have to worry about where we're
@@ -4677,7 +4675,6 @@
 // Move the potentially referenced deferred decl to the DeferredDeclsToEmit
 // list, and remove it from DeferredDecls (since we don't need it anymore).
 addDeferredDeclToEmit(DDI->second);
-EmittedDeferredDecls[DDI->first] = DDI->second;
 DeferredDecls.erase(DDI);
   }
 


Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -361,10 +361,15 @@
   llvm::DenseMap EmittedDeferredDecls;
 
   void addEmittedDeferredDecl(GlobalDecl GD) {
-if (!llvm::isa(GD.getDecl()))
-  return;
-llvm::GlobalVariable::LinkageTypes L = getFunctionLinkage(GD);
-if (llvm::GlobalValue::isLinkOnceLinkage(L) ||
+// Assume a linkage by default that does not need reemission.
+auto L = llvm::GlobalValue::ExternalLinkage;
+if (llvm::isa(GD.getDecl()))
+  L = getFunctionLinkage(GD);
+else if (auto *VD = llvm::dyn_cast(GD.getDecl()))
+  L = getLLVMLinkageVarDefinition(VD);
+
+if (llvm::GlobalValue::isInternalLinkage(L) ||
+llvm::GlobalValue::isLinkOnceLinkage(L) ||
 llvm::GlobalValue::isWeakLinkage(L)) {
   EmittedDeferredDecls[getMangledName(GD)] = GD;
 }
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -3677,7 +3677,6 @@
 // The value must be emitted, but cannot be emitted eagerly.
 assert(!MayBeEmittedEagerly(Global));
 addDeferredDeclToEmit(GD);
-EmittedDeferredDecls[MangledName] = GD;
   } else {
 // Otherwise, remember that we saw a deferred decl with this name.  The
 // first use of the mangled name will cause it to move into
@@ -4417,7 +4416,6 @@
   // DeferredDeclsToEmit list, and remove it from DeferredDecls (since we
   // don't need it anymore).
   addDeferredDeclToEmit(DDI->second);
-  EmittedDeferredDecls[DDI->first] = DDI->second;
   DeferredDecls.erase(DDI);
 
   // Otherwise, there are cases we have to worry about where we're
@@ -4677,7 +4675,6 @@
 // Move the potentially referenced deferred decl to the DeferredDeclsToEmit
 // list, and remove it from DeferredDecls (since we don't need it anymore).
 addDeferredDeclToEmit(DDI->second);
-EmittedDeferredDecls[DDI->first] = DDI->second;
 DeferredDecls.erase(DDI);
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157750: Properly handle -fsplit-machine-functions for fatbinary compilation

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

I dsabled two tests without `{arm,aarch64}-registered-target` in 
rGeeac4321c517ee8afc30ebe62c5b1778efc1173d 
; two 
post-commit comments inline




Comment at: llvm/test/CodeGen/Generic/machine-function-splitter.ll:18
+; MFS_ON: Machine Function Splitter Transformation
+; MFS_ON_NO: warning: -fsplit-machine-functions is not valid for
+;; Check that MFS is not on for non-X86 targets.

shouldn't this be `MFS_ON-NOT`?



Comment at: llvm/test/CodeGen/Generic/machine-function-splitter.ll:21
+; MFS_OFF: warning: -fsplit-machine-functions is not valid for
+; MFS_OFF_NO: Machine Function Splitter Transformation
 




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157750

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

[PATCH] D156891: [CodeGen] Remove Constant arguments from linkage functions, NFCI.

[Jonas Hahnfeld via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG2f3fe3ed97bc: [CodeGen] Remove Constant arguments from 
linkage functions, NFCI. (authored by Hahnfeld).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156891

Files:
  clang/lib/CodeGen/CGCXXABI.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprConstant.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/MicrosoftCXXABI.cpp

Index: clang/lib/CodeGen/MicrosoftCXXABI.cpp
===
--- clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -1379,8 +1379,7 @@
   case Dtor_Base:
 // The base destructor most closely tracks the user-declared constructor, so
 // we delegate back to the normal declarator case.
-return CGM.getLLVMLinkageForDeclarator(Dtor, Linkage,
-   /*IsConstantVariable=*/false);
+return CGM.getLLVMLinkageForDeclarator(Dtor, Linkage);
   case Dtor_Complete:
 // The complete destructor is like an inline function, but it may be
 // imported and therefore must be exported as well. This requires changing
Index: clang/lib/CodeGen/ItaniumCXXABI.cpp
===
--- clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -2832,7 +2832,7 @@
 static llvm::GlobalValue::LinkageTypes
 getThreadLocalWrapperLinkage(const VarDecl *VD, CodeGen::CodeGenModule ) {
   llvm::GlobalValue::LinkageTypes VarLinkage =
-  CGM.getLLVMLinkageVarDefinition(VD, /*IsConstant=*/false);
+  CGM.getLLVMLinkageVarDefinition(VD);
 
   // For internal linkage variables, we don't need an external or weak wrapper.
   if (llvm::GlobalValue::isLocalLinkage(VarLinkage))
Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -1314,12 +1314,11 @@
 
   /// Returns LLVM linkage for a declarator.
   llvm::GlobalValue::LinkageTypes
-  getLLVMLinkageForDeclarator(const DeclaratorDecl *D, GVALinkage Linkage,
-  bool IsConstantVariable);
+  getLLVMLinkageForDeclarator(const DeclaratorDecl *D, GVALinkage Linkage);
 
   /// Returns LLVM linkage for a declarator.
   llvm::GlobalValue::LinkageTypes
-  getLLVMLinkageVarDefinition(const VarDecl *VD, bool IsConstant);
+  getLLVMLinkageVarDefinition(const VarDecl *VD);
 
   /// Emit all the global annotations.
   void EmitGlobalAnnotations();
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -1979,7 +1979,7 @@
 return llvm::GlobalValue::InternalLinkage;
   }
 
-  return getLLVMLinkageForDeclarator(D, Linkage, /*IsConstantVariable=*/false);
+  return getLLVMLinkageForDeclarator(D, Linkage);
 }
 
 llvm::ConstantInt *CodeGenModule::CreateCrossDsoCfiTypeId(llvm::Metadata *MD) {
@@ -5220,8 +5220,7 @@
 AddGlobalAnnotations(D, GV);
 
   // Set the llvm linkage type as appropriate.
-  llvm::GlobalValue::LinkageTypes Linkage =
-  getLLVMLinkageVarDefinition(D, GV->isConstant());
+  llvm::GlobalValue::LinkageTypes Linkage = getLLVMLinkageVarDefinition(D);
 
   // CUDA B.2.1 "The __device__ qualifier declares a variable that resides on
   // the device. [...]"
@@ -5414,8 +5413,9 @@
   return false;
 }
 
-llvm::GlobalValue::LinkageTypes CodeGenModule::getLLVMLinkageForDeclarator(
-const DeclaratorDecl *D, GVALinkage Linkage, bool IsConstantVariable) {
+llvm::GlobalValue::LinkageTypes
+CodeGenModule::getLLVMLinkageForDeclarator(const DeclaratorDecl *D,
+   GVALinkage Linkage) {
   if (Linkage == GVA_Internal)
 return llvm::Function::InternalLinkage;
 
@@ -5485,10 +5485,10 @@
   return llvm::GlobalVariable::ExternalLinkage;
 }
 
-llvm::GlobalValue::LinkageTypes CodeGenModule::getLLVMLinkageVarDefinition(
-const VarDecl *VD, bool IsConstant) {
+llvm::GlobalValue::LinkageTypes
+CodeGenModule::getLLVMLinkageVarDefinition(const VarDecl *VD) {
   GVALinkage Linkage = getContext().GetGVALinkageForVariable(VD);
-  return getLLVMLinkageForDeclarator(VD, Linkage, IsConstant);
+  return getLLVMLinkageForDeclarator(VD, Linkage);
 }
 
 /// Replace the uses of a function that was declared with a non-proto type.
@@ -5700,7 +5700,7 @@
 Aliasee = GetOrCreateLLVMGlobal(AA->getAliasee(), DeclTy, LangAS::Default,
 /*D=*/nullptr);
 if (const auto *VD = dyn_cast(GD.getDecl()))
-  LT = 

[PATCH] D156891: [CodeGen] Remove Constant arguments from linkage functions, NFCI.

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

ping - this one is the only prerequisite left for three of my patches that were 
accepted yesterday...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156891

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

[PATCH] D157480: [clang-repl] Disambiguate global namespace identifiers

[Jonas Hahnfeld via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGba475a4a3440: [clang-repl] Disambiguate global namespace 
identifiers (authored by Hahnfeld).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157480

Files:
  clang/lib/Parse/ParseTentative.cpp
  clang/test/Interpreter/disambiguate-decl-stmt.cpp


Index: clang/test/Interpreter/disambiguate-decl-stmt.cpp
===
--- clang/test/Interpreter/disambiguate-decl-stmt.cpp
+++ clang/test/Interpreter/disambiguate-decl-stmt.cpp
@@ -38,6 +38,10 @@
 Dtor1::~Dtor1() { printf("Dtor1\n"); }
 Dtor1 d1;
 
+struct Dtor2 { ~Dtor2(); };
+::Dtor2::~Dtor2() { printf("Dtor2\n"); }
+Dtor2 d2;
+
 struct ANestedDtor { struct A1 { struct A2 { ~A2(); }; }; };
 ANestedDtor::A1::A2::~A2() { printf("Dtor A::A1::A2::~A2\n"); }
 
Index: clang/lib/Parse/ParseTentative.cpp
===
--- clang/lib/Parse/ParseTentative.cpp
+++ clang/lib/Parse/ParseTentative.cpp
@@ -62,6 +62,7 @@
   case tok::kw_static_assert:
   case tok::kw__Static_assert:
 return true;
+  case tok::coloncolon:
   case tok::identifier: {
 if (DisambiguatingWithExpression) {
   RevertingTentativeParsingAction TPA(*this);


Index: clang/test/Interpreter/disambiguate-decl-stmt.cpp
===
--- clang/test/Interpreter/disambiguate-decl-stmt.cpp
+++ clang/test/Interpreter/disambiguate-decl-stmt.cpp
@@ -38,6 +38,10 @@
 Dtor1::~Dtor1() { printf("Dtor1\n"); }
 Dtor1 d1;
 
+struct Dtor2 { ~Dtor2(); };
+::Dtor2::~Dtor2() { printf("Dtor2\n"); }
+Dtor2 d2;
+
 struct ANestedDtor { struct A1 { struct A2 { ~A2(); }; }; };
 ANestedDtor::A1::A2::~A2() { printf("Dtor A::A1::A2::~A2\n"); }
 
Index: clang/lib/Parse/ParseTentative.cpp
===
--- clang/lib/Parse/ParseTentative.cpp
+++ clang/lib/Parse/ParseTentative.cpp
@@ -62,6 +62,7 @@
   case tok::kw_static_assert:
   case tok::kw__Static_assert:
 return true;
+  case tok::coloncolon:
   case tok::identifier: {
 if (DisambiguatingWithExpression) {
   RevertingTentativeParsingAction TPA(*this);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157838: [clang-repl] Disambiguate declarations with private typedefs

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld created this revision.
Hahnfeld added reviewers: v.g.vassilev, rsmith.
Herald added a project: All.
Hahnfeld requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Member functions and static variable definitions may use typedefs that
are private in the global context, but fine in the class context.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157838

Files:
  clang/lib/Parse/ParseTentative.cpp
  clang/test/Interpreter/disambiguate-decl-stmt.cpp


Index: clang/test/Interpreter/disambiguate-decl-stmt.cpp
===
--- clang/test/Interpreter/disambiguate-decl-stmt.cpp
+++ clang/test/Interpreter/disambiguate-decl-stmt.cpp
@@ -1,8 +1,6 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -fincremental-extensions -std=c++20 %s
 // RUN: %clang_cc1 -fsyntax-only -DMS -fms-extensions -verify 
-fincremental-extensions -std=c++20 %s
 
-// expected-no-diagnostics
-
 extern "C" int printf(const char*,...);
 
 // Decls which are hard to disambiguate
@@ -43,6 +41,37 @@
 
 // Ctors
 
+// Private typedefs / using declarations
+class PrivateUsingMember { using T = int; T f(); };
+PrivateUsingMember::T PrivateUsingMember::f() { return 0; }
+
+class PrivateUsingVar { using T = int; static T i; };
+PrivateUsingVar::T PrivateUsingVar::i = 42;
+
+// The same with namespaces
+namespace PrivateUsingNamespace { class Member { using T = int; T f(); }; }
+PrivateUsingNamespace::Member::T PrivateUsingNamespace::Member::f() { return 
0; }
+
+namespace PrivateUsingNamespace { class Var { using T = int; static T i; }; }
+PrivateUsingNamespace::Var::T PrivateUsingNamespace::Var::i = 42;
+
+// The same with friend declarations
+class PrivateUsingFriendMember;
+class PrivateUsingFriendVar;
+class PrivateUsingFriend { friend class PrivateUsingFriendMember; friend class 
PrivateUsingFriendVar; using T = int; };
+class PrivateUsingFriendMember { PrivateUsingFriend::T f(); };
+PrivateUsingFriend::T PrivateUsingFriendMember::f() { return 0; }
+
+class PrivateUsingFriendVar { static PrivateUsingFriend::T i; };
+PrivateUsingFriend::T PrivateUsingFriendVar::i = 42;
+
+// The following should still diagnose (inspired by PR13642)
+// FIXME: Should not be diagnosed twice!
+class PR13642 { class Inner { public: static int i; }; };
+// expected-note@-1 2 {{implicitly declared private here}}
+PR13642::Inner::i = 5;
+// expected-error@-1 2 {{'Inner' is a private member of 'PR13642'}}
+
 // Deduction guide
 template struct A { A(); A(T); };
 A() -> A;
Index: clang/lib/Parse/ParseTentative.cpp
===
--- clang/lib/Parse/ParseTentative.cpp
+++ clang/lib/Parse/ParseTentative.cpp
@@ -82,6 +82,17 @@
   isDeductionGuide,
   DeclSpec::FriendSpecified::No))
 return true;
+} else if (SS.isNotEmpty()) {
+  // If the scope is not empty, it could alternatively be something 
like
+  // a typedef or using declaration. That declaration might be private
+  // in the global context, which would be diagnosed by calling into
+  // isCXXSimpleDeclaration, but may actually be fine in the context of
+  // member functions and static variable definitions. Check if the 
next
+  // token is also an identifier and assume a declaration.
+  // We cannot check if the scopes match because the declarations could
+  // involve namespaces and friend declarations.
+  if (NextToken().is(tok::identifier))
+return true;
 }
 break;
   }


Index: clang/test/Interpreter/disambiguate-decl-stmt.cpp
===
--- clang/test/Interpreter/disambiguate-decl-stmt.cpp
+++ clang/test/Interpreter/disambiguate-decl-stmt.cpp
@@ -1,8 +1,6 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -fincremental-extensions -std=c++20 %s
 // RUN: %clang_cc1 -fsyntax-only -DMS -fms-extensions -verify -fincremental-extensions -std=c++20 %s
 
-// expected-no-diagnostics
-
 extern "C" int printf(const char*,...);
 
 // Decls which are hard to disambiguate
@@ -43,6 +41,37 @@
 
 // Ctors
 
+// Private typedefs / using declarations
+class PrivateUsingMember { using T = int; T f(); };
+PrivateUsingMember::T PrivateUsingMember::f() { return 0; }
+
+class PrivateUsingVar { using T = int; static T i; };
+PrivateUsingVar::T PrivateUsingVar::i = 42;
+
+// The same with namespaces
+namespace PrivateUsingNamespace { class Member { using T = int; T f(); }; }
+PrivateUsingNamespace::Member::T PrivateUsingNamespace::Member::f() { return 0; }
+
+namespace PrivateUsingNamespace { class Var { using T = int; static T i; }; }
+PrivateUsingNamespace::Var::T PrivateUsingNamespace::Var::i = 42;
+
+// The same with friend declarations
+class PrivateUsingFriendMember;
+class PrivateUsingFriendVar;
+class 

[PATCH] D156897: [CodeGen] Clean up access to EmittedDeferredDecls, NFCI.

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

gentle ping :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156897

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

[PATCH] D156891: [CodeGen] Remove Constant arguments from linkage functions, NFCI.

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

gentle ping :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156891

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

[PATCH] D157477: [clang-repl] Additional test for disambiguation of templates

[Jonas Hahnfeld via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGda555f750ab2: [clang-repl] Additional test for 
disambiguation of templates (authored by Hahnfeld).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157477

Files:
  clang/test/Interpreter/disambiguate-decl-stmt.cpp


Index: clang/test/Interpreter/disambiguate-decl-stmt.cpp
===
--- clang/test/Interpreter/disambiguate-decl-stmt.cpp
+++ clang/test/Interpreter/disambiguate-decl-stmt.cpp
@@ -11,6 +11,10 @@
 namespace ns1 { template void tmplt(T &) {}}
 int arg_tmplt = 12; ns1::tmplt(arg_tmplt);
 
+namespace ns2 { template  struct S {}; }
+namespace ns3 { struct A { public: using S = int; }; }
+namespace ns3 { A::S f(A::S a); }
+
 // ParseStatementOrDeclaration returns multiple statements.
 #ifdef MS
 int g_bFlag = 1;


Index: clang/test/Interpreter/disambiguate-decl-stmt.cpp
===
--- clang/test/Interpreter/disambiguate-decl-stmt.cpp
+++ clang/test/Interpreter/disambiguate-decl-stmt.cpp
@@ -11,6 +11,10 @@
 namespace ns1 { template void tmplt(T &) {}}
 int arg_tmplt = 12; ns1::tmplt(arg_tmplt);
 
+namespace ns2 { template  struct S {}; }
+namespace ns3 { struct A { public: using S = int; }; }
+namespace ns3 { A::S f(A::S a); }
+
 // ParseStatementOrDeclaration returns multiple statements.
 #ifdef MS
 int g_bFlag = 1;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157480: [clang-repl] Disambiguate global namespace identifiers

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added inline comments.



Comment at: clang/test/Interpreter/global-namespace-disambiguate.cpp:6
+struct A { ~A(); };
+::A::~A() {}
+

v.g.vassilev wrote:
> Can we add this test to `disambiguate-decl-stmt.cpp` instead where we track 
> these things?
done


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

https://reviews.llvm.org/D157480

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

[PATCH] D157480: [clang-repl] Disambiguate global namespace identifiers

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld updated this revision to Diff 548562.
Hahnfeld marked an inline comment as done.

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

https://reviews.llvm.org/D157480

Files:
  clang/lib/Parse/ParseTentative.cpp
  clang/test/Interpreter/disambiguate-decl-stmt.cpp


Index: clang/test/Interpreter/disambiguate-decl-stmt.cpp
===
--- clang/test/Interpreter/disambiguate-decl-stmt.cpp
+++ clang/test/Interpreter/disambiguate-decl-stmt.cpp
@@ -34,6 +34,10 @@
 Dtor1::~Dtor1() { printf("Dtor1\n"); }
 Dtor1 d1;
 
+struct Dtor2 { ~Dtor2(); };
+::Dtor2::~Dtor2() { printf("Dtor2\n"); }
+Dtor2 d2;
+
 struct ANestedDtor { struct A1 { struct A2 { ~A2(); }; }; };
 ANestedDtor::A1::A2::~A2() { printf("Dtor A::A1::A2::~A2\n"); }
 
Index: clang/lib/Parse/ParseTentative.cpp
===
--- clang/lib/Parse/ParseTentative.cpp
+++ clang/lib/Parse/ParseTentative.cpp
@@ -62,6 +62,7 @@
   case tok::kw_static_assert:
   case tok::kw__Static_assert:
 return true;
+  case tok::coloncolon:
   case tok::identifier: {
 if (DisambiguatingWithExpression) {
   RevertingTentativeParsingAction TPA(*this);


Index: clang/test/Interpreter/disambiguate-decl-stmt.cpp
===
--- clang/test/Interpreter/disambiguate-decl-stmt.cpp
+++ clang/test/Interpreter/disambiguate-decl-stmt.cpp
@@ -34,6 +34,10 @@
 Dtor1::~Dtor1() { printf("Dtor1\n"); }
 Dtor1 d1;
 
+struct Dtor2 { ~Dtor2(); };
+::Dtor2::~Dtor2() { printf("Dtor2\n"); }
+Dtor2 d2;
+
 struct ANestedDtor { struct A1 { struct A2 { ~A2(); }; }; };
 ANestedDtor::A1::A2::~A2() { printf("Dtor A::A1::A2::~A2\n"); }
 
Index: clang/lib/Parse/ParseTentative.cpp
===
--- clang/lib/Parse/ParseTentative.cpp
+++ clang/lib/Parse/ParseTentative.cpp
@@ -62,6 +62,7 @@
   case tok::kw_static_assert:
   case tok::kw__Static_assert:
 return true;
+  case tok::coloncolon:
   case tok::identifier: {
 if (DisambiguatingWithExpression) {
   RevertingTentativeParsingAction TPA(*this);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157477: [clang-repl] Additional test for disambiguation of templates

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added inline comments.



Comment at: clang/test/Interpreter/namespace-template-disambiguate.cpp:7
+namespace NS2 { struct A { public: using S = int; }; }
+namespace NS2 { A::S f(A::S a); }
+

v.g.vassilev wrote:
> It might be a good idea to add this test to `disambiguate-decl-stmt.cpp` 
> where we track these things.
done


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

https://reviews.llvm.org/D157477

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

[PATCH] D157477: [clang-repl] Additional test for disambiguation of templates

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld updated this revision to Diff 548558.
Hahnfeld retitled this revision from "[clang-repl] Add test for disambiguation 
of templates" to "[clang-repl] Additional test for disambiguation of templates".
Hahnfeld edited the summary of this revision.

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

https://reviews.llvm.org/D157477

Files:
  clang/test/Interpreter/disambiguate-decl-stmt.cpp


Index: clang/test/Interpreter/disambiguate-decl-stmt.cpp
===
--- clang/test/Interpreter/disambiguate-decl-stmt.cpp
+++ clang/test/Interpreter/disambiguate-decl-stmt.cpp
@@ -11,6 +11,10 @@
 namespace ns1 { template void tmplt(T &) {}}
 int arg_tmplt = 12; ns1::tmplt(arg_tmplt);
 
+namespace ns2 { template  struct S {}; }
+namespace ns3 { struct A { public: using S = int; }; }
+namespace ns3 { A::S f(A::S a); }
+
 // ParseStatementOrDeclaration returns multiple statements.
 #ifdef MS
 int g_bFlag = 1;


Index: clang/test/Interpreter/disambiguate-decl-stmt.cpp
===
--- clang/test/Interpreter/disambiguate-decl-stmt.cpp
+++ clang/test/Interpreter/disambiguate-decl-stmt.cpp
@@ -11,6 +11,10 @@
 namespace ns1 { template void tmplt(T &) {}}
 int arg_tmplt = 12; ns1::tmplt(arg_tmplt);
 
+namespace ns2 { template  struct S {}; }
+namespace ns3 { struct A { public: using S = int; }; }
+namespace ns3 { A::S f(A::S a); }
+
 // ParseStatementOrDeclaration returns multiple statements.
 #ifdef MS
 int g_bFlag = 1;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157480: [clang-repl] Disambiguate global namespace identifiers

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld created this revision.
Hahnfeld added reviewers: v.g.vassilev, aaron.ballman.
Herald added a project: All.
Hahnfeld requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

A double colon starts an identifier name in the global namespace and must be 
tentatively parsed as such.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157480

Files:
  clang/lib/Parse/ParseTentative.cpp
  clang/test/Interpreter/global-namespace-disambiguate.cpp


Index: clang/test/Interpreter/global-namespace-disambiguate.cpp
===
--- /dev/null
+++ clang/test/Interpreter/global-namespace-disambiguate.cpp
@@ -0,0 +1,8 @@
+// UNSUPPORTED: system-aix
+
+// RUN: cat %s | clang-repl 2>&1 | FileCheck %s
+
+struct A { ~A(); };
+::A::~A() {}
+
+// CHECK-NOT: error
Index: clang/lib/Parse/ParseTentative.cpp
===
--- clang/lib/Parse/ParseTentative.cpp
+++ clang/lib/Parse/ParseTentative.cpp
@@ -62,6 +62,7 @@
   case tok::kw_static_assert:
   case tok::kw__Static_assert:
 return true;
+  case tok::coloncolon:
   case tok::identifier: {
 if (DisambiguatingWithExpression) {
   RevertingTentativeParsingAction TPA(*this);


Index: clang/test/Interpreter/global-namespace-disambiguate.cpp
===
--- /dev/null
+++ clang/test/Interpreter/global-namespace-disambiguate.cpp
@@ -0,0 +1,8 @@
+// UNSUPPORTED: system-aix
+
+// RUN: cat %s | clang-repl 2>&1 | FileCheck %s
+
+struct A { ~A(); };
+::A::~A() {}
+
+// CHECK-NOT: error
Index: clang/lib/Parse/ParseTentative.cpp
===
--- clang/lib/Parse/ParseTentative.cpp
+++ clang/lib/Parse/ParseTentative.cpp
@@ -62,6 +62,7 @@
   case tok::kw_static_assert:
   case tok::kw__Static_assert:
 return true;
+  case tok::coloncolon:
   case tok::identifier: {
 if (DisambiguatingWithExpression) {
   RevertingTentativeParsingAction TPA(*this);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157477: [clang-repl] Add test for disambiguation of templates

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld created this revision.
Hahnfeld added a reviewer: v.g.vassilev.
Herald added a project: All.
Hahnfeld requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This test case was fixed in commit rG2c4620c1 
 
("Consider the scope spec in template lookups for deduction guides."), but it 
is worth having a dedicated test case for `clang-repl`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157477

Files:
  clang/test/Interpreter/namespace-template-disambiguate.cpp


Index: clang/test/Interpreter/namespace-template-disambiguate.cpp
===
--- /dev/null
+++ clang/test/Interpreter/namespace-template-disambiguate.cpp
@@ -0,0 +1,9 @@
+// UNSUPPORTED: system-aix
+
+// RUN: cat %s | clang-repl 2>&1 | FileCheck %s
+
+namespace NS1 { template  struct S {}; }
+namespace NS2 { struct A { public: using S = int; }; }
+namespace NS2 { A::S f(A::S a); }
+
+// CHECK-NOT: error


Index: clang/test/Interpreter/namespace-template-disambiguate.cpp
===
--- /dev/null
+++ clang/test/Interpreter/namespace-template-disambiguate.cpp
@@ -0,0 +1,9 @@
+// UNSUPPORTED: system-aix
+
+// RUN: cat %s | clang-repl 2>&1 | FileCheck %s
+
+namespace NS1 { template  struct S {}; }
+namespace NS2 { struct A { public: using S = int; }; }
+namespace NS2 { A::S f(A::S a); }
+
+// CHECK-NOT: error
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156537: [CodeGen] Keep track of eagerly emitted globals

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

In D156537#4554413 , @v.g.vassilev 
wrote:

> In D156537#4554052 , @Hahnfeld 
> wrote:
>
>> @v.g.vassilev do we have a means to detect this case? In D156897 
>> , I'm refactoring all accesses to 
>> `EmittedDeferredDecl` to go via `addEmittedDeferredDecl`, so we should just 
>> add an early return when not emitting for a REPL.
>
> We have `getLangOpts().IncrementalExtensions`. Does that work?

Perfect! After D157379 , calling 
`addEmittedDeferredDecl` should immediately return in case we are not in 
incremental mode.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156537

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

[PATCH] D157379: [CodeGen] Restrict addEmittedDeferredDecl to incremental extensions

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld created this revision.
Hahnfeld added reviewers: rjmccall, v.g.vassilev, junaire.
Herald added a project: All.
Hahnfeld requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Reemission is only needed in incremental mode. With this early return,
we avoid overhead from `addEmittedDeferredDecl` in non-incremental mode.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157379

Files:
  clang/lib/CodeGen/CodeGenModule.h


Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -361,6 +361,10 @@
   llvm::DenseMap EmittedDeferredDecls;
 
   void addEmittedDeferredDecl(GlobalDecl GD) {
+// Reemission is only needed in incremental mode.
+if (!Context.getLangOpts().IncrementalExtensions)
+  return;
+
 // Assume a linkage by default that does not need reemission.
 auto L = llvm::GlobalValue::ExternalLinkage;
 if (llvm::isa(GD.getDecl()))


Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -361,6 +361,10 @@
   llvm::DenseMap EmittedDeferredDecls;
 
   void addEmittedDeferredDecl(GlobalDecl GD) {
+// Reemission is only needed in incremental mode.
+if (!Context.getLangOpts().IncrementalExtensions)
+  return;
+
 // Assume a linkage by default that does not need reemission.
 auto L = llvm::GlobalValue::ExternalLinkage;
 if (llvm::isa(GD.getDecl()))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156363: [Driver] -###: exit with code 1 if hasErrorOccurred

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

`test/Driver/openmp-offload-jit.c` was still failing for me with CUDA 
installed, so I pushed rGcb3136b0d3596f5c3984e4fb49359e2a1a28a547 
 to add an 
explicit `--cuda-path` and make it return exit code 0 on (hopefully) all 
systems.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156363

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

[PATCH] D156537: [CodeGen] Keep track of eagerly emitted globals

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

@v.g.vassilev do we have a means to detect this case? In D156897 
, I'm refactoring all accesses to 
`EmittedDeferredDecl` to go via `addEmittedDeferredDecl`, so we should just add 
an early return when not emitting for a REPL.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156537

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

[PATCH] D156897: [CodeGen] Clean up access to EmittedDeferredDecls, NFCI.

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld created this revision.
Hahnfeld added reviewers: rjmccall, v.g.vassilev, junaire.
Herald added a project: All.
Hahnfeld requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

`GlobalDecl`s should only be added to `EmittedDeferredDecl` if they
need reemission. This is checked in `addEmittedDeferredDecl`, which
is called via `addDeferredDeclToEmit`. Extend these checks to also
handle `VarDecls` (for lambdas, as tested in `Interpreter/lambda.cpp`)
and remove the direct access of `EmittedDeferredDecls` in `EmitGlobal`
that may actually end up duplicating `FunctionDecl`s.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D156897

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h


Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -361,10 +361,15 @@
   llvm::DenseMap EmittedDeferredDecls;
 
   void addEmittedDeferredDecl(GlobalDecl GD) {
-if (!llvm::isa(GD.getDecl()))
-  return;
-llvm::GlobalVariable::LinkageTypes L = getFunctionLinkage(GD);
-if (llvm::GlobalValue::isLinkOnceLinkage(L) ||
+// Assume a linkage by default that does not need reemission.
+auto L = llvm::GlobalValue::ExternalLinkage;
+if (llvm::isa(GD.getDecl()))
+  L = getFunctionLinkage(GD);
+else if (auto *VD = llvm::dyn_cast(GD.getDecl()))
+  L = getLLVMLinkageVarDefinition(VD);
+
+if (llvm::GlobalValue::isInternalLinkage(L) ||
+llvm::GlobalValue::isLinkOnceLinkage(L) ||
 llvm::GlobalValue::isWeakLinkage(L)) {
   EmittedDeferredDecls[getMangledName(GD)] = GD;
 }
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -3664,7 +3664,6 @@
 // The value must be emitted, but cannot be emitted eagerly.
 assert(!MayBeEmittedEagerly(Global));
 addDeferredDeclToEmit(GD);
-EmittedDeferredDecls[MangledName] = GD;
   } else {
 // Otherwise, remember that we saw a deferred decl with this name.  The
 // first use of the mangled name will cause it to move into
@@ -4404,7 +4403,6 @@
   // DeferredDeclsToEmit list, and remove it from DeferredDecls (since we
   // don't need it anymore).
   addDeferredDeclToEmit(DDI->second);
-  EmittedDeferredDecls[DDI->first] = DDI->second;
   DeferredDecls.erase(DDI);
 
   // Otherwise, there are cases we have to worry about where we're
@@ -4685,7 +4683,6 @@
 // Move the potentially referenced deferred decl to the DeferredDeclsToEmit
 // list, and remove it from DeferredDecls (since we don't need it anymore).
 addDeferredDeclToEmit(DDI->second);
-EmittedDeferredDecls[DDI->first] = DDI->second;
 DeferredDecls.erase(DDI);
   }
 


Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -361,10 +361,15 @@
   llvm::DenseMap EmittedDeferredDecls;
 
   void addEmittedDeferredDecl(GlobalDecl GD) {
-if (!llvm::isa(GD.getDecl()))
-  return;
-llvm::GlobalVariable::LinkageTypes L = getFunctionLinkage(GD);
-if (llvm::GlobalValue::isLinkOnceLinkage(L) ||
+// Assume a linkage by default that does not need reemission.
+auto L = llvm::GlobalValue::ExternalLinkage;
+if (llvm::isa(GD.getDecl()))
+  L = getFunctionLinkage(GD);
+else if (auto *VD = llvm::dyn_cast(GD.getDecl()))
+  L = getLLVMLinkageVarDefinition(VD);
+
+if (llvm::GlobalValue::isInternalLinkage(L) ||
+llvm::GlobalValue::isLinkOnceLinkage(L) ||
 llvm::GlobalValue::isWeakLinkage(L)) {
   EmittedDeferredDecls[getMangledName(GD)] = GD;
 }
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -3664,7 +3664,6 @@
 // The value must be emitted, but cannot be emitted eagerly.
 assert(!MayBeEmittedEagerly(Global));
 addDeferredDeclToEmit(GD);
-EmittedDeferredDecls[MangledName] = GD;
   } else {
 // Otherwise, remember that we saw a deferred decl with this name.  The
 // first use of the mangled name will cause it to move into
@@ -4404,7 +4403,6 @@
   // DeferredDeclsToEmit list, and remove it from DeferredDecls (since we
   // don't need it anymore).
   addDeferredDeclToEmit(DDI->second);
-  EmittedDeferredDecls[DDI->first] = DDI->second;
   DeferredDecls.erase(DDI);
 
   // Otherwise, there are cases we have to worry about where we're
@@ -4685,7 +4683,6 @@
 // Move the potentially referenced deferred decl to the DeferredDeclsToEmit
 // list, and remove it from DeferredDecls (since we don't 

[PATCH] D156891: [CodeGen] Remove Constant arguments from linkage functions, NFCI.

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld created this revision.
Hahnfeld added reviewers: rjmccall, efriedma, asl.
Herald added a project: All.
Hahnfeld requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This was unused since commit rGdd2362a8ba 
 last year.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D156891

Files:
  clang/lib/CodeGen/CGCXXABI.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprConstant.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/MicrosoftCXXABI.cpp

Index: clang/lib/CodeGen/MicrosoftCXXABI.cpp
===
--- clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -1379,8 +1379,7 @@
   case Dtor_Base:
 // The base destructor most closely tracks the user-declared constructor, so
 // we delegate back to the normal declarator case.
-return CGM.getLLVMLinkageForDeclarator(Dtor, Linkage,
-   /*IsConstantVariable=*/false);
+return CGM.getLLVMLinkageForDeclarator(Dtor, Linkage);
   case Dtor_Complete:
 // The complete destructor is like an inline function, but it may be
 // imported and therefore must be exported as well. This requires changing
Index: clang/lib/CodeGen/ItaniumCXXABI.cpp
===
--- clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -2839,7 +2839,7 @@
 static llvm::GlobalValue::LinkageTypes
 getThreadLocalWrapperLinkage(const VarDecl *VD, CodeGen::CodeGenModule ) {
   llvm::GlobalValue::LinkageTypes VarLinkage =
-  CGM.getLLVMLinkageVarDefinition(VD, /*IsConstant=*/false);
+  CGM.getLLVMLinkageVarDefinition(VD);
 
   // For internal linkage variables, we don't need an external or weak wrapper.
   if (llvm::GlobalValue::isLocalLinkage(VarLinkage))
Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -1316,12 +1316,11 @@
 
   /// Returns LLVM linkage for a declarator.
   llvm::GlobalValue::LinkageTypes
-  getLLVMLinkageForDeclarator(const DeclaratorDecl *D, GVALinkage Linkage,
-  bool IsConstantVariable);
+  getLLVMLinkageForDeclarator(const DeclaratorDecl *D, GVALinkage Linkage);
 
   /// Returns LLVM linkage for a declarator.
   llvm::GlobalValue::LinkageTypes
-  getLLVMLinkageVarDefinition(const VarDecl *VD, bool IsConstant);
+  getLLVMLinkageVarDefinition(const VarDecl *VD);
 
   /// Emit all the global annotations.
   void EmitGlobalAnnotations();
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -1974,7 +1974,7 @@
 return llvm::GlobalValue::InternalLinkage;
   }
 
-  return getLLVMLinkageForDeclarator(D, Linkage, /*IsConstantVariable=*/false);
+  return getLLVMLinkageForDeclarator(D, Linkage);
 }
 
 llvm::ConstantInt *CodeGenModule::CreateCrossDsoCfiTypeId(llvm::Metadata *MD) {
@@ -5228,8 +5228,7 @@
 AddGlobalAnnotations(D, GV);
 
   // Set the llvm linkage type as appropriate.
-  llvm::GlobalValue::LinkageTypes Linkage =
-  getLLVMLinkageVarDefinition(D, GV->isConstant());
+  llvm::GlobalValue::LinkageTypes Linkage = getLLVMLinkageVarDefinition(D);
 
   // CUDA B.2.1 "The __device__ qualifier declares a variable that resides on
   // the device. [...]"
@@ -5422,8 +5421,9 @@
   return false;
 }
 
-llvm::GlobalValue::LinkageTypes CodeGenModule::getLLVMLinkageForDeclarator(
-const DeclaratorDecl *D, GVALinkage Linkage, bool IsConstantVariable) {
+llvm::GlobalValue::LinkageTypes
+CodeGenModule::getLLVMLinkageForDeclarator(const DeclaratorDecl *D,
+   GVALinkage Linkage) {
   if (Linkage == GVA_Internal)
 return llvm::Function::InternalLinkage;
 
@@ -5493,10 +5493,10 @@
   return llvm::GlobalVariable::ExternalLinkage;
 }
 
-llvm::GlobalValue::LinkageTypes CodeGenModule::getLLVMLinkageVarDefinition(
-const VarDecl *VD, bool IsConstant) {
+llvm::GlobalValue::LinkageTypes
+CodeGenModule::getLLVMLinkageVarDefinition(const VarDecl *VD) {
   GVALinkage Linkage = getContext().GetGVALinkageForVariable(VD);
-  return getLLVMLinkageForDeclarator(VD, Linkage, IsConstant);
+  return getLLVMLinkageForDeclarator(VD, Linkage);
 }
 
 /// Replace the uses of a function that was declared with a non-proto type.
@@ -5708,7 +5708,7 @@
 Aliasee = GetOrCreateLLVMGlobal(AA->getAliasee(), DeclTy, LangAS::Default,
 /*D=*/nullptr);
 if (const auto *VD = dyn_cast(GD.getDecl()))

[PATCH] D156363: [Driver] -###: exit with code 1 if hasErrorOccurred

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

Hi, after this change I have three failing tests:

  Failed Tests (3):
Clang :: Driver/lto.cu
Clang :: Driver/openmp-offload-gpu.c
Clang :: Driver/openmp-offload-jit.c

I have CUDA installed on my system, not sure if a bot is testing this 
configuration... Could you elaborate why the `clang` invocations in these tests 
are now expected to return a non-zero code?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156363

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

[PATCH] D156806: [Modules] Add test for merging of template member parent

[Jonas Hahnfeld via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG99b54743106b: [Modules] Add test for merging of template 
member parent (authored by Hahnfeld).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156806

Files:
  clang/test/Modules/merge-template-members-parent.cpp


Index: clang/test/Modules/merge-template-members-parent.cpp
===
--- /dev/null
+++ clang/test/Modules/merge-template-members-parent.cpp
@@ -0,0 +1,64 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -emit-obj -fmodules -fimplicit-module-maps 
-fmodules-cache-path=%t %t/merge.cpp -o %t/merge.o
+
+//--- V.h
+#ifndef V_H
+#define V_H
+template 
+struct V {
+  ~V() {}
+};
+#endif
+
+//--- A.h
+#include "V.h"
+
+void A(const V );
+
+//--- B.h
+#include "V.h"
+
+inline V B() {
+  return {};
+}
+
+//--- C.h
+#include "V.h"
+
+#include "A.h"
+
+class C {
+public:
+  C(const V ) {
+V v2;
+  }
+};
+
+C GetC() {
+   return {{}};
+}
+
+// This include *MUST* come last.
+#include "B.h"
+
+//--- module.modulemap
+module "V" { header "V.h" export * }
+module "A" { header "A.h" export * }
+module "B" { header "B.h" export * }
+module "C" { header "C.h" export * }
+
+//--- merge.cpp
+#include "C.h"
+
+template 
+C GetC_main() {
+   return {{}};
+}
+
+void f() {
+   GetC_main();
+   GetC();
+}


Index: clang/test/Modules/merge-template-members-parent.cpp
===
--- /dev/null
+++ clang/test/Modules/merge-template-members-parent.cpp
@@ -0,0 +1,64 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -emit-obj -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %t/merge.cpp -o %t/merge.o
+
+//--- V.h
+#ifndef V_H
+#define V_H
+template 
+struct V {
+  ~V() {}
+};
+#endif
+
+//--- A.h
+#include "V.h"
+
+void A(const V );
+
+//--- B.h
+#include "V.h"
+
+inline V B() {
+  return {};
+}
+
+//--- C.h
+#include "V.h"
+
+#include "A.h"
+
+class C {
+public:
+  C(const V ) {
+V v2;
+  }
+};
+
+C GetC() {
+   return {{}};
+}
+
+// This include *MUST* come last.
+#include "B.h"
+
+//--- module.modulemap
+module "V" { header "V.h" export * }
+module "A" { header "A.h" export * }
+module "B" { header "B.h" export * }
+module "C" { header "C.h" export * }
+
+//--- merge.cpp
+#include "C.h"
+
+template 
+C GetC_main() {
+   return {{}};
+}
+
+void f() {
+   GetC_main();
+   GetC();
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D137787: [CodeGen] Relax assertion on generating destructor call

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

The problem is back, but has been fixed upstream by 
https://reviews.llvm.org/rG61c7a9140becb19c5b1bc644e54452c6f782f5d5. I managed 
to reduce a test case triggering the assert touched in this revision, see 
https://reviews.llvm.org/D156806


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137787

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

[PATCH] D156806: [Modules] Add test for merging of template member parent

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld created this revision.
Hahnfeld added reviewers: rsmith, ChuanqiXu.
Herald added a project: All.
Hahnfeld requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This is a reduced test case originally meant to be addressed by 
https://reviews.llvm.org/D137787. It was recently fixed by commit rG61c7a9140b 
 ("Commit 
to a primary definition for a class when we load its first member."), noting 
the difficulty to come up with a reduced test case. This setup with four 
modules seems to fail consistently before the fix mentioned above.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D156806

Files:
  clang/test/Modules/merge-template-members-parent.cpp


Index: clang/test/Modules/merge-template-members-parent.cpp
===
--- /dev/null
+++ clang/test/Modules/merge-template-members-parent.cpp
@@ -0,0 +1,64 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -emit-obj -fmodules -fimplicit-module-maps 
-fmodules-cache-path=%t %t/merge.cpp -o %t/merge.o
+
+//--- V.h
+#ifndef V_H
+#define V_H
+template 
+struct V {
+  ~V() {}
+};
+#endif
+
+//--- A.h
+#include "V.h"
+
+void A(const V );
+
+//--- B.h
+#include "V.h"
+
+inline V B() {
+  return {};
+}
+
+//--- C.h
+#include "V.h"
+
+#include "A.h"
+
+class C {
+public:
+  C(const V ) {
+V v2;
+  }
+};
+
+C GetC() {
+   return {{}};
+}
+
+// This include *MUST* come last.
+#include "B.h"
+
+//--- module.modulemap
+module "V" { header "V.h" export * }
+module "A" { header "A.h" export * }
+module "B" { header "B.h" export * }
+module "C" { header "C.h" export * }
+
+//--- merge.cpp
+#include "C.h"
+
+template 
+C GetC_main() {
+   return {{}};
+}
+
+void f() {
+   GetC_main();
+   GetC();
+}


Index: clang/test/Modules/merge-template-members-parent.cpp
===
--- /dev/null
+++ clang/test/Modules/merge-template-members-parent.cpp
@@ -0,0 +1,64 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -emit-obj -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %t/merge.cpp -o %t/merge.o
+
+//--- V.h
+#ifndef V_H
+#define V_H
+template 
+struct V {
+  ~V() {}
+};
+#endif
+
+//--- A.h
+#include "V.h"
+
+void A(const V );
+
+//--- B.h
+#include "V.h"
+
+inline V B() {
+  return {};
+}
+
+//--- C.h
+#include "V.h"
+
+#include "A.h"
+
+class C {
+public:
+  C(const V ) {
+V v2;
+  }
+};
+
+C GetC() {
+   return {{}};
+}
+
+// This include *MUST* come last.
+#include "B.h"
+
+//--- module.modulemap
+module "V" { header "V.h" export * }
+module "A" { header "A.h" export * }
+module "B" { header "B.h" export * }
+module "C" { header "C.h" export * }
+
+//--- merge.cpp
+#include "C.h"
+
+template 
+C GetC_main() {
+   return {{}};
+}
+
+void f() {
+   GetC_main();
+   GetC();
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156660: [CodeGen] Assert that EmittedDeferredDecls is empty

[Jonas Hahnfeld via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG5ea647dea635: [CodeGen] Assert that EmittedDeferredDecls is 
empty (authored by Hahnfeld).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156660

Files:
  clang/lib/CodeGen/CodeGenModule.cpp


Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -7455,6 +7455,8 @@
   assert(NewBuilder->DeferredDecls.empty() &&
  "Newly created module should not have deferred decls");
   NewBuilder->DeferredDecls = std::move(DeferredDecls);
+  assert(EmittedDeferredDecls.empty() &&
+ "Still have (unmerged) EmittedDeferredDecls deferred decls");
 
   assert(NewBuilder->DeferredVTables.empty() &&
  "Newly created module should not have deferred vtables");
@@ -7470,10 +7472,5 @@
 
   NewBuilder->TBAA = std::move(TBAA);
 
-  assert(NewBuilder->EmittedDeferredDecls.empty() &&
- "Still have (unmerged) EmittedDeferredDecls deferred decls");
-
-  NewBuilder->EmittedDeferredDecls = std::move(EmittedDeferredDecls);
-
   NewBuilder->ABI->MangleCtx = std::move(ABI->MangleCtx);
 }


Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -7455,6 +7455,8 @@
   assert(NewBuilder->DeferredDecls.empty() &&
  "Newly created module should not have deferred decls");
   NewBuilder->DeferredDecls = std::move(DeferredDecls);
+  assert(EmittedDeferredDecls.empty() &&
+ "Still have (unmerged) EmittedDeferredDecls deferred decls");
 
   assert(NewBuilder->DeferredVTables.empty() &&
  "Newly created module should not have deferred vtables");
@@ -7470,10 +7472,5 @@
 
   NewBuilder->TBAA = std::move(TBAA);
 
-  assert(NewBuilder->EmittedDeferredDecls.empty() &&
- "Still have (unmerged) EmittedDeferredDecls deferred decls");
-
-  NewBuilder->EmittedDeferredDecls = std::move(EmittedDeferredDecls);
-
   NewBuilder->ABI->MangleCtx = std::move(ABI->MangleCtx);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156660: [CodeGen] Assert that EmittedDeferredDecls is empty

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld created this revision.
Hahnfeld added reviewers: v.g.vassilev, junaire, rjmccall.
Herald added a project: All.
Hahnfeld requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Its contents are transferred into `DeferredDecls` in `Release()`, so it
should be empty in `moveLazyEmissionStates()`. This matches the code
downstream in Cling.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D156660

Files:
  clang/lib/CodeGen/CodeGenModule.cpp


Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -7455,6 +7455,8 @@
   assert(NewBuilder->DeferredDecls.empty() &&
  "Newly created module should not have deferred decls");
   NewBuilder->DeferredDecls = std::move(DeferredDecls);
+  assert(EmittedDeferredDecls.empty() &&
+ "Still have (unmerged) EmittedDeferredDecls deferred decls");
 
   assert(NewBuilder->DeferredVTables.empty() &&
  "Newly created module should not have deferred vtables");
@@ -7470,10 +7472,5 @@
 
   NewBuilder->TBAA = std::move(TBAA);
 
-  assert(NewBuilder->EmittedDeferredDecls.empty() &&
- "Still have (unmerged) EmittedDeferredDecls deferred decls");
-
-  NewBuilder->EmittedDeferredDecls = std::move(EmittedDeferredDecls);
-
   NewBuilder->ABI->MangleCtx = std::move(ABI->MangleCtx);
 }


Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -7455,6 +7455,8 @@
   assert(NewBuilder->DeferredDecls.empty() &&
  "Newly created module should not have deferred decls");
   NewBuilder->DeferredDecls = std::move(DeferredDecls);
+  assert(EmittedDeferredDecls.empty() &&
+ "Still have (unmerged) EmittedDeferredDecls deferred decls");
 
   assert(NewBuilder->DeferredVTables.empty() &&
  "Newly created module should not have deferred vtables");
@@ -7470,10 +7472,5 @@
 
   NewBuilder->TBAA = std::move(TBAA);
 
-  assert(NewBuilder->EmittedDeferredDecls.empty() &&
- "Still have (unmerged) EmittedDeferredDecls deferred decls");
-
-  NewBuilder->EmittedDeferredDecls = std::move(EmittedDeferredDecls);
-
   NewBuilder->ABI->MangleCtx = std::move(ABI->MangleCtx);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154328: [AST] Add API to iterate already loaded specializations

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

Right, it's not ideal to land APIs without usage in Clang itself... The context 
is, once again, ROOT and the Cling interpreter (where we are already using this 
API via downstream patches): When unloading a "transaction" (now 
`PartialTranslationUnit` in `clang-repl`) we need to "unload" all contained 
declarations, ie make the code generator forget that it had already emitted 
them. For templates, this is a bit tricky so we need to iterate specializations 
without triggering more deserialization. This will get important with D41416 
 where we only load needed specializations.

When playing a bit earlier this week, I learned that the problem related to 
"unloading" of template specializations can be observed upstream in 
`clang-repl`:

  clang-repl> template  void f() {}
  clang-repl> f();
  clang-repl> %undo
  clang-repl> template <> void f() {}
  In file included from <<< inputs >>>:1:
  input_line_3:1:18: error: explicit specialization of 'f' after 
instantiation
  1 | template <> void f() {}
|  ^
  input_line_2:1:1: note: implicit instantiation first required here
  1 | f();
| ^
  error: Parsing failed.

Let me see if I can come up a fix for that in `clang-repl` that would then need 
this API. But for now, it's also fine for us if the patch stays pending in 
review, still better than completely disconnected local commits...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154328

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

[PATCH] D156537: [CodeGen] Keep track of eagerly emitted globals

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld created this revision.
Hahnfeld added reviewers: v.g.vassilev, rjmccall, aaron.ballman.
Herald added a project: All.
Hahnfeld requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

An inline virtual function must be emitted, but we need to remember
it and emit the same definition again in the future in case later
LLVM optimizations stripped it from the Module. The added test case
shows the problem; before this patch, it would fail with:

  Symbols not found: [ _ZN1AD0Ev, _ZN1AD1Ev ]


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D156537

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/Interpreter/inline-virtual.cpp


Index: clang/test/Interpreter/inline-virtual.cpp
===
--- /dev/null
+++ clang/test/Interpreter/inline-virtual.cpp
@@ -0,0 +1,21 @@
+// REQUIRES: host-supports-jit
+// UNSUPPORTED: system-aix
+// RUN: cat %s | clang-repl | FileCheck %s
+// RUN: cat %s | clang-repl -Xcc -O2 | FileCheck %s
+
+extern "C" int printf(const char *, ...);
+
+struct A { int a; A(int a) : a(a) {} virtual ~A(); };
+
+// Then define the virtual destructor as inline out-of-line, in a separate
+// PartialTranslationUnit.
+inline A::~A() { printf("~A(%d)\n", a); }
+
+// Create one instance with new and delete it.
+A *a1 = new A(1);
+delete a1;
+// CHECK: ~A(1)
+
+// Also create one global that will be auto-destructed.
+A a2(2);
+// CHECK: ~A(2)
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -3638,6 +3638,7 @@
   if (MustBeEmitted(Global) && MayBeEmittedEagerly(Global)) {
 // Emit the definition if it can't be deferred.
 EmitGlobalDefinition(GD);
+addEmittedDeferredDecl(GD);
 return;
   }
 


Index: clang/test/Interpreter/inline-virtual.cpp
===
--- /dev/null
+++ clang/test/Interpreter/inline-virtual.cpp
@@ -0,0 +1,21 @@
+// REQUIRES: host-supports-jit
+// UNSUPPORTED: system-aix
+// RUN: cat %s | clang-repl | FileCheck %s
+// RUN: cat %s | clang-repl -Xcc -O2 | FileCheck %s
+
+extern "C" int printf(const char *, ...);
+
+struct A { int a; A(int a) : a(a) {} virtual ~A(); };
+
+// Then define the virtual destructor as inline out-of-line, in a separate
+// PartialTranslationUnit.
+inline A::~A() { printf("~A(%d)\n", a); }
+
+// Create one instance with new and delete it.
+A *a1 = new A(1);
+delete a1;
+// CHECK: ~A(1)
+
+// Also create one global that will be auto-destructed.
+A a2(2);
+// CHECK: ~A(2)
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -3638,6 +3638,7 @@
   if (MustBeEmitted(Global) && MayBeEmittedEagerly(Global)) {
 // Emit the definition if it can't be deferred.
 EmitGlobalDefinition(GD);
+addEmittedDeferredDecl(GD);
 return;
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156425: [clang-repl] Remove redundant tests

[Jonas Hahnfeld via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGac6e9e69bac7: [clang-repl] Remove redundant tests (authored 
by Hahnfeld).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156425

Files:
  clang/test/Interpreter/code-undo.cpp
  clang/test/Interpreter/execute-weak.cpp
  clang/test/Interpreter/execute.cpp
  clang/test/Interpreter/fail.cpp
  clang/test/Interpreter/lambda.cpp


Index: clang/test/Interpreter/lambda.cpp
===
--- clang/test/Interpreter/lambda.cpp
+++ clang/test/Interpreter/lambda.cpp
@@ -1,9 +1,5 @@
-// RUN: clang-repl "int x = 10;" "int y=7; err;" "int y = 10;"
-// RUN: clang-repl "int i = 10;" 'extern "C" int printf(const char*,...);' \
-// RUN:'auto r1 = printf("i = %d\n", i);' | FileCheck 
--check-prefix=CHECK-DRIVER %s
 // REQUIRES: host-supports-jit
 // UNSUPPORTED: system-aix
-// CHECK-DRIVER: i = 10
 // RUN: cat %s | clang-repl | FileCheck %s
 // RUN: cat %s | clang-repl -Xcc -O2 | FileCheck %s
 extern "C" int printf(const char *, ...);
Index: clang/test/Interpreter/fail.cpp
===
--- clang/test/Interpreter/fail.cpp
+++ clang/test/Interpreter/fail.cpp
@@ -3,11 +3,8 @@
 // error, and then successfully recovers if we decide it's a success then for
 // the non-interactive mode the exit code should be a failure.
 // RUN: clang-repl "int x = 10;" "int y=7; err;" "int y = 10;"
-// RUN: clang-repl "int i = 10;" 'extern "C" int printf(const char*,...);' \
-// RUN:'auto r1 = printf("i = %d\n", i);' | FileCheck 
--check-prefix=CHECK-DRIVER %s
 // REQUIRES: host-supports-jit
 // UNSUPPORTED: system-aix
-// CHECK-DRIVER: i = 10
 // RUN: cat %s | not clang-repl | FileCheck %s
 BOOM!
 extern "C" int printf(const char *, ...);
Index: clang/test/Interpreter/execute.cpp
===
--- clang/test/Interpreter/execute.cpp
+++ clang/test/Interpreter/execute.cpp
@@ -1,9 +1,10 @@
+// UNSUPPORTED: system-aix
+
 // clang-format off
-// RUN: clang-repl "int x = 10;" "int y=7; err;" "int y = 10;"
 // RUN: clang-repl "int i = 10;" 'extern "C" int printf(const char*,...);' \
 // RUN:'auto r1 = printf("i = %d\n", i);' | FileCheck 
--check-prefix=CHECK-DRIVER %s
-// UNSUPPORTED: system-aix
 // CHECK-DRIVER: i = 10
+
 // RUN: cat %s | clang-repl | FileCheck %s
 // RUN: cat %s | clang-repl -Xcc -O2 | FileCheck %s
 extern "C" int printf(const char *, ...);
Index: clang/test/Interpreter/execute-weak.cpp
===
--- clang/test/Interpreter/execute-weak.cpp
+++ clang/test/Interpreter/execute-weak.cpp
@@ -1,8 +1,3 @@
-// RUN: clang-repl "int x = 10;" "int y=7; err;" "int y = 10;"
-// RUN: clang-repl "int i = 10;" 'extern "C" int printf(const char*,...);' \
-// RUN:'auto r1 = printf("i = %d\n", i);' | FileCheck 
--check-prefix=CHECK-DRIVER %s
-// CHECK-DRIVER: i = 10
-//
 // UNSUPPORTED: system-aix, system-windows
 // RUN: cat %s | clang-repl | FileCheck %s
 
Index: clang/test/Interpreter/code-undo.cpp
===
--- clang/test/Interpreter/code-undo.cpp
+++ clang/test/Interpreter/code-undo.cpp
@@ -1,7 +1,4 @@
-// RUN: clang-repl "int i = 10;" 'extern "C" int printf(const char*,...);' \
-// RUN:'auto r1 = printf("i = %d\n", i);' | FileCheck 
--check-prefix=CHECK-DRIVER %s
 // UNSUPPORTED: system-aix
-// CHECK-DRIVER: i = 10
 // RUN: cat %s | clang-repl | FileCheck %s
 extern "C" int printf(const char *, ...);
 int x1 = 0;


Index: clang/test/Interpreter/lambda.cpp
===
--- clang/test/Interpreter/lambda.cpp
+++ clang/test/Interpreter/lambda.cpp
@@ -1,9 +1,5 @@
-// RUN: clang-repl "int x = 10;" "int y=7; err;" "int y = 10;"
-// RUN: clang-repl "int i = 10;" 'extern "C" int printf(const char*,...);' \
-// RUN:'auto r1 = printf("i = %d\n", i);' | FileCheck --check-prefix=CHECK-DRIVER %s
 // REQUIRES: host-supports-jit
 // UNSUPPORTED: system-aix
-// CHECK-DRIVER: i = 10
 // RUN: cat %s | clang-repl | FileCheck %s
 // RUN: cat %s | clang-repl -Xcc -O2 | FileCheck %s
 extern "C" int printf(const char *, ...);
Index: clang/test/Interpreter/fail.cpp
===
--- clang/test/Interpreter/fail.cpp
+++ clang/test/Interpreter/fail.cpp
@@ -3,11 +3,8 @@
 // error, and then successfully recovers if we decide it's a success then for
 // the non-interactive mode the exit code should be a failure.
 // RUN: clang-repl "int x = 10;" "int y=7; err;" "int y = 10;"
-// RUN: clang-repl "int i = 10;" 'extern "C" int printf(const char*,...);' \
-// RUN:'auto r1 = printf("i = 

[PATCH] D156425: [clang-repl] Remove redundant tests

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld created this revision.
Hahnfeld added a reviewer: v.g.vassilev.
Herald added a project: All.
Hahnfeld requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

They only need to be tested once in `execute.cpp` and `fail.cpp`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D156425

Files:
  clang/test/Interpreter/code-undo.cpp
  clang/test/Interpreter/execute-weak.cpp
  clang/test/Interpreter/execute.cpp
  clang/test/Interpreter/fail.cpp
  clang/test/Interpreter/lambda.cpp


Index: clang/test/Interpreter/lambda.cpp
===
--- clang/test/Interpreter/lambda.cpp
+++ clang/test/Interpreter/lambda.cpp
@@ -1,9 +1,5 @@
-// RUN: clang-repl "int x = 10;" "int y=7; err;" "int y = 10;"
-// RUN: clang-repl "int i = 10;" 'extern "C" int printf(const char*,...);' \
-// RUN:'auto r1 = printf("i = %d\n", i);' | FileCheck 
--check-prefix=CHECK-DRIVER %s
 // REQUIRES: host-supports-jit
 // UNSUPPORTED: system-aix
-// CHECK-DRIVER: i = 10
 // RUN: cat %s | clang-repl | FileCheck %s
 // RUN: cat %s | clang-repl -Xcc -O2 | FileCheck %s
 extern "C" int printf(const char *, ...);
Index: clang/test/Interpreter/fail.cpp
===
--- clang/test/Interpreter/fail.cpp
+++ clang/test/Interpreter/fail.cpp
@@ -3,11 +3,8 @@
 // error, and then successfully recovers if we decide it's a success then for
 // the non-interactive mode the exit code should be a failure.
 // RUN: clang-repl "int x = 10;" "int y=7; err;" "int y = 10;"
-// RUN: clang-repl "int i = 10;" 'extern "C" int printf(const char*,...);' \
-// RUN:'auto r1 = printf("i = %d\n", i);' | FileCheck 
--check-prefix=CHECK-DRIVER %s
 // REQUIRES: host-supports-jit
 // UNSUPPORTED: system-aix
-// CHECK-DRIVER: i = 10
 // RUN: cat %s | not clang-repl | FileCheck %s
 BOOM!
 extern "C" int printf(const char *, ...);
Index: clang/test/Interpreter/execute.cpp
===
--- clang/test/Interpreter/execute.cpp
+++ clang/test/Interpreter/execute.cpp
@@ -1,9 +1,10 @@
+// UNSUPPORTED: system-aix
+
 // clang-format off
-// RUN: clang-repl "int x = 10;" "int y=7; err;" "int y = 10;"
 // RUN: clang-repl "int i = 10;" 'extern "C" int printf(const char*,...);' \
 // RUN:'auto r1 = printf("i = %d\n", i);' | FileCheck 
--check-prefix=CHECK-DRIVER %s
-// UNSUPPORTED: system-aix
 // CHECK-DRIVER: i = 10
+
 // RUN: cat %s | clang-repl | FileCheck %s
 // RUN: cat %s | clang-repl -Xcc -O2 | FileCheck %s
 extern "C" int printf(const char *, ...);
Index: clang/test/Interpreter/execute-weak.cpp
===
--- clang/test/Interpreter/execute-weak.cpp
+++ clang/test/Interpreter/execute-weak.cpp
@@ -1,8 +1,3 @@
-// RUN: clang-repl "int x = 10;" "int y=7; err;" "int y = 10;"
-// RUN: clang-repl "int i = 10;" 'extern "C" int printf(const char*,...);' \
-// RUN:'auto r1 = printf("i = %d\n", i);' | FileCheck 
--check-prefix=CHECK-DRIVER %s
-// CHECK-DRIVER: i = 10
-//
 // UNSUPPORTED: system-aix, system-windows
 // RUN: cat %s | clang-repl | FileCheck %s
 
Index: clang/test/Interpreter/code-undo.cpp
===
--- clang/test/Interpreter/code-undo.cpp
+++ clang/test/Interpreter/code-undo.cpp
@@ -1,7 +1,4 @@
-// RUN: clang-repl "int i = 10;" 'extern "C" int printf(const char*,...);' \
-// RUN:'auto r1 = printf("i = %d\n", i);' | FileCheck 
--check-prefix=CHECK-DRIVER %s
 // UNSUPPORTED: system-aix
-// CHECK-DRIVER: i = 10
 // RUN: cat %s | clang-repl | FileCheck %s
 extern "C" int printf(const char *, ...);
 int x1 = 0;


Index: clang/test/Interpreter/lambda.cpp
===
--- clang/test/Interpreter/lambda.cpp
+++ clang/test/Interpreter/lambda.cpp
@@ -1,9 +1,5 @@
-// RUN: clang-repl "int x = 10;" "int y=7; err;" "int y = 10;"
-// RUN: clang-repl "int i = 10;" 'extern "C" int printf(const char*,...);' \
-// RUN:'auto r1 = printf("i = %d\n", i);' | FileCheck --check-prefix=CHECK-DRIVER %s
 // REQUIRES: host-supports-jit
 // UNSUPPORTED: system-aix
-// CHECK-DRIVER: i = 10
 // RUN: cat %s | clang-repl | FileCheck %s
 // RUN: cat %s | clang-repl -Xcc -O2 | FileCheck %s
 extern "C" int printf(const char *, ...);
Index: clang/test/Interpreter/fail.cpp
===
--- clang/test/Interpreter/fail.cpp
+++ clang/test/Interpreter/fail.cpp
@@ -3,11 +3,8 @@
 // error, and then successfully recovers if we decide it's a success then for
 // the non-interactive mode the exit code should be a failure.
 // RUN: clang-repl "int x = 10;" "int y=7; err;" "int y = 10;"
-// RUN: clang-repl "int i = 10;" 'extern "C" int printf(const char*,...);' \
-// RUN:'auto r1 = printf("i = %d\n", 

[PATCH] D154324: [C++20] [Modules] [ODRHash] Use CanonicalType for base classes

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

In D154324#4522551 , @alexfh wrote:

> BTW, if in a.h I change
>
>   typename std::enable_if<::p::P::value>::type>
>
> to
>
>   typename std::enable_if::value>::type>
>
> Compilation succeeds.

For the fun of it, could you test https://reviews.llvm.org/D153003 on this 
reproducer and also the internal, real code?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154324

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

[PATCH] D154328: [AST] Add API to iterate already loaded specializations

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

gentle ping :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154328

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

[PATCH] D154856: [MemProf] Use new option/pass for profile feedback and matching

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

Ok, turns out I had old test binaries around that got automatically picked up 
by `lit`. Sorry for the noise.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154856

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

[PATCH] D154856: [MemProf] Use new option/pass for profile feedback and matching

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

Hi @tejohnson, I'm seeing crashes after this revision:

  Failed Tests (2):
LLVM-Unit :: Passes/./PluginsTests/0/2
LLVM-Unit :: Passes/./PluginsTests/1/2

A stack trace looks as follows:

   #0 0x7fba1f899667 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) 
/home/jhahnfel/LLVM/src/llvm/lib/Support/Unix/Signals.inc:602:13
   #1 0x7fba1f897752 llvm::sys::RunSignalHandlers() 
/home/jhahnfel/LLVM/src/llvm/lib/Support/Signals.cpp:105:18
   #2 0x7fba1f899cef SignalHandler(int) 
/home/jhahnfel/LLVM/src/llvm/lib/Support/Unix/Signals.inc:413:1
   #3 0x7fba234cacf0 __restore_rt (/lib64/libpthread.so.0+0x12cf0)
   #4 0x7fba1a416ea2 __memcpy_avx_unaligned_erms (/lib64/libc.so.6+0xceea2)
   #5 0x7fba1f81c09f std::__cxx11::basic_string, std::allocator>::_M_length(unsigned long) 
/opt/rh/gcc-toolset-12/root/usr/lib/gcc/x86_64-redhat-linux/12/../../../../include/c++/12/bits/basic_string.h:229:26
   #6 0x7fba1f81c09f std::__cxx11::basic_string, std::allocator>::_M_set_length(unsigned long) 
/opt/rh/gcc-toolset-12/root/usr/lib/gcc/x86_64-redhat-linux/12/../../../../include/c++/12/bits/basic_string.h:267:2
   #7 0x7fba1f81c09f void std::__cxx11::basic_string, std::allocator>::_M_construct(char*, 
char*, std::forward_iterator_tag) 
/opt/rh/gcc-toolset-12/root/usr/lib/gcc/x86_64-redhat-linux/12/../../../../include/c++/12/bits/basic_string.tcc:247:2
   #8 0x7fba1f81c09f std::__cxx11::basic_string, 
std::allocator>::basic_string(std::__cxx11::basic_string, std::allocator> const&) 
/opt/rh/gcc-toolset-12/root/usr/lib/gcc/x86_64-redhat-linux/12/../../../../include/c++/12/bits/basic_string.h:544:2
   #9 0x7fba1f81c09f llvm::PGOOptions::PGOOptions(llvm::PGOOptions const&) 
/home/jhahnfel/LLVM/src/llvm/lib/Support/PGOOptions.cpp:54:13
  #10 0x7fba22e95528 void 
std::_Optional_payload_base::_M_construct(llvm::PGOOptions const&) 
/opt/rh/gcc-toolset-12/root/usr/lib/gcc/x86_64-redhat-linux/12/../../../../include/c++/12/optional:280:21
  #11 0x7fba22e95528 
std::_Optional_payload_base::_Optional_payload_base(bool, 
std::_Optional_payload_base const&) 
/opt/rh/gcc-toolset-12/root/usr/lib/gcc/x86_64-redhat-linux/12/../../../../include/c++/12/optional:145:10
  #12 0x7fba22e95528 std::_Optional_payload::_Optional_payload(bool, std::_Optional_payload_base 
const&) 
/opt/rh/gcc-toolset-12/root/usr/lib/gcc/x86_64-redhat-linux/12/../../../../include/c++/12/optional:397:42
  #13 0x7fba22e95528 std::_Optional_payload::_Optional_payload(bool, std::_Optional_payload_base 
const&) 
/opt/rh/gcc-toolset-12/root/usr/lib/gcc/x86_64-redhat-linux/12/../../../../include/c++/12/optional:431:57
  #14 0x7fba22e95528 std::_Optional_base::_Optional_base(std::_Optional_base 
const&) 
/opt/rh/gcc-toolset-12/root/usr/lib/gcc/x86_64-redhat-linux/12/../../../../include/c++/12/optional:538:9
  #15 0x7fba22e95528 
std::optional::optional(std::optional 
const&) 
/opt/rh/gcc-toolset-12/root/usr/lib/gcc/x86_64-redhat-linux/12/../../../../include/c++/12/optional:705:11
  #16 0x7fba22e95528 llvm::PassBuilder::PassBuilder(llvm::TargetMachine*, 
llvm::PipelineTuningOptions, std::optional, 
llvm::PassInstrumentationCallbacks*) 
/home/jhahnfel/LLVM/src/llvm/lib/Passes/PassBuilder.cpp:407:25
  #17 0x00231c10 
std::_Optional_payload_base::_M_reset() 
/opt/rh/gcc-toolset-12/root/usr/lib/gcc/x86_64-redhat-linux/12/../../../../include/c++/12/optional:317:12
  #18 0x00231c10 std::_Optional_payload::~_Optional_payload() 
/opt/rh/gcc-toolset-12/root/usr/lib/gcc/x86_64-redhat-linux/12/../../../../include/c++/12/optional:439:57
  #19 0x00231c10 std::_Optional_base::~_Optional_base() 
/opt/rh/gcc-toolset-12/root/usr/lib/gcc/x86_64-redhat-linux/12/../../../../include/c++/12/optional:510:12
  #20 0x00231c10 PluginsTests_LoadMultiplePlugins_Test::TestBody() 
/home/jhahnfel/LLVM/src/llvm/unittests/Passes/PluginsTest.cpp:102:15
  #21 0x00243a7f 
testing::internal::UnitTestImpl::os_stack_trace_getter() 
/home/jhahnfel/LLVM/src/third-party/unittest/googletest/src/gtest.cc:5635:7
  #22 0x00243a7f testing::Test::Run() 
/home/jhahnfel/LLVM/src/third-party/unittest/googletest/src/gtest.cc:2515:9
  #23 0x002451b9 testing::TestInfo::Run() 
/home/jhahnfel/LLVM/src/third-party/unittest/googletest/src/gtest.cc:0:0
  #24 0x002459e1 testing::TestSuite::Run() 
/home/jhahnfel/LLVM/src/third-party/unittest/googletest/src/gtest.cc:2815:44
  #25 0x002524c7 testing::internal::UnitTestImpl::RunAllTests() 
/home/jhahnfel/LLVM/src/third-party/unittest/googletest/src/gtest.cc:5337:24
  #26 0x00251d0c testing::UnitTest::Run() 
/home/jhahnfel/LLVM/src/third-party/unittest/googletest/src/gtest.cc:4925:10
  #27 0x0023535c main 
/home/jhahnfel/LLVM/src/third-party/unittest/UnitTestMain/TestMain.cpp:55:3
  #28 0x7fba1a382d85 __libc_start_main (/lib64/libc.so.6+0x3ad85)
  #29 0x0022eb9e _start 

[PATCH] D41416: [modules] [pch] Do not deserialize all lazy template specializations when looking for one.

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added inline comments.



Comment at: clang/include/clang/AST/DeclTemplate.h:786-788
+uint32_t DeclID = ~0U;
+unsigned ODRHash = ~0U;
+bool IsPartial = false;

ChuanqiXu wrote:
> Maybe this can save some space.
Can you elaborate why this would save space? The best practice is to order data 
members by size to avoid padding gaps. As far as I can see, this is already 
done here.


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

https://reviews.llvm.org/D41416

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

[PATCH] D41416: [modules] [pch] Do not deserialize all lazy template specializations when looking for one.

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

In D41416#4487516 , @ChuanqiXu wrote:

> But some local in tree tests got failed. I took some time to investigate them 
> but I didn't get insights :  (

If the failures involve template template parameters, please note that you need 
https://reviews.llvm.org/D153003 for that to properly hash and find in the list 
of lazy specializations.




Comment at: clang/lib/AST/DeclTemplate.cpp:366
+  Args,
+  TemplateParameterList 
*TPL) const {
+  CommonBase *CommonBasePtr = getMostRecentDecl()->getCommonPtr();

ChuanqiXu wrote:
> TPL here looks not used.
This is required because of the argument forwarding from 
`findSpecializationImpl` below...


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

https://reviews.llvm.org/D41416

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

[PATCH] D134813: Properly print unnamed TagDecl objects in diagnostics

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

In D134813#4482822 , @aaron.ballman 
wrote:

> In D134813#4482819 , @Hahnfeld 
> wrote:
>
>> Thanks! Note that the same probably holds true (but I didn't test) for all 
>> other classes that override `printName(raw_ostream , const PrintingPolicy 
>> )`, ie `DecompositionDecl`, `MSGuidDecl`, 
>> `UnnamedGlobalConstantDecl`, and `TemplateParamObjectDecl`.
>
> You're correct, but I figured those are somewhat uncommon AST nodes, so we 
> can probably expose those APIs as-needed rather than doing all of them.

Oh, alright.

> However, if you prefer they all get handled, it's easy enough!

No, not needed. I was only running into it with an `EnumDecl` while going to 
LLVM 16. But as I said, it's easy enough to work around.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134813

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

[PATCH] D134813: Properly print unnamed TagDecl objects in diagnostics

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

In D134813#4482808 , @aaron.ballman 
wrote:

> In D134813#4482674 , @Hahnfeld 
> wrote:
>
>> Out of curiosity, not sure if it's worth fixing because it's easy enough to 
>> work around: I think after this change, it's not possible anymore to call 
>> `printName(raw_ostream )` on a (statically typed) `TagDecl` / `EnumDecl` 
>> because it's hidden by `TagDecl::printName(raw_ostream , const 
>> PrintingPolicy )` while it works perfectly for a `NamedDecl`. Is this 
>> intentional?
>
> That's not intentional, thank you for pointing it out! This should be 
> corrected in 0566791ab28b5b842c3befea63d7d47fe9ba1a59 
> 

Thanks! Note that the same probably holds true (but I didn't test) for all 
other classes that override `printName(raw_ostream , const PrintingPolicy 
)`, ie `DecompositionDecl`, `MSGuidDecl`, `UnnamedGlobalConstantDecl`, 
and `TemplateParamObjectDecl`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134813

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

[PATCH] D134813: Properly print unnamed TagDecl objects in diagnostics

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

Out of curiosity, not sure if it's worth fixing because it's easy enough to 
work around: I think after this change, it's not possible anymore to call 
`printName(raw_ostream )` on a (statically typed) `TagDecl` / `EnumDecl` 
because it's hidden by `TagDecl::printName(raw_ostream , const 
PrintingPolicy )` while it works perfectly for a `NamedDecl`. Is this 
intentional?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134813

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

[PATCH] D137787: [CodeGen] Relax assertion on generating destructor call

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld abandoned this revision.
Hahnfeld added a comment.

apparently not needed anymore downstream...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137787

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

[PATCH] D154328: [AST] Add API to iterate already loaded specializations

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld created this revision.
Hahnfeld added reviewers: erichkeane, aaron.ballman.
Herald added a project: All.
Hahnfeld requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

These new functions allow to look at specializations without triggering 
deserialization, which might be problematic in some contexts.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D154328

Files:
  clang/include/clang/AST/DeclTemplate.h


Index: clang/include/clang/AST/DeclTemplate.h
===
--- clang/include/clang/AST/DeclTemplate.h
+++ clang/include/clang/AST/DeclTemplate.h
@@ -1114,6 +1114,20 @@
 return makeSpecIterator(getSpecializations(), true);
   }
 
+  /// All specializations that that have already been loaded, ie avoiding
+  /// deserialization of lazily registered specializations.
+  spec_range loaded_specializations() const {
+return spec_range(loaded_spec_begin(), loaded_spec_end());
+  }
+
+  spec_iterator loaded_spec_begin() const {
+return makeSpecIterator(getCommonPtr()->Specializations, false);
+  }
+
+  spec_iterator loaded_spec_end() const {
+return makeSpecIterator(getCommonPtr()->Specializations, true);
+  }
+
   /// Return whether this function template is an abbreviated function 
template,
   /// e.g. `void foo(auto x)` or `template void foo(auto x)`
   bool isAbbreviated() const {
@@ -2454,6 +2468,20 @@
 return makeSpecIterator(getSpecializations(), true);
   }
 
+  /// All specializations that that have already been loaded, ie avoiding
+  /// deserialization of lazily registered specializations.
+  spec_range loaded_specializations() const {
+return spec_range(loaded_spec_begin(), loaded_spec_end());
+  }
+
+  spec_iterator loaded_spec_begin() const {
+return makeSpecIterator(getCommonPtr()->Specializations, false);
+  }
+
+  spec_iterator loaded_spec_end() const {
+return makeSpecIterator(getCommonPtr()->Specializations, true);
+  }
+
   // Implement isa/cast/dyncast support
   static bool classof(const Decl *D) { return classofKind(D->getKind()); }
   static bool classofKind(Kind K) { return K == ClassTemplate; }
@@ -3262,6 +3290,20 @@
 return makeSpecIterator(getSpecializations(), true);
   }
 
+  /// All specializations that that have already been loaded, ie avoiding
+  /// deserialization of lazily registered specializations.
+  spec_range loaded_specializations() const {
+return spec_range(loaded_spec_begin(), loaded_spec_end());
+  }
+
+  spec_iterator loaded_spec_begin() const {
+return makeSpecIterator(getCommonPtr()->Specializations, false);
+  }
+
+  spec_iterator loaded_spec_end() const {
+return makeSpecIterator(getCommonPtr()->Specializations, true);
+  }
+
   // Implement isa/cast/dyncast support
   static bool classof(const Decl *D) { return classofKind(D->getKind()); }
   static bool classofKind(Kind K) { return K == VarTemplate; }


Index: clang/include/clang/AST/DeclTemplate.h
===
--- clang/include/clang/AST/DeclTemplate.h
+++ clang/include/clang/AST/DeclTemplate.h
@@ -1114,6 +1114,20 @@
 return makeSpecIterator(getSpecializations(), true);
   }
 
+  /// All specializations that that have already been loaded, ie avoiding
+  /// deserialization of lazily registered specializations.
+  spec_range loaded_specializations() const {
+return spec_range(loaded_spec_begin(), loaded_spec_end());
+  }
+
+  spec_iterator loaded_spec_begin() const {
+return makeSpecIterator(getCommonPtr()->Specializations, false);
+  }
+
+  spec_iterator loaded_spec_end() const {
+return makeSpecIterator(getCommonPtr()->Specializations, true);
+  }
+
   /// Return whether this function template is an abbreviated function template,
   /// e.g. `void foo(auto x)` or `template void foo(auto x)`
   bool isAbbreviated() const {
@@ -2454,6 +2468,20 @@
 return makeSpecIterator(getSpecializations(), true);
   }
 
+  /// All specializations that that have already been loaded, ie avoiding
+  /// deserialization of lazily registered specializations.
+  spec_range loaded_specializations() const {
+return spec_range(loaded_spec_begin(), loaded_spec_end());
+  }
+
+  spec_iterator loaded_spec_begin() const {
+return makeSpecIterator(getCommonPtr()->Specializations, false);
+  }
+
+  spec_iterator loaded_spec_end() const {
+return makeSpecIterator(getCommonPtr()->Specializations, true);
+  }
+
   // Implement isa/cast/dyncast support
   static bool classof(const Decl *D) { return classofKind(D->getKind()); }
   static bool classofKind(Kind K) { return K == ClassTemplate; }
@@ -3262,6 +3290,20 @@
 return makeSpecIterator(getSpecializations(), true);
   }
 
+  /// All specializations that that have already been loaded, ie avoiding
+  /// deserialization of lazily registered specializations.
+  spec_range loaded_specializations() const {
+return 

[PATCH] D153003: [ODRHash] Fix ODR hashing of template names

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

In D153003#4458323 , @ChuanqiXu wrote:

> In D153003#4456595 , @rsmith wrote:
>
>> I think the behavior change for the testcase here is correct, though I'm not 
>> sure that the patch is getting that behaviour change in the right way. Per 
>> [temp.type]/1.4 (http://eel.is/c++draft/temp.type#1.4),
>>
>>> Two template-ids are the same if [...] their corresponding template 
>>> template-arguments refer to the same template.
>>
>> so `B` and `B` are the same type. The stricter "same sequence of 
>> tokens" rule doesn't apply here, because using-declarations are not 
>> definitions.
>
> Got it. Thanks for your commenting. I can't reopen this page. So I file an 
> issue here https://github.com/llvm/llvm-project/issues/63595. @Hahnfeld you 
> can still work on this by following the @rsmith 's suggestion  if you're 
> still interested. Or I'd like to take it.

You mean re-open this review? Which suggestions exactly?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153003

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

[PATCH] D153003: [ODRHash] Fix ODR hashing of template names

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld abandoned this revision.
Hahnfeld added a comment.

Ok, I understand that fixing `ODRHash` is the wrong approach for our needs - 
we'll likely need to implement a custom hashing of template arguments to work 
as a lookup for lazy loading.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153003

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

[PATCH] D153003: [ODRHash] Fix ODR hashing of template names

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

> We can't say this abstractly. It should be fine to work in ODRHash for C++20 
> modules issues as long as we don't lose informations.

I honestly don't understand this: For the approach to work, we would need to 
hash the two cases of an unqualified and a qualified template to the same 
value. You oppose to this approach because it would lose information. I can 
only conclude that the approach doesn't work with `ODRHash`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153003

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

[PATCH] D153003: [ODRHash] Fix ODR hashing of template names

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

In D153003#4427412 , @ChuanqiXu wrote:

> An important node here is that ODRHash is used to check the AST Nodes are 
> keeping the same across compilations. There is gap to use ODRHash to check 
> the semantical equality.

Oh ok... I'm coming from the context of https://reviews.llvm.org/D41416 which 
uses `ODRHash`es to decide which `Decl`s to load. The problem at hand is that 
the hash in the module was computed from a `TemplateName::Template` while the 
instantiating context has a fully qualified template name. I was thinking to 
"solve" this by making them hash to the same value, but I'm hearing that using 
`ODRHash` for this purpose is the wrong approach to begin with?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153003

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

[PATCH] D153003: [ODRHash] Fix ODR hashing of template names

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

> For example, the template name with QualifiedTemplate kind has different hash 
> name with the name with DependentTemplate kind. But it is not true after the 
> patch.

Yes, I do understand. However, what I'm asking is why we need to differentiate 
between the two, ie in which cases do we want these two to have a different 
hash?

> No. IsSameEntity is a weaker check. It simply checks whether two named decls 
> refers to the same name.

I'm confused: Hashing always works the way that two "identical" objects hash to 
the same value, while two "non-identical" objects may also hash to the same 
value (hash collision). The way I understand your statement is that two 
"identical" Decls, including `isSame` returning `true`, can have a different 
hashes. This doesn't make sense to me, could you maybe clarify?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153003

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

[PATCH] D152995: Remove clang/ModuleInfo.txt

[Jonas Hahnfeld via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGce8ff3facc63: Remove clang/ModuleInfo.txt (authored by 
Hahnfeld).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152995

Files:
  clang/ModuleInfo.txt


Index: clang/ModuleInfo.txt
===
--- clang/ModuleInfo.txt
+++ /dev/null
@@ -1,5 +0,0 @@
-# This file provides information for llvm-top
-DepModule: llvm 
-ConfigCmd:
-ConfigTest:
-BuildCmd:


Index: clang/ModuleInfo.txt
===
--- clang/ModuleInfo.txt
+++ /dev/null
@@ -1,5 +0,0 @@
-# This file provides information for llvm-top
-DepModule: llvm 
-ConfigCmd:
-ConfigTest:
-BuildCmd:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D153003: [ODRHash] Fix ODR hashing of template names

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

In D153003#4423926 , @ChuanqiXu wrote:

> This looks not so good. In this way, we can't disambiguate other template 
> types. At least we added the kind and a TODO here.

Which template name types would we need to disambiguate? We could also 
normalize the `Kind`, for example from `QualifiedTemplate` to `Template` (which 
is the case I care about).

> BTW, the attached test case is in fact in correct. See 
> https://eel.is/c++draft/basic.def.odr#14.3, such mergeable declarations 
> shouldn't be attached to named modules. (And this is also a defect that now 
> we don't diagnose it correctly).

I'm not sure I understand, could you elaborate? "correct" as in "the merge 
error is right" or "should work as written". Also there are a number of `export 
struct` in the tests, but you are saying this should not be included in the 
modules? How would this work?

> Even if you put them into the global module fragment, I think we should try 
> to look other places like `isSameEntity` to solve this.

Sure, but this first requires the hash to be identical, no? My understanding is 
that two (structurally) identical Decls must always hash to the same value, but 
the same value does not imply that `isSameEntity` and friends eventually agree 
(due to hash collisions).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153003

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

[PATCH] D153003: [ODRHash] Fix ODR hashing of template names

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld created this revision.
Hahnfeld added reviewers: rtrieu, vsapsai, ChuanqiXu, Bigcheese.
Herald added a project: All.
Hahnfeld requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

For hashing, we should not differentiate between templates and qualified
templates and only look at the underlying name. As an example, consider
the added test case of a template template parameter; the two modules
have slightly differing `using` declarations (`using C = B` vs `B`)
which should be treated as identical.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153003

Files:
  clang/lib/AST/ODRHash.cpp
  clang/test/Modules/merge-template-template-parameters.cppm


Index: clang/test/Modules/merge-template-template-parameters.cppm
===
--- /dev/null
+++ clang/test/Modules/merge-template-template-parameters.cppm
@@ -0,0 +1,44 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -I%t %t/module1.cppm -o 
%t/module1.pcm
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -I%t %t/module2.cppm -o 
%t/module2.pcm
+// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t %t/merge.cpp -verify 
-fsyntax-only
+
+//--- header.h
+namespace NS {
+template 
+class A {
+};
+
+template  class T>
+class B {
+};
+}
+
+//--- module1.cppm
+// inside NS, using C = B
+module;
+export module module1;
+#include "header.h"
+namespace NS {
+using C = B;
+}
+export struct D : NS::C {};
+
+//--- module2.cppm
+// inside NS, using C = B
+module;
+export module module2;
+#include "header.h"
+namespace NS {
+using C = B;
+}
+export struct D : NS::C {};
+
+//--- merge.cpp
+// expected-no-diagnostics
+import module1;
+import module2;
+D d;
Index: clang/lib/AST/ODRHash.cpp
===
--- clang/lib/AST/ODRHash.cpp
+++ clang/lib/AST/ODRHash.cpp
@@ -139,23 +139,8 @@
 }
 
 void ODRHash::AddTemplateName(TemplateName Name) {
-  auto Kind = Name.getKind();
-  ID.AddInteger(Kind);
-
-  switch (Kind) {
-  case TemplateName::Template:
-AddDecl(Name.getAsTemplateDecl());
-break;
-  // TODO: Support these cases.
-  case TemplateName::OverloadedTemplate:
-  case TemplateName::AssumedTemplate:
-  case TemplateName::QualifiedTemplate:
-  case TemplateName::DependentTemplate:
-  case TemplateName::SubstTemplateTemplateParm:
-  case TemplateName::SubstTemplateTemplateParmPack:
-  case TemplateName::UsingTemplate:
-break;
-  }
+  if (auto *TD = Name.getAsTemplateDecl())
+AddDecl(TD);
 }
 
 void ODRHash::AddTemplateArgument(TemplateArgument TA) {


Index: clang/test/Modules/merge-template-template-parameters.cppm
===
--- /dev/null
+++ clang/test/Modules/merge-template-template-parameters.cppm
@@ -0,0 +1,44 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -I%t %t/module1.cppm -o %t/module1.pcm
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -I%t %t/module2.cppm -o %t/module2.pcm
+// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t %t/merge.cpp -verify -fsyntax-only
+
+//--- header.h
+namespace NS {
+template 
+class A {
+};
+
+template  class T>
+class B {
+};
+}
+
+//--- module1.cppm
+// inside NS, using C = B
+module;
+export module module1;
+#include "header.h"
+namespace NS {
+using C = B;
+}
+export struct D : NS::C {};
+
+//--- module2.cppm
+// inside NS, using C = B
+module;
+export module module2;
+#include "header.h"
+namespace NS {
+using C = B;
+}
+export struct D : NS::C {};
+
+//--- merge.cpp
+// expected-no-diagnostics
+import module1;
+import module2;
+D d;
Index: clang/lib/AST/ODRHash.cpp
===
--- clang/lib/AST/ODRHash.cpp
+++ clang/lib/AST/ODRHash.cpp
@@ -139,23 +139,8 @@
 }
 
 void ODRHash::AddTemplateName(TemplateName Name) {
-  auto Kind = Name.getKind();
-  ID.AddInteger(Kind);
-
-  switch (Kind) {
-  case TemplateName::Template:
-AddDecl(Name.getAsTemplateDecl());
-break;
-  // TODO: Support these cases.
-  case TemplateName::OverloadedTemplate:
-  case TemplateName::AssumedTemplate:
-  case TemplateName::QualifiedTemplate:
-  case TemplateName::DependentTemplate:
-  case TemplateName::SubstTemplateTemplateParm:
-  case TemplateName::SubstTemplateTemplateParmPack:
-  case TemplateName::UsingTemplate:
-break;
-  }
+  if (auto *TD = Name.getAsTemplateDecl())
+AddDecl(TD);
 }
 
 void ODRHash::AddTemplateArgument(TemplateArgument TA) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D152995: Remove clang/ModuleInfo.txt

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

Just checking to be sure there is no other use case that the file is used for, 
but I doubt it...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152995

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

[PATCH] D152995: Remove clang/ModuleInfo.txt

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld created this revision.
Hahnfeld added reviewers: rsmith, aaron.ballman.
Herald added a project: All.
Hahnfeld requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The script `build-for-llvm-top.sh` and LLVM's `ModuleInfo.txt` are gone since a 
long time (commit rGd20ea7dc59 
 in 
November 2011), and `llvm-top` itself has even been removed from llvm-archive 
(it can be found here: 
https://github.com/llvm/llvm-archive/tree/cab7f8f160f0bd8d20d9a4036aa4083f2bc2740a/llvm-top)
 so delete Clang's `ModuleInfo.txt` as well.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D152995

Files:
  clang/ModuleInfo.txt


Index: clang/ModuleInfo.txt
===
--- clang/ModuleInfo.txt
+++ /dev/null
@@ -1,5 +0,0 @@
-# This file provides information for llvm-top
-DepModule: llvm 
-ConfigCmd:
-ConfigTest:
-BuildCmd:


Index: clang/ModuleInfo.txt
===
--- clang/ModuleInfo.txt
+++ /dev/null
@@ -1,5 +0,0 @@
-# This file provides information for llvm-top
-DepModule: llvm 
-ConfigCmd:
-ConfigTest:
-BuildCmd:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150773: [clang][modules] Add features for recent C++ versions

[Jonas Hahnfeld via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG53c03a3db16c: [clang][modules] Add features for recent C++ 
versions (authored by Hahnfeld).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150773

Files:
  clang/docs/Modules.rst
  clang/docs/ReleaseNotes.rst
  clang/lib/Basic/Module.cpp
  clang/test/Modules/Inputs/DependsOnModule.framework/module.map
  clang/test/Modules/requires.m


Index: clang/test/Modules/requires.m
===
--- clang/test/Modules/requires.m
+++ clang/test/Modules/requires.m
@@ -22,11 +22,17 @@
 @import DependsOnModule.CXX14; // expected-note {{module imported here}}
 // expected-error@DependsOnModule.framework/module.map:46 {{module 
'DependsOnModule.CXX17' requires feature 'cplusplus17'}}
 @import DependsOnModule.CXX17; // expected-note {{module imported here}}
+// expected-error@DependsOnModule.framework/module.map:49 {{module 
'DependsOnModule.CXX20' requires feature 'cplusplus20'}}
+@import DependsOnModule.CXX20; // expected-note {{module imported here}}
+// expected-error@DependsOnModule.framework/module.map:52 {{module 
'DependsOnModule.CXX23' requires feature 'cplusplus23'}}
+@import DependsOnModule.CXX23; // expected-note {{module imported here}}
+// expected-error@DependsOnModule.framework/module.map:55 {{module 
'DependsOnModule.CXX26' requires feature 'cplusplus26'}}
+@import DependsOnModule.CXX26; // expected-note {{module imported here}}
 #else
-// expected-error@DependsOnModule.framework/module.map:49 {{module 
'DependsOnModule.C99' requires feature 'c99'}}
+// expected-error@DependsOnModule.framework/module.map:58 {{module 
'DependsOnModule.C99' requires feature 'c99'}}
 @import DependsOnModule.C99; // expected-note {{module imported here}}
-// expected-error@DependsOnModule.framework/module.map:52 {{module 
'DependsOnModule.C11' requires feature 'c11'}}
+// expected-error@DependsOnModule.framework/module.map:61 {{module 
'DependsOnModule.C11' requires feature 'c11'}}
 @import DependsOnModule.C11; // expected-note {{module imported here}}
-// expected-error@DependsOnModule.framework/module.map:55 {{module 
'DependsOnModule.C17' requires feature 'c17'}}
+// expected-error@DependsOnModule.framework/module.map:64 {{module 
'DependsOnModule.C17' requires feature 'c17'}}
 @import DependsOnModule.C17; // expected-note {{module imported here}}
 #endif
Index: clang/test/Modules/Inputs/DependsOnModule.framework/module.map
===
--- clang/test/Modules/Inputs/DependsOnModule.framework/module.map
+++ clang/test/Modules/Inputs/DependsOnModule.framework/module.map
@@ -46,6 +46,15 @@
   explicit module CXX17 {
 requires cplusplus17
   }
+  explicit module CXX20 {
+requires cplusplus20
+  }
+  explicit module CXX23 {
+requires cplusplus23
+  }
+  explicit module CXX26 {
+requires cplusplus26
+  }
   explicit module C99 {
 requires c99
   }
Index: clang/lib/Basic/Module.cpp
===
--- clang/lib/Basic/Module.cpp
+++ clang/lib/Basic/Module.cpp
@@ -107,6 +107,9 @@
 .Case("cplusplus11", LangOpts.CPlusPlus11)
 .Case("cplusplus14", LangOpts.CPlusPlus14)
 .Case("cplusplus17", LangOpts.CPlusPlus17)
+.Case("cplusplus20", LangOpts.CPlusPlus20)
+.Case("cplusplus23", LangOpts.CPlusPlus23)
+.Case("cplusplus26", LangOpts.CPlusPlus26)
 .Case("c99", LangOpts.C99)
 .Case("c11", LangOpts.C11)
 .Case("c17", LangOpts.C17)
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -94,6 +94,7 @@
 - Clang now implements `[temp.deduct]p9`. Substitution failures inside lambdas 
from
   unevaluated contexts will be surfaced as errors. They were previously 
handled as
   SFINAE.
+- Clang now supports `requires cplusplus20` for module maps.
 
 C++23 Feature Support
 ^
@@ -108,6 +109,7 @@
   longer have to be constexpr compatible but rather support a less restricted 
requirements for constexpr
   functions. Which include allowing non-literal types as return values and 
parameters, allow calling of
   non-constexpr functions and constructors.
+- Clang now supports `requires cplusplus23` for module maps.
 
 C++2c Feature Support
 ^
Index: clang/docs/Modules.rst
===
--- clang/docs/Modules.rst
+++ clang/docs/Modules.rst
@@ -573,6 +573,12 @@
 cplusplus17
   C++17 support is available.
 
+cplusplus20
+  C++20 

[PATCH] D150773: [clang][modules] Add features for recent C++ versions

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld updated this revision to Diff 523021.
Hahnfeld added a comment.

> LGTM, but please add a release note when landing.

There wasn't a category specific to modules, so I just added two entries for 
C++20 and C++23 - ok?


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

https://reviews.llvm.org/D150773

Files:
  clang/docs/Modules.rst
  clang/docs/ReleaseNotes.rst
  clang/lib/Basic/Module.cpp
  clang/test/Modules/Inputs/DependsOnModule.framework/module.map
  clang/test/Modules/requires.m


Index: clang/test/Modules/requires.m
===
--- clang/test/Modules/requires.m
+++ clang/test/Modules/requires.m
@@ -22,11 +22,17 @@
 @import DependsOnModule.CXX14; // expected-note {{module imported here}}
 // expected-error@DependsOnModule.framework/module.map:46 {{module 
'DependsOnModule.CXX17' requires feature 'cplusplus17'}}
 @import DependsOnModule.CXX17; // expected-note {{module imported here}}
+// expected-error@DependsOnModule.framework/module.map:49 {{module 
'DependsOnModule.CXX20' requires feature 'cplusplus20'}}
+@import DependsOnModule.CXX20; // expected-note {{module imported here}}
+// expected-error@DependsOnModule.framework/module.map:52 {{module 
'DependsOnModule.CXX23' requires feature 'cplusplus23'}}
+@import DependsOnModule.CXX23; // expected-note {{module imported here}}
+// expected-error@DependsOnModule.framework/module.map:55 {{module 
'DependsOnModule.CXX26' requires feature 'cplusplus26'}}
+@import DependsOnModule.CXX26; // expected-note {{module imported here}}
 #else
-// expected-error@DependsOnModule.framework/module.map:49 {{module 
'DependsOnModule.C99' requires feature 'c99'}}
+// expected-error@DependsOnModule.framework/module.map:58 {{module 
'DependsOnModule.C99' requires feature 'c99'}}
 @import DependsOnModule.C99; // expected-note {{module imported here}}
-// expected-error@DependsOnModule.framework/module.map:52 {{module 
'DependsOnModule.C11' requires feature 'c11'}}
+// expected-error@DependsOnModule.framework/module.map:61 {{module 
'DependsOnModule.C11' requires feature 'c11'}}
 @import DependsOnModule.C11; // expected-note {{module imported here}}
-// expected-error@DependsOnModule.framework/module.map:55 {{module 
'DependsOnModule.C17' requires feature 'c17'}}
+// expected-error@DependsOnModule.framework/module.map:64 {{module 
'DependsOnModule.C17' requires feature 'c17'}}
 @import DependsOnModule.C17; // expected-note {{module imported here}}
 #endif
Index: clang/test/Modules/Inputs/DependsOnModule.framework/module.map
===
--- clang/test/Modules/Inputs/DependsOnModule.framework/module.map
+++ clang/test/Modules/Inputs/DependsOnModule.framework/module.map
@@ -46,6 +46,15 @@
   explicit module CXX17 {
 requires cplusplus17
   }
+  explicit module CXX20 {
+requires cplusplus20
+  }
+  explicit module CXX23 {
+requires cplusplus23
+  }
+  explicit module CXX26 {
+requires cplusplus26
+  }
   explicit module C99 {
 requires c99
   }
Index: clang/lib/Basic/Module.cpp
===
--- clang/lib/Basic/Module.cpp
+++ clang/lib/Basic/Module.cpp
@@ -107,6 +107,9 @@
 .Case("cplusplus11", LangOpts.CPlusPlus11)
 .Case("cplusplus14", LangOpts.CPlusPlus14)
 .Case("cplusplus17", LangOpts.CPlusPlus17)
+.Case("cplusplus20", LangOpts.CPlusPlus20)
+.Case("cplusplus23", LangOpts.CPlusPlus23)
+.Case("cplusplus26", LangOpts.CPlusPlus26)
 .Case("c99", LangOpts.C99)
 .Case("c11", LangOpts.C11)
 .Case("c17", LangOpts.C17)
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -94,6 +94,7 @@
 - Clang now implements `[temp.deduct]p9`. Substitution failures inside lambdas 
from
   unevaluated contexts will be surfaced as errors. They were previously 
handled as
   SFINAE.
+- Clang now supports `requires cplusplus20` for module maps.
 
 C++23 Feature Support
 ^
@@ -108,6 +109,7 @@
   longer have to be constexpr compatible but rather support a less restricted 
requirements for constexpr
   functions. Which include allowing non-literal types as return values and 
parameters, allow calling of
   non-constexpr functions and constructors.
+- Clang now supports `requires cplusplus23` for module maps.
 
 C++2c Feature Support
 ^
Index: clang/docs/Modules.rst
===
--- clang/docs/Modules.rst
+++ clang/docs/Modules.rst
@@ -573,6 +573,12 @@
 cplusplus17
   C++17 support is available.
 
+cplusplus20
+  C++20 support is available.
+
+cplusplus23
+  C++23 support is 

[PATCH] D150773: [clang][modules] Add features for recent C++ versions

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld created this revision.
Hahnfeld added reviewers: ChuanqiXu, Bigcheese, v.g.vassilev, aaron.ballman.
Herald added a project: All.
Hahnfeld requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Add `cplusplus20`, `cplusplus23`, and `cplusplus26` (but don't document the 
latter, following the current policy).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150773

Files:
  clang/docs/Modules.rst
  clang/lib/Basic/Module.cpp
  clang/test/Modules/Inputs/DependsOnModule.framework/module.map
  clang/test/Modules/requires.m


Index: clang/test/Modules/requires.m
===
--- clang/test/Modules/requires.m
+++ clang/test/Modules/requires.m
@@ -22,11 +22,17 @@
 @import DependsOnModule.CXX14; // expected-note {{module imported here}}
 // expected-error@DependsOnModule.framework/module.map:46 {{module 
'DependsOnModule.CXX17' requires feature 'cplusplus17'}}
 @import DependsOnModule.CXX17; // expected-note {{module imported here}}
+// expected-error@DependsOnModule.framework/module.map:49 {{module 
'DependsOnModule.CXX20' requires feature 'cplusplus20'}}
+@import DependsOnModule.CXX20; // expected-note {{module imported here}}
+// expected-error@DependsOnModule.framework/module.map:52 {{module 
'DependsOnModule.CXX23' requires feature 'cplusplus23'}}
+@import DependsOnModule.CXX23; // expected-note {{module imported here}}
+// expected-error@DependsOnModule.framework/module.map:55 {{module 
'DependsOnModule.CXX26' requires feature 'cplusplus26'}}
+@import DependsOnModule.CXX26; // expected-note {{module imported here}}
 #else
-// expected-error@DependsOnModule.framework/module.map:49 {{module 
'DependsOnModule.C99' requires feature 'c99'}}
+// expected-error@DependsOnModule.framework/module.map:58 {{module 
'DependsOnModule.C99' requires feature 'c99'}}
 @import DependsOnModule.C99; // expected-note {{module imported here}}
-// expected-error@DependsOnModule.framework/module.map:52 {{module 
'DependsOnModule.C11' requires feature 'c11'}}
+// expected-error@DependsOnModule.framework/module.map:61 {{module 
'DependsOnModule.C11' requires feature 'c11'}}
 @import DependsOnModule.C11; // expected-note {{module imported here}}
-// expected-error@DependsOnModule.framework/module.map:55 {{module 
'DependsOnModule.C17' requires feature 'c17'}}
+// expected-error@DependsOnModule.framework/module.map:64 {{module 
'DependsOnModule.C17' requires feature 'c17'}}
 @import DependsOnModule.C17; // expected-note {{module imported here}}
 #endif
Index: clang/test/Modules/Inputs/DependsOnModule.framework/module.map
===
--- clang/test/Modules/Inputs/DependsOnModule.framework/module.map
+++ clang/test/Modules/Inputs/DependsOnModule.framework/module.map
@@ -46,6 +46,15 @@
   explicit module CXX17 {
 requires cplusplus17
   }
+  explicit module CXX20 {
+requires cplusplus20
+  }
+  explicit module CXX23 {
+requires cplusplus23
+  }
+  explicit module CXX26 {
+requires cplusplus26
+  }
   explicit module C99 {
 requires c99
   }
Index: clang/lib/Basic/Module.cpp
===
--- clang/lib/Basic/Module.cpp
+++ clang/lib/Basic/Module.cpp
@@ -107,6 +107,9 @@
 .Case("cplusplus11", LangOpts.CPlusPlus11)
 .Case("cplusplus14", LangOpts.CPlusPlus14)
 .Case("cplusplus17", LangOpts.CPlusPlus17)
+.Case("cplusplus20", LangOpts.CPlusPlus20)
+.Case("cplusplus23", LangOpts.CPlusPlus23)
+.Case("cplusplus26", LangOpts.CPlusPlus26)
 .Case("c99", LangOpts.C99)
 .Case("c11", LangOpts.C11)
 .Case("c17", LangOpts.C17)
Index: clang/docs/Modules.rst
===
--- clang/docs/Modules.rst
+++ clang/docs/Modules.rst
@@ -573,6 +573,12 @@
 cplusplus17
   C++17 support is available.
 
+cplusplus20
+  C++20 support is available.
+
+cplusplus23
+  C++23 support is available.
+
 c99
   C99 support is available.
 


Index: clang/test/Modules/requires.m
===
--- clang/test/Modules/requires.m
+++ clang/test/Modules/requires.m
@@ -22,11 +22,17 @@
 @import DependsOnModule.CXX14; // expected-note {{module imported here}}
 // expected-error@DependsOnModule.framework/module.map:46 {{module 'DependsOnModule.CXX17' requires feature 'cplusplus17'}}
 @import DependsOnModule.CXX17; // expected-note {{module imported here}}
+// expected-error@DependsOnModule.framework/module.map:49 {{module 'DependsOnModule.CXX20' requires feature 'cplusplus20'}}
+@import DependsOnModule.CXX20; // expected-note {{module imported here}}
+// expected-error@DependsOnModule.framework/module.map:52 

[PATCH] D149551: [Interpreter] Filter out RISC-V +relax feature

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld abandoned this revision.
Hahnfeld added a comment.

Linker relaxation and proper alignment handling for RISC-V is implemented in 
https://reviews.llvm.org/D149526, so this isn't needed anymore.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149551

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

[PATCH] D146389: [clang-repl][CUDA] Initial interactive CUDA support for clang-repl

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

Some comments, but otherwise LGTM




Comment at: clang/include/clang/Interpreter/Interpreter.h:43
 public:
+  IncrementalCompilerBuilder(){};
+

and this should probably be run through `clang-format`...



Comment at: clang/include/clang/Interpreter/Interpreter.h:45
+
+  void SetCompilerArgs(const std::vector Args) {
+UserArgs = Args;





Comment at: clang/lib/CodeGen/CodeGenModule.cpp:6257
+  // Device code should not be at top level.
+  if (LangOpts.CUDA && LangOpts.CUDAIsDevice)
+return;

tra wrote:
> Could you give me an example of what exactly we'll be skipping here?
> Will it affect `__device__` variables? 
> 
This concerns statements at the global scope that only concern the REPL; see 
https://reviews.llvm.org/D127284 for the original revision. Global variables on 
the other hand are passed via `EmitTopLevelDecl` -> `EmitGlobal`.



Comment at: clang/lib/Interpreter/Interpreter.cpp:143
   // FIXME: Print proper driver diagnostics if the driver flags are wrong.
   // We do C++ by default; append right after argv[0] if no "-x" given
   ClangArgv.insert(ClangArgv.end(), "-Xclang");

This comment should move as well



Comment at: clang/lib/Interpreter/Interpreter.cpp:144-146
   ClangArgv.insert(ClangArgv.end(), "-Xclang");
   ClangArgv.insert(ClangArgv.end(), "-fincremental-extensions");
   ClangArgv.insert(ClangArgv.end(), "-c");

This doesn't do what the comments claim - it appends at the end, not prepends. 
For that it would need to be `ClangArgv.insert(ClangArgv.begin(), "-c")`. 
@v.g.vassilev what do we want here? (probably doesn't block this revision, but 
it's odd nevertheless)



Comment at: clang/lib/Interpreter/Offload.h:36-37
+
+  // Write last PTX to the fatbinary file
+  // llvm::Error WriteFatbinary() const;
+

unused


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146389

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

[PATCH] D149551: [Interpreter] Filter out RISC-V +relax feature

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld created this revision.
Hahnfeld added reviewers: v.g.vassilev, sgraenitz, lhames, StephenFan.
Herald added subscribers: VincentWu, vkmr, luismarques, sameer.abuasal, 
s.egerton, Jim, PkmX, rogfer01, shiva0217, kito-cheng, simoncook, arichardson.
Herald added a project: All.
Hahnfeld requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

JITLink for RISC-V does not implement linker relaxation and until now
only ignored the related alignment relocations. However, the logic has
a flaw and it is actually not valid to ignore them. Just disable the
feature to have less problems.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149551

Files:
  clang/lib/Interpreter/IncrementalExecutor.cpp


Index: clang/lib/Interpreter/IncrementalExecutor.cpp
===
--- clang/lib/Interpreter/IncrementalExecutor.cpp
+++ clang/lib/Interpreter/IncrementalExecutor.cpp
@@ -43,7 +43,12 @@
   llvm::ErrorAsOutParameter EAO();
 
   auto JTMB = JITTargetMachineBuilder(TI.getTriple());
-  JTMB.addFeatures(TI.getTargetOpts().Features);
+  for (const auto  : TI.getTargetOpts().Features) {
+if (F == "+relax")
+  continue;
+JTMB.getFeatures().AddFeature(F);
+  }
+
   LLJITBuilder Builder;
   Builder.setJITTargetMachineBuilder(JTMB);
   // Enable debugging of JIT'd code (only works on JITLink for ELF and MachO).


Index: clang/lib/Interpreter/IncrementalExecutor.cpp
===
--- clang/lib/Interpreter/IncrementalExecutor.cpp
+++ clang/lib/Interpreter/IncrementalExecutor.cpp
@@ -43,7 +43,12 @@
   llvm::ErrorAsOutParameter EAO();
 
   auto JTMB = JITTargetMachineBuilder(TI.getTriple());
-  JTMB.addFeatures(TI.getTargetOpts().Features);
+  for (const auto  : TI.getTargetOpts().Features) {
+if (F == "+relax")
+  continue;
+JTMB.getFeatures().AddFeature(F);
+  }
+
   LLJITBuilder Builder;
   Builder.setJITTargetMachineBuilder(JTMB);
   // Enable debugging of JIT'd code (only works on JITLink for ELF and MachO).
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146389: [clang-repl][CUDA] Initial interactive CUDA support for clang-repl

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added inline comments.



Comment at: clang/lib/CodeGen/CodeGenAction.cpp:296
   }
+  LinkModules.clear();
   return false; // success

v.g.vassilev wrote:
> argentite wrote:
> > Hahnfeld wrote:
> > > This looks like a change that has implications beyond support for CUDA. 
> > > Would it make sense to break this out into a separate review, ie does 
> > > this change something in the current setup?
> > It actually was a separate patch: D146388 Should I submit that for review?
> > It seems to be required because we call `LinkInModules()` once for every 
> > interpreter iteration.
> > 
> The problem with going for a separate change is that we cannot test it. 
> Landing it without a test makes the history commit unclear. This patch (and 
> the tests we add here) will at least indirectly test that change.
Ok, that's why I was asking if it changes something in the current setup (ie 
can be tested). Thanks for clarifying.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146389

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

[PATCH] D146389: [clang-repl][CUDA] Initial interactive CUDA support for clang-repl

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added inline comments.



Comment at: clang/include/clang/Interpreter/Interpreter.h:43-44
 public:
   static llvm::Expected>
   create(std::vector );
+  static llvm::Expected>

If I understand the change correctly, the "old" `create` function on its own is 
not sufficient anymore to create a fully working `CompilerInstance` - should we 
make it `private`? (and unify the two `Builder` classes into one?)

Alternatively, we could keep the current function working and move the bulk of 
the work to an `createInternal` function. What do you think?



Comment at: clang/lib/CodeGen/CodeGenAction.cpp:296
   }
+  LinkModules.clear();
   return false; // success

This looks like a change that has implications beyond support for CUDA. Would 
it make sense to break this out into a separate review, ie does this change 
something in the current setup?



Comment at: clang/lib/Interpreter/IncrementalParser.cpp:125
 
+static CodeGenerator *getCodeGen(FrontendAction *Act);
+

Is it possible to move the `static` function here instead of forward declaring? 
Or does it depend on other functions in this file?



Comment at: clang/lib/Interpreter/IncrementalParser.cpp:143-156
+  // An initial PTU is needed as CUDA includes some headers automatically
+  auto PTU = ParseOrWrapTopLevelDecl();
+  if (auto E = PTU.takeError()) {
+consumeError(std::move(E)); // FIXME
+return; // PTU.takeError();
+  }
+

Should this be done as part of the "generic" `IncrementalParser` constructor, 
or only in the CUDA case by something else? Maybe `Interpreter::createWithCUDA`?



Comment at: clang/lib/Interpreter/Interpreter.cpp:270-274
+  if (DeviceParser) {
+auto DevicePTU = DeviceParser->Parse(Code);
+if (auto E = DevicePTU.takeError())
+  return E;
+  }

Just wondering about the order here: Do we have to parse the device side first? 
Does it make a difference for diagnostics? Maybe you can add a comment about 
the choice here...



Comment at: clang/lib/Interpreter/Offload.cpp:1
+//===-- Offload.cpp - CUDA Offloading ---*- C++ 
-*-===//
+//

v.g.vassilev wrote:
> How about `DeviceOffload.cpp`?
Or `IncrementalCUDADeviceParser.cpp` for the moment - not sure what other 
classes will be added in the future, and if they should be in the same TU.



Comment at: clang/lib/Interpreter/Offload.cpp:90-91
+  PTXCode += '\0';
+  while (PTXCode.size() % 8)
+PTXCode += '\0';
+  return PTXCode.str();

Is padding to 8 bytes a requirement for PTX? Maybe add a coment...



Comment at: clang/lib/Interpreter/Offload.h:20
+
+class DeviceCodeInlinerAction;
+

unused


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146389

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

[PATCH] D146497: libclang: Pass Clang install directory to driver via argv[0].

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

In D146497#4216226 , @aaron.ballman 
wrote:

> The issue doesn't reproduce for me locally on my Windows machine, so it may 
> be something specific to the VE setup.

FWIW I'm not doing anything specific for VE, just

  set(CMAKE_C_COMPILER "clang" CACHE STRING "")
  set(CMAKE_CXX_COMPILER "clang++" CACHE STRING "")
  set(CMAKE_BUILD_TYPE "RelWithDebInfo" CACHE STRING "")
  set(LLVM_APPEND_VC_REV OFF CACHE STRING "")
  set(LLVM_ENABLE_ASSERTIONS ON CACHE BOOL "")
  set(LLVM_ENABLE_PROJECTS "clang" CACHE STRING "")
  
  set(LLVM_ENABLE_LLD ON CACHE BOOL "")
  set(LLVM_LINK_LLVM_DYLIB ON CACHE BOOL "")

in the file I load as the `initial-cache` (`cmake -C`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146497

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

[PATCH] D146497: libclang: Pass Clang install directory to driver via argv[0].

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

FYI this commit breaks `clang/test/Index/index-file.cu` for me:

  warning: CUDA version 11.8 is only partially supported 
[-Wunknown-cuda-version]
  warning: CUDA version 11.8 is only partially supported 
[-Wunknown-cuda-version]
  /home/jhahnfel/LLVM/src/clang/test/Index/index-file.cu:8:20: error: 
CHECK-HOST-NOT: excluded string found in input
  // CHECK-HOST-NOT: macro definition=__CUDA_ARCH__
 ^
  :6218:48: note: found here
  // CHECK: __clang_cuda_runtime_wrapper.h:70:9: macro definition=__CUDA_ARCH__ 
Extent=[70:9 - 70:27]
 ^~

It //could// be related to the warning about my locally installed CUDA version, 
but `__CUDA_ARCH__` should really not be defined when compiling for the host. 
If you have any idea what's going on here, please let me know - I'm a bit 
puzzled right now...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146497

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

[PATCH] D142606: Lazyly initialize uncommon toolchain detector

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld reopened this revision.
Hahnfeld added a comment.

This broke `clang/test/Driver/rocm-detect.hip` on a number of platforms 
(including the pre-merge checks on this PR), I've reverted for now in 
b5ee4f755fcff56243f6ff0cea9e7a722259304a 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142606

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

[PATCH] D142196: [clang][Lex] Add back PPCallbacks::FileNotFound

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

In D142196#4077708 , @jansvoboda11 
wrote:

> Yes, I meant the `pp-trace` docs. Why would "we didn't find the included 
> file" not be a traceable event?

My thinking is the following: If the preprocessor cannot find the included file 
and the Hook doesn't return `true`, it will run into a fatal error and stop 
parsing the source file. I don't know if that's actually a relevant case, but I 
have no experience with `pp-trace`, so maybe?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142196

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

[PATCH] D142196: [clang][Lex] Add back PPCallbacks::FileNotFound

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

I checked a second time and couldn't find other API docs. I've now pushed this 
change for now (also to get it included in `release/16.x`), but please let me 
know for any follow-ups. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142196

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

[PATCH] D142196: [clang][Lex] Add back PPCallbacks::FileNotFound

[Jonas Hahnfeld via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Hahnfeld marked an inline comment as done.
Closed by commit rG01eb01c7fd7a: [clang][Lex] Add back 
PPCallbacks::FileNotFound (authored by Hahnfeld).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142196

Files:
  clang/include/clang/Lex/PPCallbacks.h
  clang/lib/Lex/PPDirectives.cpp
  clang/unittests/Lex/PPCallbacksTest.cpp

Index: clang/unittests/Lex/PPCallbacksTest.cpp
===
--- clang/unittests/Lex/PPCallbacksTest.cpp
+++ clang/unittests/Lex/PPCallbacksTest.cpp
@@ -444,6 +444,50 @@
   ASSERT_EQ("\"tri\?\?-graph.h\"", GetSourceString(Range));
 }
 
+TEST_F(PPCallbacksTest, FileNotFoundSkipped) {
+  const char *SourceText = "#include \"skipped.h\"\n";
+
+  std::unique_ptr SourceBuf =
+  llvm::MemoryBuffer::getMemBuffer(SourceText);
+  SourceMgr.setMainFileID(SourceMgr.createFileID(std::move(SourceBuf)));
+
+  HeaderSearch HeaderInfo(std::make_shared(), SourceMgr,
+  Diags, LangOpts, Target.get());
+  TrivialModuleLoader ModLoader;
+
+  DiagnosticConsumer *DiagConsumer = new DiagnosticConsumer;
+  DiagnosticsEngine FileNotFoundDiags(DiagID, DiagOpts.get(), DiagConsumer);
+  Preprocessor PP(std::make_shared(), FileNotFoundDiags,
+  LangOpts, SourceMgr, HeaderInfo, ModLoader,
+  /*IILookup=*/nullptr,
+  /*OwnsHeaderSearch=*/false);
+  PP.Initialize(*Target);
+
+  class FileNotFoundCallbacks : public PPCallbacks {
+  public:
+unsigned int NumCalls = 0;
+bool FileNotFound(StringRef FileName) override {
+  NumCalls++;
+  return FileName == "skipped.h";
+}
+  };
+
+  auto *Callbacks = new FileNotFoundCallbacks;
+  PP.addPPCallbacks(std::unique_ptr(Callbacks));
+
+  // Lex source text.
+  PP.EnterMainSourceFile();
+  while (true) {
+Token Tok;
+PP.Lex(Tok);
+if (Tok.is(tok::eof))
+  break;
+  }
+
+  ASSERT_EQ(1u, Callbacks->NumCalls);
+  ASSERT_EQ(0u, DiagConsumer->getNumErrors());
+}
+
 TEST_F(PPCallbacksTest, OpenCLExtensionPragmaEnabled) {
   const char* Source =
 "#pragma OPENCL EXTENSION cl_khr_fp64 : enable\n";
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -2000,6 +2000,10 @@
   if (File)
 return File;
 
+  // Give the clients a chance to silently skip this include.
+  if (Callbacks && Callbacks->FileNotFound(Filename))
+return std::nullopt;
+
   if (SuppressIncludeNotFoundError)
 return std::nullopt;
 
Index: clang/include/clang/Lex/PPCallbacks.h
===
--- clang/include/clang/Lex/PPCallbacks.h
+++ clang/include/clang/Lex/PPCallbacks.h
@@ -83,6 +83,16 @@
const Token ,
SrcMgr::CharacteristicKind FileType) {}
 
+  /// Callback invoked whenever the preprocessor cannot find a file for an
+  /// inclusion directive.
+  ///
+  /// \param FileName The name of the file being included, as written in the
+  /// source code.
+  ///
+  /// \returns true to indicate that the preprocessor should skip this file
+  /// and not issue any diagnostic.
+  virtual bool FileNotFound(StringRef FileName) { return false; }
+
   /// Callback invoked whenever an inclusion directive of
   /// any kind (\c \#include, \c \#import, etc.) has been processed, regardless
   /// of whether the inclusion will actually result in an inclusion.
@@ -451,6 +461,14 @@
 Second->FileSkipped(SkippedFile, FilenameTok, FileType);
   }
 
+  bool FileNotFound(StringRef FileName) override {
+bool Skip = First->FileNotFound(FileName);
+// Make sure to invoke the second callback, no matter if the first already
+// returned true to skip the file.
+Skip |= Second->FileNotFound(FileName);
+return Skip;
+  }
+
   void InclusionDirective(SourceLocation HashLoc, const Token ,
   StringRef FileName, bool IsAngled,
   CharSourceRange FilenameRange,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D142196: [clang][Lex] Add back PPCallbacks::FileNotFound

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld marked an inline comment as done.
Hahnfeld added a comment.

In D142196#4074249 , @jansvoboda11 
wrote:

> Also I think we should mention this API in our docs, right where the removed 
> original used to be.

Which API docs are you referring to? I'm only aware of the `pp-trace` docs 
(updated in https://reviews.llvm.org/D125258 for the removal), but I didn't add 
`FileNotFound` to `pp-trace` because it's more of a hook than a "traceable 
event" - should I add it nevertheless?




Comment at: clang/include/clang/Lex/PPCallbacks.h:87
+  /// Callback invoked whenever an inclusion directive results in a
+  /// file-not-found error.
+  ///

jansvoboda11 wrote:
> The wording is a bit misleading due to:
> ```
>   if (SuppressIncludeNotFoundError)
> return std::nullopt;
> ```
> after invoking this callback. Reading the comment, I assume the diagnostic 
> has already been issued. Can we tweak this to clarify things?
Please let me know if this sounds better.


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

https://reviews.llvm.org/D142196

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

[PATCH] D142196: [clang][Lex] Add back PPCallbacks::FileNotFound

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld updated this revision to Diff 491522.
Hahnfeld added a comment.

Update comment.


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

https://reviews.llvm.org/D142196

Files:
  clang/include/clang/Lex/PPCallbacks.h
  clang/lib/Lex/PPDirectives.cpp
  clang/unittests/Lex/PPCallbacksTest.cpp

Index: clang/unittests/Lex/PPCallbacksTest.cpp
===
--- clang/unittests/Lex/PPCallbacksTest.cpp
+++ clang/unittests/Lex/PPCallbacksTest.cpp
@@ -444,6 +444,50 @@
   ASSERT_EQ("\"tri\?\?-graph.h\"", GetSourceString(Range));
 }
 
+TEST_F(PPCallbacksTest, FileNotFoundSkipped) {
+  const char *SourceText = "#include \"skipped.h\"\n";
+
+  std::unique_ptr SourceBuf =
+  llvm::MemoryBuffer::getMemBuffer(SourceText);
+  SourceMgr.setMainFileID(SourceMgr.createFileID(std::move(SourceBuf)));
+
+  HeaderSearch HeaderInfo(std::make_shared(), SourceMgr,
+  Diags, LangOpts, Target.get());
+  TrivialModuleLoader ModLoader;
+
+  DiagnosticConsumer *DiagConsumer = new DiagnosticConsumer;
+  DiagnosticsEngine FileNotFoundDiags(DiagID, DiagOpts.get(), DiagConsumer);
+  Preprocessor PP(std::make_shared(), FileNotFoundDiags,
+  LangOpts, SourceMgr, HeaderInfo, ModLoader,
+  /*IILookup=*/nullptr,
+  /*OwnsHeaderSearch=*/false);
+  PP.Initialize(*Target);
+
+  class FileNotFoundCallbacks : public PPCallbacks {
+  public:
+unsigned int NumCalls = 0;
+bool FileNotFound(StringRef FileName) override {
+  NumCalls++;
+  return FileName == "skipped.h";
+}
+  };
+
+  auto *Callbacks = new FileNotFoundCallbacks;
+  PP.addPPCallbacks(std::unique_ptr(Callbacks));
+
+  // Lex source text.
+  PP.EnterMainSourceFile();
+  while (true) {
+Token Tok;
+PP.Lex(Tok);
+if (Tok.is(tok::eof))
+  break;
+  }
+
+  ASSERT_EQ(1u, Callbacks->NumCalls);
+  ASSERT_EQ(0u, DiagConsumer->getNumErrors());
+}
+
 TEST_F(PPCallbacksTest, OpenCLExtensionPragmaEnabled) {
   const char* Source =
 "#pragma OPENCL EXTENSION cl_khr_fp64 : enable\n";
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -2000,6 +2000,10 @@
   if (File)
 return File;
 
+  // Give the clients a chance to silently skip this include.
+  if (Callbacks && Callbacks->FileNotFound(Filename))
+return std::nullopt;
+
   if (SuppressIncludeNotFoundError)
 return std::nullopt;
 
Index: clang/include/clang/Lex/PPCallbacks.h
===
--- clang/include/clang/Lex/PPCallbacks.h
+++ clang/include/clang/Lex/PPCallbacks.h
@@ -83,6 +83,16 @@
const Token ,
SrcMgr::CharacteristicKind FileType) {}
 
+  /// Callback invoked whenever the preprocessor cannot find a file for an
+  /// inclusion directive.
+  ///
+  /// \param FileName The name of the file being included, as written in the
+  /// source code.
+  ///
+  /// \returns true to indicate that the preprocessor should skip this file
+  /// and not issue any diagnostic.
+  virtual bool FileNotFound(StringRef FileName) { return false; }
+
   /// Callback invoked whenever an inclusion directive of
   /// any kind (\c \#include, \c \#import, etc.) has been processed, regardless
   /// of whether the inclusion will actually result in an inclusion.
@@ -451,6 +461,14 @@
 Second->FileSkipped(SkippedFile, FilenameTok, FileType);
   }
 
+  bool FileNotFound(StringRef FileName) override {
+bool Skip = First->FileNotFound(FileName);
+// Make sure to invoke the second callback, no matter if the first already
+// returned true to skip the file.
+Skip |= Second->FileNotFound(FileName);
+return Skip;
+  }
+
   void InclusionDirective(SourceLocation HashLoc, const Token ,
   StringRef FileName, bool IsAngled,
   CharSourceRange FilenameRange,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits