[PATCH] D112481: [Sema] fix nondeterminism in ASTContext::getDeducedTemplateSpecializationType

[Igor Sugak via Phabricator via cfe-commits]

sugak added inline comments.



Comment at: clang/lib/AST/ASTContext.cpp:5640
+  DeducedTemplateSpecializationType::Profile(
+  ID, Template, DeducedType, IsDependent || Template.isDependent());
   if (DeducedTemplateSpecializationType *DTST =

bruno wrote:
> Should this be done in the implementation (like done for the ctor)?
> ```
> static void Profile(llvm::FoldingSetNodeID &ID, TemplateName Template,
>   QualType Deduced, bool IsDependent) {
> Template.Profile(ID);
> ID.AddPointer(Deduced.getAsOpaquePtr());
> ID.AddBoolean(IsDependent || Template.isDependent());
>   }
> ```
Thanks for the suggestion! Indeed that look better.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112481

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

[PATCH] D112481: [Sema] fix nondeterminism in ASTContext::getDeducedTemplateSpecializationType

[Igor Sugak via Phabricator via cfe-commits]

sugak updated this revision to Diff 382375.
sugak added a comment.

Updated following @bruno's suggestion, and fixed tests (thanks @weiwang)!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112481

Files:
  clang/include/clang/AST/Type.h
  clang/lib/AST/ASTContext.cpp


Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -5644,6 +5644,9 @@
 
   auto *DTST = new (*this, TypeAlignment)
   DeducedTemplateSpecializationType(Template, DeducedType, IsDependent);
+  llvm::FoldingSetNodeID TempID;
+  DTST->Profile(TempID);
+  assert(ID == TempID && "ID does not match");
   Types.push_back(DTST);
   if (InsertPos)
 DeducedTemplateSpecializationTypes.InsertNode(DTST, InsertPos);
Index: clang/include/clang/AST/Type.h
===
--- clang/include/clang/AST/Type.h
+++ clang/include/clang/AST/Type.h
@@ -5071,8 +5071,10 @@
   static void Profile(llvm::FoldingSetNodeID &ID, TemplateName Template,
   QualType Deduced, bool IsDependent) {
 Template.Profile(ID);
-ID.AddPointer(Deduced.getAsOpaquePtr());
-ID.AddBoolean(IsDependent);
+QualType CanonicalType =
+Deduced.isNull() ? Deduced : Deduced.getCanonicalType();
+ID.AddPointer(CanonicalType.getAsOpaquePtr());
+ID.AddBoolean(IsDependent || Template.isDependent());
   }
 
   static bool classof(const Type *T) {


Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -5644,6 +5644,9 @@
 
   auto *DTST = new (*this, TypeAlignment)
   DeducedTemplateSpecializationType(Template, DeducedType, IsDependent);
+  llvm::FoldingSetNodeID TempID;
+  DTST->Profile(TempID);
+  assert(ID == TempID && "ID does not match");
   Types.push_back(DTST);
   if (InsertPos)
 DeducedTemplateSpecializationTypes.InsertNode(DTST, InsertPos);
Index: clang/include/clang/AST/Type.h
===
--- clang/include/clang/AST/Type.h
+++ clang/include/clang/AST/Type.h
@@ -5071,8 +5071,10 @@
   static void Profile(llvm::FoldingSetNodeID &ID, TemplateName Template,
   QualType Deduced, bool IsDependent) {
 Template.Profile(ID);
-ID.AddPointer(Deduced.getAsOpaquePtr());
-ID.AddBoolean(IsDependent);
+QualType CanonicalType =
+Deduced.isNull() ? Deduced : Deduced.getCanonicalType();
+ID.AddPointer(CanonicalType.getAsOpaquePtr());
+ID.AddBoolean(IsDependent || Template.isDependent());
   }
 
   static bool classof(const Type *T) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112481: [Sema] fix nondeterminism in ASTContext::getDeducedTemplateSpecializationType

[Igor Sugak via Phabricator via cfe-commits]

sugak added a comment.

ping @rsmith


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112481

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

[PATCH] D71731: [Format] fix dereference of pointers in co_yeld and co_return statements

[Igor Sugak via Phabricator via cfe-commits]

sugak created this revision.
sugak added reviewers: modocache, sammccall, arthur.j.odwyer.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
sugak edited the summary of this revision.

  // Before:
  co_yield* x;
  co_return* x;
  // After:
  co_yield *x;
  co_return *x;

Add unit-test to cover these cases.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71731

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -7223,6 +7223,8 @@
   verifyIndependentOfContext("return 10 * b;");
   verifyIndependentOfContext("return *b * *c;");
   verifyIndependentOfContext("return a & ~b;");
+  verifyIndependentOfContext("co_yield *b * *c;");
+  verifyIndependentOfContext("co_return *b * *c;");
   verifyIndependentOfContext("f(b ? *c : *d);");
   verifyIndependentOfContext("int a = b ? *c : *d;");
   verifyIndependentOfContext("*b = a;");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1766,7 +1766,8 @@
 if (PrevToken->isOneOf(tok::l_paren, tok::l_square, tok::l_brace,
tok::comma, tok::semi, tok::kw_return, tok::colon,
tok::equal, tok::kw_delete, tok::kw_sizeof,
-   tok::kw_throw) ||
+   tok::kw_throw, tok::kw_co_return,
+   tok::kw_co_yield) ||
 PrevToken->isOneOf(TT_BinaryOperator, TT_ConditionalExpr,
TT_UnaryOperator, TT_CastRParen))
   return TT_UnaryOperator;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -7223,6 +7223,8 @@
   verifyIndependentOfContext("return 10 * b;");
   verifyIndependentOfContext("return *b * *c;");
   verifyIndependentOfContext("return a & ~b;");
+  verifyIndependentOfContext("co_yield *b * *c;");
+  verifyIndependentOfContext("co_return *b * *c;");
   verifyIndependentOfContext("f(b ? *c : *d);");
   verifyIndependentOfContext("int a = b ? *c : *d;");
   verifyIndependentOfContext("*b = a;");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1766,7 +1766,8 @@
 if (PrevToken->isOneOf(tok::l_paren, tok::l_square, tok::l_brace,
tok::comma, tok::semi, tok::kw_return, tok::colon,
tok::equal, tok::kw_delete, tok::kw_sizeof,
-   tok::kw_throw) ||
+   tok::kw_throw, tok::kw_co_return,
+   tok::kw_co_yield) ||
 PrevToken->isOneOf(TT_BinaryOperator, TT_ConditionalExpr,
TT_UnaryOperator, TT_CastRParen))
   return TT_UnaryOperator;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39478: [clang-format] Handle leading comments in using declaration

[Igor Sugak via Phabricator via cfe-commits]

sugak created this revision.

This fixes clang-format internal assertion for the following code:

  /* override */ using std::string;

Added add a unit test.


https://reviews.llvm.org/D39478

Files:
  lib/Format/UsingDeclarationsSorter.cpp
  unittests/Format/UsingDeclarationsSorterTest.cpp


Index: unittests/Format/UsingDeclarationsSorterTest.cpp
===
--- unittests/Format/UsingDeclarationsSorterTest.cpp
+++ unittests/Format/UsingDeclarationsSorterTest.cpp
@@ -328,6 +328,13 @@
   {tooling::Range(19, 1)}));
 }
 
+TEST_F(UsingDeclarationsSorterTest, 
SortsUsingDeclarationsWithLeadingkComments) {
+  EXPECT_EQ("/* comment */ using a;\n"
+"/* comment */ using b;",
+sortUsingDeclarations("/* comment */ using b;\n"
+  "/* comment */ using a;"));
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: lib/Format/UsingDeclarationsSorter.cpp
===
--- lib/Format/UsingDeclarationsSorter.cpp
+++ lib/Format/UsingDeclarationsSorter.cpp
@@ -133,15 +133,18 @@
   tooling::Replacements Fixes;
   SmallVector UsingDeclarations;
   for (size_t I = 0, E = AnnotatedLines.size(); I != E; ++I) {
+auto FirstTok = AnnotatedLines[I]->First;
 if (AnnotatedLines[I]->InPPDirective ||
-!AnnotatedLines[I]->startsWith(tok::kw_using) ||
-AnnotatedLines[I]->First->Finalized) {
+!AnnotatedLines[I]->startsWith(tok::kw_using) || FirstTok->Finalized) {
   endUsingDeclarationBlock(&UsingDeclarations, SourceMgr, &Fixes);
   continue;
 }
-if (AnnotatedLines[I]->First->NewlinesBefore > 1)
+if (FirstTok->NewlinesBefore > 1) {
   endUsingDeclarationBlock(&UsingDeclarations, SourceMgr, &Fixes);
-std::string Label = computeUsingDeclarationLabel(AnnotatedLines[I]->First);
+}
+auto UsingTok =
+FirstTok->is(tok::comment) ? FirstTok->getNextNonComment() : FirstTok;
+std::string Label = computeUsingDeclarationLabel(UsingTok);
 if (Label.empty()) {
   endUsingDeclarationBlock(&UsingDeclarations, SourceMgr, &Fixes);
   continue;


Index: unittests/Format/UsingDeclarationsSorterTest.cpp
===
--- unittests/Format/UsingDeclarationsSorterTest.cpp
+++ unittests/Format/UsingDeclarationsSorterTest.cpp
@@ -328,6 +328,13 @@
   {tooling::Range(19, 1)}));
 }
 
+TEST_F(UsingDeclarationsSorterTest, SortsUsingDeclarationsWithLeadingkComments) {
+  EXPECT_EQ("/* comment */ using a;\n"
+"/* comment */ using b;",
+sortUsingDeclarations("/* comment */ using b;\n"
+  "/* comment */ using a;"));
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: lib/Format/UsingDeclarationsSorter.cpp
===
--- lib/Format/UsingDeclarationsSorter.cpp
+++ lib/Format/UsingDeclarationsSorter.cpp
@@ -133,15 +133,18 @@
   tooling::Replacements Fixes;
   SmallVector UsingDeclarations;
   for (size_t I = 0, E = AnnotatedLines.size(); I != E; ++I) {
+auto FirstTok = AnnotatedLines[I]->First;
 if (AnnotatedLines[I]->InPPDirective ||
-!AnnotatedLines[I]->startsWith(tok::kw_using) ||
-AnnotatedLines[I]->First->Finalized) {
+!AnnotatedLines[I]->startsWith(tok::kw_using) || FirstTok->Finalized) {
   endUsingDeclarationBlock(&UsingDeclarations, SourceMgr, &Fixes);
   continue;
 }
-if (AnnotatedLines[I]->First->NewlinesBefore > 1)
+if (FirstTok->NewlinesBefore > 1) {
   endUsingDeclarationBlock(&UsingDeclarations, SourceMgr, &Fixes);
-std::string Label = computeUsingDeclarationLabel(AnnotatedLines[I]->First);
+}
+auto UsingTok =
+FirstTok->is(tok::comment) ? FirstTok->getNextNonComment() : FirstTok;
+std::string Label = computeUsingDeclarationLabel(UsingTok);
 if (Label.empty()) {
   endUsingDeclarationBlock(&UsingDeclarations, SourceMgr, &Fixes);
   continue;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39478: [clang-format] Handle leading comments in using declaration

[Igor Sugak via Phabricator via cfe-commits]

sugak added inline comments.



Comment at: lib/Format/UsingDeclarationsSorter.cpp:50
 std::string computeUsingDeclarationLabel(const FormatToken *UsingTok) {
   assert(UsingTok && UsingTok->is(tok::kw_using) && "Expecting a using token");
   std::string Label;

and this is the assertions that fails for the code referenced in the summary.


https://reviews.llvm.org/D39478



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

[PATCH] D39478: [clang-format] Handle leading comments in using declaration

[Igor Sugak via Phabricator via cfe-commits]

sugak updated this revision to Diff 121165.
sugak marked 3 inline comments as done.
sugak added a comment.

Updated per comments.

@djasper : Thank you for the review! Would you commit this for me?


https://reviews.llvm.org/D39478

Files:
  lib/Format/UsingDeclarationsSorter.cpp
  unittests/Format/UsingDeclarationsSorterTest.cpp


Index: unittests/Format/UsingDeclarationsSorterTest.cpp
===
--- unittests/Format/UsingDeclarationsSorterTest.cpp
+++ unittests/Format/UsingDeclarationsSorterTest.cpp
@@ -328,6 +328,13 @@
   {tooling::Range(19, 1)}));
 }
 
+TEST_F(UsingDeclarationsSorterTest, 
SortsUsingDeclarationsWithLeadingkComments) {
+  EXPECT_EQ("/* comment */ using a;\n"
+"/* comment */ using b;",
+sortUsingDeclarations("/* comment */ using b;\n"
+  "/* comment */ using a;"));
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: lib/Format/UsingDeclarationsSorter.cpp
===
--- lib/Format/UsingDeclarationsSorter.cpp
+++ lib/Format/UsingDeclarationsSorter.cpp
@@ -133,15 +133,17 @@
   tooling::Replacements Fixes;
   SmallVector UsingDeclarations;
   for (size_t I = 0, E = AnnotatedLines.size(); I != E; ++I) {
+const auto *FirstTok = AnnotatedLines[I]->First;
 if (AnnotatedLines[I]->InPPDirective ||
-!AnnotatedLines[I]->startsWith(tok::kw_using) ||
-AnnotatedLines[I]->First->Finalized) {
+!AnnotatedLines[I]->startsWith(tok::kw_using) || FirstTok->Finalized) {
   endUsingDeclarationBlock(&UsingDeclarations, SourceMgr, &Fixes);
   continue;
 }
-if (AnnotatedLines[I]->First->NewlinesBefore > 1)
+if (FirstTok->NewlinesBefore > 1)
   endUsingDeclarationBlock(&UsingDeclarations, SourceMgr, &Fixes);
-std::string Label = computeUsingDeclarationLabel(AnnotatedLines[I]->First);
+const auto *UsingTok =
+FirstTok->is(tok::comment) ? FirstTok->getNextNonComment() : FirstTok;
+std::string Label = computeUsingDeclarationLabel(UsingTok);
 if (Label.empty()) {
   endUsingDeclarationBlock(&UsingDeclarations, SourceMgr, &Fixes);
   continue;


Index: unittests/Format/UsingDeclarationsSorterTest.cpp
===
--- unittests/Format/UsingDeclarationsSorterTest.cpp
+++ unittests/Format/UsingDeclarationsSorterTest.cpp
@@ -328,6 +328,13 @@
   {tooling::Range(19, 1)}));
 }
 
+TEST_F(UsingDeclarationsSorterTest, SortsUsingDeclarationsWithLeadingkComments) {
+  EXPECT_EQ("/* comment */ using a;\n"
+"/* comment */ using b;",
+sortUsingDeclarations("/* comment */ using b;\n"
+  "/* comment */ using a;"));
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: lib/Format/UsingDeclarationsSorter.cpp
===
--- lib/Format/UsingDeclarationsSorter.cpp
+++ lib/Format/UsingDeclarationsSorter.cpp
@@ -133,15 +133,17 @@
   tooling::Replacements Fixes;
   SmallVector UsingDeclarations;
   for (size_t I = 0, E = AnnotatedLines.size(); I != E; ++I) {
+const auto *FirstTok = AnnotatedLines[I]->First;
 if (AnnotatedLines[I]->InPPDirective ||
-!AnnotatedLines[I]->startsWith(tok::kw_using) ||
-AnnotatedLines[I]->First->Finalized) {
+!AnnotatedLines[I]->startsWith(tok::kw_using) || FirstTok->Finalized) {
   endUsingDeclarationBlock(&UsingDeclarations, SourceMgr, &Fixes);
   continue;
 }
-if (AnnotatedLines[I]->First->NewlinesBefore > 1)
+if (FirstTok->NewlinesBefore > 1)
   endUsingDeclarationBlock(&UsingDeclarations, SourceMgr, &Fixes);
-std::string Label = computeUsingDeclarationLabel(AnnotatedLines[I]->First);
+const auto *UsingTok =
+FirstTok->is(tok::comment) ? FirstTok->getNextNonComment() : FirstTok;
+std::string Label = computeUsingDeclarationLabel(UsingTok);
 if (Label.empty()) {
   endUsingDeclarationBlock(&UsingDeclarations, SourceMgr, &Fixes);
   continue;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39587: [clang-format] Handle unary operator overload with arguments and specifiers

[Igor Sugak via Phabricator via cfe-commits]

sugak created this revision.

Before:

  int operator++(int)noexcept;

After:

  int operator++(int) noexcept;


https://reviews.llvm.org/D39587

Files:
  lib/Format/TokenAnnotator.cpp
  unittests/Format/FormatTest.cpp


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -5591,6 +5591,7 @@
   verifyFormat("bool operator!=();");
   verifyFormat("int operator+();");
   verifyFormat("int operator++();");
+  verifyFormat("int operator++(int) volatile noexcept;");
   verifyFormat("bool operator,();");
   verifyFormat("bool operator();");
   verifyFormat("bool operator()();");
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -696,7 +696,8 @@
   CurrentToken->Type = TT_PointerOrReference;
 consumeToken();
 if (CurrentToken &&
-CurrentToken->Previous->isOneOf(TT_BinaryOperator, tok::comma))
+CurrentToken->Previous->isOneOf(TT_BinaryOperator, 
TT_UnaryOperator,
+tok::comma))
   CurrentToken->Previous->Type = TT_OverloadedOperator;
   }
   if (CurrentToken) {


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -5591,6 +5591,7 @@
   verifyFormat("bool operator!=();");
   verifyFormat("int operator+();");
   verifyFormat("int operator++();");
+  verifyFormat("int operator++(int) volatile noexcept;");
   verifyFormat("bool operator,();");
   verifyFormat("bool operator();");
   verifyFormat("bool operator()();");
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -696,7 +696,8 @@
   CurrentToken->Type = TT_PointerOrReference;
 consumeToken();
 if (CurrentToken &&
-CurrentToken->Previous->isOneOf(TT_BinaryOperator, tok::comma))
+CurrentToken->Previous->isOneOf(TT_BinaryOperator, TT_UnaryOperator,
+tok::comma))
   CurrentToken->Previous->Type = TT_OverloadedOperator;
   }
   if (CurrentToken) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39587: [clang-format] Handle unary operator overload with arguments and specifiers

[Igor Sugak via Phabricator via cfe-commits]

sugak added a comment.

@djasper: would you accept and push this for me :)


https://reviews.llvm.org/D39587



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

[PATCH] D39478: [clang-format] Handle leading comments in using declaration

[Igor Sugak via Phabricator via cfe-commits]

sugak added a comment.

@djasper: ping :)


https://reviews.llvm.org/D39478



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

[PATCH] D39478: [clang-format] Handle leading comments in using declaration

[Igor Sugak via Phabricator via cfe-commits]

sugak added a comment.

@djasper: no, I do not.


https://reviews.llvm.org/D39478



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

[PATCH] D112481: [Sema] fix nondeterminism in ASTContext::getDeducedTemplateSpecializationType

[Igor Sugak via Phabricator via cfe-commits]

sugak created this revision.
sugak added a reviewer: rsmith.
Herald added a subscriber: mgrang.
sugak requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

`DeducedTemplateSpecializationTypes` is a  
`llvm::FoldingSet` [1], where  
`FoldingSetNodeID` is based on the values: `TemplateName`, `QualType`, 
`IsDeducedAsDependent`, those values are also used 
`DeducedTemplateSpecializationType` constructor arguments.

The problem is that `FoldingSetNodeID` created by the static 
`DeducedTemplateSpecializationType::Profile` may not be equal 
to`FoldingSetNodeID` created by a member 
`DeducedTemplateSpecializationType::Profile` of instance created with the same 
`TemplateName`, `QualType`, `IsDeducedAsDependent`, which makes 
`DeducedTemplateSpecializationTypes`.

Specifically, while `IsDeducedAsDependent` value is passes to the constructor, 
`IsDependent()` method on the created instance may return a different value, 
because `IsDependent` is not saved as is:

name=clang/include/clang/AST/Type.h
  DeducedTemplateSpecializationType(TemplateName Template,  QualType 
DeducedAsType, bool IsDeducedAsDependent)
  : DeducedType(DeducedTemplateSpecialization, DeducedAsType,
toTypeDependence(Template.getDependence()) | // <~  also 
considers `TemplateName` parameter
(IsDeducedAsDependent ? 
TypeDependence::DependentInstantiation : TypeDependence::None)),

For example, if an instance A with key `FoldingSetNodeID {A, B, false}` is 
inserted. Then a key `FoldingSetNodeID {A, B, true}` is probed: if it happens 
to correspond to the same bucket in `FoldingSet` as the first key, and 
`A.Profile()` returns  `FoldingSetNodeID {A, B, true}`, then we have a cache 
hit. If the bucket for the second key is different from the first key, it's a 
no hit, even if `A.Profile()` returns  `FoldingSetNodeID {A, B, true}`. Since  
`TemplateName`, `QualType` parameter values involve memory pointers, the query 
result depend on allocator, and may differ from run to run.

When this is used as part of modules compilation, it may result in "module out 
of date" errors, if imported modules are built on different machines.

1. https://llvm.org/docs/ProgrammersManual.html#llvm-adt-foldingset-h


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D112481

Files:
  clang/lib/AST/ASTContext.cpp


Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -5491,13 +5491,16 @@
   void *InsertPos = nullptr;
   llvm::FoldingSetNodeID ID;
   DeducedTemplateSpecializationType::Profile(ID, Template, DeducedType,
- IsDependent);
+ IsDependent || 
Template.isDependent());
   if (DeducedTemplateSpecializationType *DTST =
   DeducedTemplateSpecializationTypes.FindNodeOrInsertPos(ID, 
InsertPos))
 return QualType(DTST, 0);
 
   auto *DTST = new (*this, TypeAlignment)
   DeducedTemplateSpecializationType(Template, DeducedType, IsDependent);
+  llvm::FoldingSetNodeID TempID;
+  DTST->Profile(TempID);
+  assert(ID == TempID && "ID does not match");
   Types.push_back(DTST);
   if (InsertPos)
 DeducedTemplateSpecializationTypes.InsertNode(DTST, InsertPos);


Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -5491,13 +5491,16 @@
   void *InsertPos = nullptr;
   llvm::FoldingSetNodeID ID;
   DeducedTemplateSpecializationType::Profile(ID, Template, DeducedType,
- IsDependent);
+ IsDependent || Template.isDependent());
   if (DeducedTemplateSpecializationType *DTST =
   DeducedTemplateSpecializationTypes.FindNodeOrInsertPos(ID, InsertPos))
 return QualType(DTST, 0);
 
   auto *DTST = new (*this, TypeAlignment)
   DeducedTemplateSpecializationType(Template, DeducedType, IsDependent);
+  llvm::FoldingSetNodeID TempID;
+  DTST->Profile(TempID);
+  assert(ID == TempID && "ID does not match");
   Types.push_back(DTST);
   if (InsertPos)
 DeducedTemplateSpecializationTypes.InsertNode(DTST, InsertPos);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112481: [Sema] fix nondeterminism in ASTContext::getDeducedTemplateSpecializationType

[Igor Sugak via Phabricator via cfe-commits]

sugak updated this revision to Diff 382109.
sugak added a comment.

apply clang-format suggestion


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112481

Files:
  clang/lib/AST/ASTContext.cpp


Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -5636,14 +5636,17 @@
   // Look in the folding set for an existing type.
   void *InsertPos = nullptr;
   llvm::FoldingSetNodeID ID;
-  DeducedTemplateSpecializationType::Profile(ID, Template, DeducedType,
- IsDependent);
+  DeducedTemplateSpecializationType::Profile(
+  ID, Template, DeducedType, IsDependent || Template.isDependent());
   if (DeducedTemplateSpecializationType *DTST =
   DeducedTemplateSpecializationTypes.FindNodeOrInsertPos(ID, 
InsertPos))
 return QualType(DTST, 0);
 
   auto *DTST = new (*this, TypeAlignment)
   DeducedTemplateSpecializationType(Template, DeducedType, IsDependent);
+  llvm::FoldingSetNodeID TempID;
+  DTST->Profile(TempID);
+  assert(ID == TempID && "ID does not match");
   Types.push_back(DTST);
   if (InsertPos)
 DeducedTemplateSpecializationTypes.InsertNode(DTST, InsertPos);


Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -5636,14 +5636,17 @@
   // Look in the folding set for an existing type.
   void *InsertPos = nullptr;
   llvm::FoldingSetNodeID ID;
-  DeducedTemplateSpecializationType::Profile(ID, Template, DeducedType,
- IsDependent);
+  DeducedTemplateSpecializationType::Profile(
+  ID, Template, DeducedType, IsDependent || Template.isDependent());
   if (DeducedTemplateSpecializationType *DTST =
   DeducedTemplateSpecializationTypes.FindNodeOrInsertPos(ID, InsertPos))
 return QualType(DTST, 0);
 
   auto *DTST = new (*this, TypeAlignment)
   DeducedTemplateSpecializationType(Template, DeducedType, IsDependent);
+  llvm::FoldingSetNodeID TempID;
+  DTST->Profile(TempID);
+  assert(ID == TempID && "ID does not match");
   Types.push_back(DTST);
   if (InsertPos)
 DeducedTemplateSpecializationTypes.InsertNode(DTST, InsertPos);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70172: [CUDA][HIP][OpenMP] Emit deferred diagnostics by a post-parsing AST travese

[Igor Sugak via Phabricator via cfe-commits]

sugak added subscribers: weiwang, sugak.
sugak added a comment.
Herald added a subscriber: sstefan1.

Hi @yaxunl! I'm working on upgrading a large codebase from LLVM-9 to LLVM-12. I 
noticed on average 10% compilation speed regression that seems to be caused 
this change. We use Clang modules and historically provide `-fopenmp` compiler 
flag by default. The problem seem to be that compiling and importing modules is 
now slower, with the generated modules size increased by 2X. llvm-bcanalyzer 
tool shows that it's dominated by `DECLS_TO_CHECK_FOR_DEFERRED_DIAGS`.  If I 
understand it right, your change is only relevant when target offloading is 
used. I inspected all of `#pragma omp` directives and can confirm that we don't 
use it.

I see that most of this code is gated by OpenMP flag. I wonder if there is a 
finer grain way to enable openmp parallel code generation without target 
offloading? Would it make sense to extend this code to check if 
`-fopenom-targets` is set before recording `DECLS_TO_CHECK_FOR_DEFERRED_DIAGS`?

Note, this was measured @weiwang's https://reviews.llvm.org/D101793.


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

https://reviews.llvm.org/D70172

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