r352436 - [ASTImporter] Fix handling of overriden methods during ASTImport

[Shafik Yaghmour via cfe-commits]

Author: shafik
Date: Mon Jan 28 13:55:33 2019
New Revision: 352436

URL: http://llvm.org/viewvc/llvm-project?rev=352436&view=rev
Log:
[ASTImporter] Fix handling of overriden methods during ASTImport

Summary:
When importing classes we may add a CXXMethodDecl more than once to a 
CXXRecordDecl when handling overrides. This patch will fix the cases we 
currently know about and handle the case where we are only dealing with 
declarations.

Differential Revision: https://reviews.llvm.org/D56936

Modified:
cfe/trunk/lib/AST/ASTImporter.cpp
cfe/trunk/test/Analysis/Inputs/ctu-other.cpp
cfe/trunk/test/Analysis/ctu-main.cpp
cfe/trunk/unittests/AST/ASTImporterTest.cpp

Modified: cfe/trunk/lib/AST/ASTImporter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=352436&r1=352435&r2=352436&view=diff
==
--- cfe/trunk/lib/AST/ASTImporter.cpp (original)
+++ cfe/trunk/lib/AST/ASTImporter.cpp Mon Jan 28 13:55:33 2019
@@ -437,6 +437,8 @@ namespace clang {
 
 Error ImportTemplateInformation(FunctionDecl *FromFD, FunctionDecl *ToFD);
 
+Error ImportFunctionDeclBody(FunctionDecl *FromFD, FunctionDecl *ToFD);
+
 bool IsStructuralMatch(Decl *From, Decl *To, bool Complain);
 bool IsStructuralMatch(RecordDecl *FromRecord, RecordDecl *ToRecord,
bool Complain = true);
@@ -2944,6 +2946,17 @@ ASTNodeImporter::FindFunctionTemplateSpe
   return FoundSpec;
 }
 
+Error ASTNodeImporter::ImportFunctionDeclBody(FunctionDecl *FromFD,
+  FunctionDecl *ToFD) {
+  if (Stmt *FromBody = FromFD->getBody()) {
+if (ExpectedStmt ToBodyOrErr = import(FromBody))
+  ToFD->setBody(*ToBodyOrErr);
+else
+  return ToBodyOrErr.takeError();
+  }
+  return Error::success();
+}
+
 ExpectedDecl ASTNodeImporter::VisitFunctionDecl(FunctionDecl *D) {
 
   SmallVector Redecls = getCanonicalForwardRedeclChain(D);
@@ -2967,7 +2980,7 @@ ExpectedDecl ASTNodeImporter::VisitFunct
   if (ToD)
 return ToD;
 
-  const FunctionDecl *FoundByLookup = nullptr;
+  FunctionDecl *FoundByLookup = nullptr;
   FunctionTemplateDecl *FromFT = D->getDescribedFunctionTemplate();
 
   // If this is a function template specialization, then try to find the same
@@ -3038,6 +3051,25 @@ ExpectedDecl ASTNodeImporter::VisitFunct
 }
   }
 
+  // We do not allow more than one in-class declaration of a function. This is
+  // because AST clients like VTableBuilder asserts on this. VTableBuilder
+  // assumes there is only one in-class declaration. Building a redecl
+  // chain would result in more than one in-class declaration for
+  // overrides (even if they are part of the same redecl chain inside the
+  // derived class.)
+  if (FoundByLookup) {
+if (auto *MD = dyn_cast(FoundByLookup)) {
+  if (D->getLexicalDeclContext() == D->getDeclContext()) {
+if (!D->doesThisDeclarationHaveABody())
+  return Importer.MapImported(D, FoundByLookup);
+else {
+  // Let's continue and build up the redecl chain in this case.
+  // FIXME Merge the functions into one decl.
+}
+  }
+}
+  }
+
   DeclarationNameInfo NameInfo(Name, Loc);
   // Import additional name location/type info.
   if (Error Err = ImportDeclarationNameLoc(D->getNameInfo(), NameInfo))
@@ -3199,12 +3231,10 @@ ExpectedDecl ASTNodeImporter::VisitFunct
   }
 
   if (D->doesThisDeclarationHaveABody()) {
-if (Stmt *FromBody = D->getBody()) {
-  if (ExpectedStmt ToBodyOrErr = import(FromBody))
-ToFunction->setBody(*ToBodyOrErr);
-  else
-return ToBodyOrErr.takeError();
-}
+Error Err = ImportFunctionDeclBody(D, ToFunction);
+
+if (Err)
+  return std::move(Err);
   }
 
   // FIXME: Other bits to merge?

Modified: cfe/trunk/test/Analysis/Inputs/ctu-other.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/Inputs/ctu-other.cpp?rev=352436&r1=352435&r2=352436&view=diff
==
--- cfe/trunk/test/Analysis/Inputs/ctu-other.cpp (original)
+++ cfe/trunk/test/Analysis/Inputs/ctu-other.cpp Mon Jan 28 13:55:33 2019
@@ -24,33 +24,38 @@ namespace embed_ns {
 int fens(int x) {
   return x - 3;
 }
-}
+} // namespace embed_ns
 
 class embed_cls {
 public:
-  int fecl(int x) {
-return x - 7;
-  }
+  int fecl(int x);
 };
+int embed_cls::fecl(int x) {
+  return x - 7;
 }
+} // namespace myns
 
 class mycls {
 public:
-  int fcl(int x) {
-return x + 5;
-  }
-  static int fscl(int x) {
-return x + 6;
-  }
+  int fcl(int x);
+  static int fscl(int x);
 
   class embed_cls2 {
   public:
-int fecl2(int x) {
-  return x - 11;
-}
+int fecl2(int x);
   };
 };
 
+int mycls::fcl(int x) {
+  return x + 5;
+}
+int mycls::fscl(int x) {
+  return x + 6;
+}
+int mycls::embed_cls2::fecl2(int x) {
+  return x - 11;
+}
+
 namespace chns {
 in

r345784 - Revert "[ASTImporter][Structural Eq] Check for isBeingDefined"

[Shafik Yaghmour via cfe-commits]

Author: shafik
Date: Wed Oct 31 14:53:15 2018
New Revision: 345784

URL: http://llvm.org/viewvc/llvm-project?rev=345784&view=rev
Log:
Revert "[ASTImporter][Structural Eq] Check for isBeingDefined"

This reverts commit r345760

because it caused an assertion in the lldb test suite. This is the log from the 
build bot: http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/12003/

Modified:
cfe/trunk/lib/AST/ASTStructuralEquivalence.cpp
cfe/trunk/unittests/AST/ASTImporterTest.cpp

Modified: cfe/trunk/lib/AST/ASTStructuralEquivalence.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTStructuralEquivalence.cpp?rev=345784&r1=345783&r2=345784&view=diff
==
--- cfe/trunk/lib/AST/ASTStructuralEquivalence.cpp (original)
+++ cfe/trunk/lib/AST/ASTStructuralEquivalence.cpp Wed Oct 31 14:53:15 2018
@@ -1016,8 +1016,7 @@ static bool IsStructurallyEquivalent(Str
 return false;
 
   // Compare the definitions of these two records. If either or both are
-  // incomplete (i.e. it is a forward decl), we assume that they are
-  // equivalent.
+  // incomplete, we assume that they are equivalent.
   D1 = D1->getDefinition();
   D2 = D2->getDefinition();
   if (!D1 || !D2)
@@ -1032,11 +1031,6 @@ static bool IsStructurallyEquivalent(Str
 if (D1->hasExternalLexicalStorage() || D2->hasExternalLexicalStorage())
   return true;
 
-  // If one definition is currently being defined, we do not compare for
-  // equality and we assume that the decls are equal.
-  if (D1->isBeingDefined() || D2->isBeingDefined())
-return true;
-
   if (auto *D1CXX = dyn_cast(D1)) {
 if (auto *D2CXX = dyn_cast(D2)) {
   if (D1CXX->hasExternalLexicalStorage() &&

Modified: cfe/trunk/unittests/AST/ASTImporterTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/ASTImporterTest.cpp?rev=345784&r1=345783&r2=345784&view=diff
==
--- cfe/trunk/unittests/AST/ASTImporterTest.cpp (original)
+++ cfe/trunk/unittests/AST/ASTImporterTest.cpp Wed Oct 31 14:53:15 2018
@@ -3726,45 +3726,6 @@ TEST_P(ImportFunctionTemplateSpecializat
   EXPECT_EQ(To1->getPreviousDecl(), To0);
 }
 
-TEST_P(ASTImporterTestBase,
-ImportShouldNotReportFalseODRErrorWhenRecordIsBeingDefined) {
-  {
-Decl *FromTU = getTuDecl(
-R"(
-template 
-struct B;
-)",
-Lang_CXX, "input0.cc");
-auto *FromD = FirstDeclMatcher().match(
-FromTU, classTemplateDecl(hasName("B")));
-
-Import(FromD, Lang_CXX);
-  }
-
-  {
-Decl *FromTU = getTuDecl(
-R"(
-template 
-struct B {
-  void f();
-  B* b;
-};
-)",
-Lang_CXX, "input1.cc");
-FunctionDecl *FromD = FirstDeclMatcher().match(
-FromTU, functionDecl(hasName("f")));
-Import(FromD, Lang_CXX);
-auto *FromCTD = FirstDeclMatcher().match(
-FromTU, classTemplateDecl(hasName("B")));
-auto *ToCTD = cast(Import(FromCTD, Lang_CXX));
-EXPECT_TRUE(ToCTD->isThisDeclarationADefinition());
-
-// We expect no (ODR) warning during the import.
-auto *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
-EXPECT_EQ(0u, ToTU->getASTContext().getDiagnostics().getNumWarnings());
-  }
-}
-
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, DeclContextTest,
 ::testing::Values(ArgVector()), );
 


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

r355332 - [ASTImporter] Handle built-in when importing SourceLocation and FileID

[Shafik Yaghmour via cfe-commits]

Author: shafik
Date: Mon Mar  4 12:25:54 2019
New Revision: 355332

URL: http://llvm.org/viewvc/llvm-project?rev=355332&view=rev
Log:
[ASTImporter] Handle built-in when importing SourceLocation and FileID

Summary:
Currently when we see a built-in we try and import the include location. 
Instead what we do now is find the buffer like we do for the invalid case and 
copy that over to the to context.

Differential Revision: https://reviews.llvm.org/D58743

Modified:
cfe/trunk/include/clang/AST/ASTImporter.h
cfe/trunk/lib/AST/ASTImporter.cpp

Modified: cfe/trunk/include/clang/AST/ASTImporter.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTImporter.h?rev=355332&r1=355331&r2=355332&view=diff
==
--- cfe/trunk/include/clang/AST/ASTImporter.h (original)
+++ cfe/trunk/include/clang/AST/ASTImporter.h Mon Mar  4 12:25:54 2019
@@ -337,9 +337,9 @@ class TypeSourceInfo;
 ///
 /// \returns The equivalent file ID in the source manager of the "to"
 /// context, or the import error.
-llvm::Expected Import_New(FileID);
+llvm::Expected Import_New(FileID, bool IsBuiltin = false);
 // FIXME: Remove this version.
-FileID Import(FileID);
+FileID Import(FileID, bool IsBuiltin = false);
 
 /// Import the given C++ constructor initializer from the "from"
 /// context into the "to" context.

Modified: cfe/trunk/lib/AST/ASTImporter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=355332&r1=355331&r2=355332&view=diff
==
--- cfe/trunk/lib/AST/ASTImporter.cpp (original)
+++ cfe/trunk/lib/AST/ASTImporter.cpp Mon Mar  4 12:25:54 2019
@@ -8229,9 +8229,10 @@ SourceLocation ASTImporter::Import(Sourc
 return {};
 
   SourceManager &FromSM = FromContext.getSourceManager();
+  bool IsBuiltin = FromSM.isWrittenInBuiltinFile(FromLoc);
 
   std::pair Decomposed = FromSM.getDecomposedLoc(FromLoc);
-  FileID ToFileID = Import(Decomposed.first);
+  FileID ToFileID = Import(Decomposed.first, IsBuiltin);
   if (ToFileID.isInvalid())
 return {};
   SourceManager &ToSM = ToContext.getSourceManager();
@@ -8246,13 +8247,13 @@ SourceRange ASTImporter::Import(SourceRa
   return SourceRange(Import(FromRange.getBegin()), Import(FromRange.getEnd()));
 }
 
-Expected ASTImporter::Import_New(FileID FromID) {
-  FileID ToID = Import(FromID);
+Expected ASTImporter::Import_New(FileID FromID, bool IsBuiltin) {
+  FileID ToID = Import(FromID, IsBuiltin);
   if (ToID.isInvalid() && FromID.isValid())
 return llvm::make_error();
   return ToID;
 }
-FileID ASTImporter::Import(FileID FromID) {
+FileID ASTImporter::Import(FileID FromID, bool IsBuiltin) {
   llvm::DenseMap::iterator Pos = ImportedFileIDs.find(FromID);
   if (Pos != ImportedFileIDs.end())
 return Pos->second;
@@ -8278,25 +8279,29 @@ FileID ASTImporter::Import(FileID FromID
 }
 ToID = ToSM.getFileID(MLoc);
   } else {
-// Include location of this file.
-SourceLocation ToIncludeLoc = Import(FromSLoc.getFile().getIncludeLoc());
-
 const SrcMgr::ContentCache *Cache = FromSLoc.getFile().getContentCache();
-if (Cache->OrigEntry && Cache->OrigEntry->getDir()) {
-  // FIXME: We probably want to use getVirtualFile(), so we don't hit the
-  // disk again
-  // FIXME: We definitely want to re-use the existing MemoryBuffer, rather
-  // than mmap the files several times.
-  const FileEntry *Entry =
-  ToFileManager.getFile(Cache->OrigEntry->getName());
-  // FIXME: The filename may be a virtual name that does probably not
-  // point to a valid file and we get no Entry here. In this case try with
-  // the memory buffer below.
-  if (Entry)
-ToID = ToSM.createFileID(Entry, ToIncludeLoc,
- FromSLoc.getFile().getFileCharacteristic());
+
+if (!IsBuiltin) {
+  // Include location of this file.
+  SourceLocation ToIncludeLoc = Import(FromSLoc.getFile().getIncludeLoc());
+
+  if (Cache->OrigEntry && Cache->OrigEntry->getDir()) {
+// FIXME: We probably want to use getVirtualFile(), so we don't hit the
+// disk again
+// FIXME: We definitely want to re-use the existing MemoryBuffer, 
rather
+// than mmap the files several times.
+const FileEntry *Entry =
+ToFileManager.getFile(Cache->OrigEntry->getName());
+// FIXME: The filename may be a virtual name that does probably not
+// point to a valid file and we get no Entry here. In this case try 
with
+// the memory buffer below.
+if (Entry)
+  ToID = ToSM.createFileID(Entry, ToIncludeLoc,
+   FromSLoc.getFile().getFileCharacteristic());
+  }
 }
-if (ToID.isInvalid()) {
+
+if (ToID.isInvalid() || IsBuiltin) {
   // FIXME: We want to re-use the exist

r357100 - [ASTImporter] Fix IsStructuralMatch specialization for EnumDecl to prevent re-importing an EnumDecl while trying to complete it

[Shafik Yaghmour via cfe-commits]

Author: shafik
Date: Wed Mar 27 10:47:36 2019
New Revision: 357100

URL: http://llvm.org/viewvc/llvm-project?rev=357100&view=rev
Log:
[ASTImporter] Fix IsStructuralMatch specialization for EnumDecl to prevent 
re-importing an EnumDecl while trying to complete it

Summary:
We may try and re-import an EnumDecl while trying to complete it in 
IsStructuralMatch(...) specialization for EnumDecl. This change mirrors a 
similar fix for the specialization for RecordDecl.

Differential Revision: https://reviews.llvm.org/D59845

Modified:
cfe/trunk/lib/AST/ASTImporter.cpp

Modified: cfe/trunk/lib/AST/ASTImporter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=357100&r1=357099&r2=357100&view=diff
==
--- cfe/trunk/lib/AST/ASTImporter.cpp (original)
+++ cfe/trunk/lib/AST/ASTImporter.cpp Wed Mar 27 10:47:36 2019
@@ -1946,6 +1946,12 @@ bool ASTNodeImporter::IsStructuralMatch(
 }
 
 bool ASTNodeImporter::IsStructuralMatch(EnumDecl *FromEnum, EnumDecl *ToEnum) {
+  // Eliminate a potential failure point where we attempt to re-import
+  // something we're trying to import while completin ToEnum
+  if (Decl *ToOrigin = Importer.GetOriginalDecl(ToEnum))
+if (auto *ToOriginEnum = dyn_cast(ToOrigin))
+ToEnum = ToOriginEnum;
+
   StructuralEquivalenceContext Ctx(
   Importer.getFromContext(), Importer.getToContext(),
   Importer.getNonEquivalentDecls(), 
getStructuralEquivalenceKind(Importer));


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

r357940 - [ASTImporter] Call to HandleNameConflict in VisitEnumDecl mistakeningly using Name instead of SearchName

[Shafik Yaghmour via cfe-commits]

Author: shafik
Date: Mon Apr  8 13:50:21 2019
New Revision: 357940

URL: http://llvm.org/viewvc/llvm-project?rev=357940&view=rev
Log:
[ASTImporter] Call to HandleNameConflict in VisitEnumDecl mistakeningly using 
Name instead of SearchName

Summary:
https://reviews.llvm.org/D51633 added error handling to the 
ASTNodeImporter::VisitEnumDecl(...) for the conflicting names case. This could 
lead to erroneous return of an error in that case since we should have been 
using SearchName. Name may be empty in the case where we find the name via 
getTypedefNameForAnonDecl(...).

Differential Revision: https://reviews.llvm.org/D59665

Modified:
cfe/trunk/lib/AST/ASTImporter.cpp

Modified: cfe/trunk/lib/AST/ASTImporter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=357940&r1=357939&r2=357940&view=diff
==
--- cfe/trunk/lib/AST/ASTImporter.cpp (original)
+++ cfe/trunk/lib/AST/ASTImporter.cpp Mon Apr  8 13:50:21 2019
@@ -2441,7 +2441,7 @@ ExpectedDecl ASTNodeImporter::VisitEnumD
 }
 
 if (!ConflictingDecls.empty()) {
-  Name = Importer.HandleNameConflict(Name, DC, IDNS,
+  Name = Importer.HandleNameConflict(SearchName, DC, IDNS,
  ConflictingDecls.data(),
  ConflictingDecls.size());
   if (!Name)


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

r359338 - [ASTImporter] Copy Argument Passing Restrictions setting when importing a CXXRecordDecl definition

[Shafik Yaghmour via cfe-commits]

Author: shafik
Date: Fri Apr 26 11:51:28 2019
New Revision: 359338

URL: http://llvm.org/viewvc/llvm-project?rev=359338&view=rev
Log:
[ASTImporter] Copy Argument Passing Restrictions setting when importing a 
CXXRecordDecl definition

Summary:
For a CXXRecordDecl the RecordDeclBits are stored in the DeclContext. Currently 
when we import the definition of a CXXRecordDecl via the ASTImporter we do not 
copy over this data.
This change will add support for copying the ArgPassingRestrictions from 
RecordDeclBits to fix an LLDB expression parsing bug where we would set it to 
not pass in registers.
Note, we did not copy over any other of the RecordDeclBits since we don't have 
tests for those. We know that copying over LoadedFieldsFromExternalStorage 
would be a error and that may be the case for others as well.

The companion LLDB review: https://reviews.llvm.org/D61146

Differential Review: https://reviews.llvm.org/D61140

Added:
cfe/trunk/test/Import/cxx-record-flags/
cfe/trunk/test/Import/cxx-record-flags/Inputs/
cfe/trunk/test/Import/cxx-record-flags/Inputs/F.cpp
cfe/trunk/test/Import/cxx-record-flags/test.cpp
Modified:
cfe/trunk/lib/AST/ASTImporter.cpp

Modified: cfe/trunk/lib/AST/ASTImporter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=359338&r1=359337&r2=359338&view=diff
==
--- cfe/trunk/lib/AST/ASTImporter.cpp (original)
+++ cfe/trunk/lib/AST/ASTImporter.cpp Fri Apr 26 11:51:28 2019
@@ -1767,6 +1767,9 @@ Error ASTNodeImporter::ImportDefinition(
 ToData.HasDeclaredCopyAssignmentWithConstParam
   = FromData.HasDeclaredCopyAssignmentWithConstParam;
 
+// Copy over the data stored in RecordDeclBits
+ToCXX->setArgPassingRestrictions(FromCXX->getArgPassingRestrictions());
+
 SmallVector Bases;
 for (const auto &Base1 : FromCXX->bases()) {
   ExpectedType TyOrErr = import(Base1.getType());

Added: cfe/trunk/test/Import/cxx-record-flags/Inputs/F.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Import/cxx-record-flags/Inputs/F.cpp?rev=359338&view=auto
==
--- cfe/trunk/test/Import/cxx-record-flags/Inputs/F.cpp (added)
+++ cfe/trunk/test/Import/cxx-record-flags/Inputs/F.cpp Fri Apr 26 11:51:28 2019
@@ -0,0 +1,9 @@
+class FTrivial {
+  int i;
+};
+
+struct FNonTrivial {
+  virtual ~FNonTrivial() = default;
+  int i;
+};
+

Added: cfe/trunk/test/Import/cxx-record-flags/test.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Import/cxx-record-flags/test.cpp?rev=359338&view=auto
==
--- cfe/trunk/test/Import/cxx-record-flags/test.cpp (added)
+++ cfe/trunk/test/Import/cxx-record-flags/test.cpp Fri Apr 26 11:51:28 2019
@@ -0,0 +1,14 @@
+// RUN: clang-import-test -dump-ast -import %S/Inputs/F.cpp -expression %s | 
FileCheck %s
+
+// CHECK: FTrivial
+// CHECK: DefinitionData
+// CHECK-SAME: pass_in_registers
+
+// CHECK: FNonTrivial
+// CHECK-NOT: pass_in_registers
+// CHECK: DefaultConstructor
+
+void expr() {
+  FTrivial f1;
+  FNonTrivial f2;
+}


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

r368591 - [ASTDump] Add is_anonymous to VisitCXXRecordDecl

[Shafik Yaghmour via cfe-commits]

Author: shafik
Date: Mon Aug 12 10:07:49 2019
New Revision: 368591

URL: http://llvm.org/viewvc/llvm-project?rev=368591&view=rev
Log:
[ASTDump] Add is_anonymous to VisitCXXRecordDecl

Summary:
Adding is_anonymous the ASTDump for CXXRecordDecl. This turned out to be useful 
when debugging some problems with how LLDB creates ASTs from DWARF.

Differential Revision: https://reviews.llvm.org/D66028

Modified:
cfe/trunk/lib/AST/TextNodeDumper.cpp
cfe/trunk/test/AST/ast-dump-records.cpp

Modified: cfe/trunk/lib/AST/TextNodeDumper.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/TextNodeDumper.cpp?rev=368591&r1=368590&r2=368591&view=diff
==
--- cfe/trunk/lib/AST/TextNodeDumper.cpp (original)
+++ cfe/trunk/lib/AST/TextNodeDumper.cpp Mon Aug 12 10:07:49 2019
@@ -1537,6 +1537,7 @@ void TextNodeDumper::VisitCXXRecordDecl(
 FLAG(isGenericLambda, generic);
 FLAG(isLambda, lambda);
 
+FLAG(isAnonymousStructOrUnion, is_anonymous);
 FLAG(canPassInRegisters, pass_in_registers);
 FLAG(isEmpty, empty);
 FLAG(isAggregate, aggregate);

Modified: cfe/trunk/test/AST/ast-dump-records.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/AST/ast-dump-records.cpp?rev=368591&r1=368590&r2=368591&view=diff
==
--- cfe/trunk/test/AST/ast-dump-records.cpp (original)
+++ cfe/trunk/test/AST/ast-dump-records.cpp Mon Aug 12 10:07:49 2019
@@ -65,7 +65,7 @@ struct C {
 
   union {
 // CHECK-NEXT: CXXRecordDecl 0x{{[^ ]*}}  line:[[@LINE-1]]:3 union definition
-// CHECK-NEXT: DefinitionData pass_in_registers aggregate standard_layout 
trivially_copyable pod trivial literal
+// CHECK-NEXT: DefinitionData is_anonymous pass_in_registers aggregate 
standard_layout trivially_copyable pod trivial literal
 // CHECK-NEXT: DefaultConstructor exists trivial needs_implicit
 // CHECK-NEXT: CopyConstructor simple trivial has_const_param 
needs_implicit implicit_has_const_param
 // CHECK-NEXT: MoveConstructor exists simple trivial needs_implicit
@@ -87,7 +87,7 @@ struct C {
 
   struct {
 // CHECK-NEXT: CXXRecordDecl 0x{{[^ ]*}}  line:[[@LINE-1]]:3 struct definition
-// CHECK-NEXT: DefinitionData pass_in_registers aggregate standard_layout 
trivially_copyable pod trivial literal
+// CHECK-NEXT: DefinitionData is_anonymous pass_in_registers aggregate 
standard_layout trivially_copyable pod trivial literal
 // CHECK-NEXT: DefaultConstructor exists trivial needs_implicit
 // CHECK-NEXT: CopyConstructor simple trivial has_const_param 
needs_implicit implicit_has_const_param
 // CHECK-NEXT: MoveConstructor exists simple trivial needs_implicit
@@ -194,7 +194,7 @@ union G {
 
   union {
 // CHECK-NEXT: CXXRecordDecl 0x{{[^ ]*}}  line:[[@LINE-1]]:3 union definition
-// CHECK-NEXT: DefinitionData pass_in_registers aggregate standard_layout 
trivially_copyable pod trivial literal
+// CHECK-NEXT: DefinitionData is_anonymous pass_in_registers aggregate 
standard_layout trivially_copyable pod trivial literal
 // CHECK-NEXT: DefaultConstructor exists trivial needs_implicit
 // CHECK-NEXT: CopyConstructor simple trivial has_const_param 
needs_implicit implicit_has_const_param
 // CHECK-NEXT: MoveConstructor exists simple trivial needs_implicit
@@ -217,7 +217,7 @@ union G {
 
   struct {
 // CHECK-NEXT: CXXRecordDecl 0x{{[^ ]*}}  line:[[@LINE-1]]:3 struct definition
-// CHECK-NEXT: DefinitionData pass_in_registers aggregate standard_layout 
trivially_copyable pod trivial literal
+// CHECK-NEXT: DefinitionData is_anonymous pass_in_registers aggregate 
standard_layout trivially_copyable pod trivial literal
 // CHECK-NEXT: DefaultConstructor exists trivial needs_implicit
 // CHECK-NEXT: CopyConstructor simple trivial has_const_param 
needs_implicit implicit_has_const_param
 // CHECK-NEXT: MoveConstructor exists simple trivial needs_implicit


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

r361650 - [ASTImporter] Call to HandleNameConflict in VisitRecordDecl mistakeningly using Name instead of SearchName

[Shafik Yaghmour via cfe-commits]

Author: shafik
Date: Fri May 24 09:53:44 2019
New Revision: 361650

URL: http://llvm.org/viewvc/llvm-project?rev=361650&view=rev
Log:
[ASTImporter] Call to HandleNameConflict in VisitRecordDecl mistakeningly using 
Name instead of SearchName

Summary:
https://reviews.llvm.org/D51633 added error handling to the 
ASTNodeImporter::VisitRecordDecl for the conflicting names case. This could 
lead to erroneous return of an error in that case since we should have been 
using SearchName. Name may be empty in the case where we find the name via 
D->getTypedefNameForAnonDecl()->getDeclName().

This fix is very similar to https://reviews.llvm.org/D59665

Differential Revision: https://reviews.llvm.org/D62352

Modified:
cfe/trunk/lib/AST/ASTImporter.cpp

Modified: cfe/trunk/lib/AST/ASTImporter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=361650&r1=361649&r2=361650&view=diff
==
--- cfe/trunk/lib/AST/ASTImporter.cpp (original)
+++ cfe/trunk/lib/AST/ASTImporter.cpp Fri May 24 09:53:44 2019
@@ -2585,7 +2585,7 @@ ExpectedDecl ASTNodeImporter::VisitRecor
 } // for
 
 if (!ConflictingDecls.empty() && SearchName) {
-  Name = Importer.HandleNameConflict(Name, DC, IDNS,
+  Name = Importer.HandleNameConflict(SearchName, DC, IDNS,
  ConflictingDecls.data(),
  ConflictingDecls.size());
   if (!Name)


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

[clang] aa7ce60 - [Clang] Avoid crashes when parsing using enum declarations

[Shafik Yaghmour via cfe-commits]

Author: Shafik Yaghmour
Date: 2022-08-27T15:18:36-07:00
New Revision: aa7ce60536a642e825d26d89b4a4347a36b63360

URL: 
https://github.com/llvm/llvm-project/commit/aa7ce60536a642e825d26d89b4a4347a36b63360
DIFF: 
https://github.com/llvm/llvm-project/commit/aa7ce60536a642e825d26d89b4a4347a36b63360.diff

LOG: [Clang] Avoid crashes when parsing using enum declarations

In Parser::ParseUsingDeclaration(...) when we call ParseEnumSpecifier(...) it is
not calling SetTypeSpecError() on DS when it detects an error. That means that
DS is left set to TST_unspecified. When we then pass DS into
Sema::ActOnUsingEnumDeclaration(...) we hit an llvm_unreachable(...) since it
expects it to be one of three states TST_error, TST_enum or TST_typename.

This fixes https://github.com/llvm/llvm-project/issues/57347

Differential Revision: https://reviews.llvm.org/D132695

Added: 
clang/test/Parser/cxx20-using-enum.cpp

Modified: 
clang/lib/Parse/ParseDecl.cpp
clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct/p9-0x.cpp
clang/test/CXX/drs/dr3xx.cpp
clang/test/Parser/recovery.cpp
clang/test/Sema/enum.c
clang/test/SemaCXX/cxx1y-variable-templates_top_level.cpp
clang/test/SemaCXX/enum-scoped.cpp

Removed: 




diff  --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index 97a856a4a8f50..c502fc5e01019 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -4647,6 +4647,7 @@ void Parser::ParseEnumSpecifier(SourceLocation StartLoc, 
DeclSpec &DS,
 
 if (Spec.isSet() && Tok.isNot(tok::identifier)) {
   Diag(Tok, diag::err_expected) << tok::identifier;
+  DS.SetTypeSpecError();
   if (Tok.isNot(tok::l_brace)) {
 // Has no name and is not a definition.
 // Skip the rest of this declarator, up until the comma or semicolon.
@@ -4663,6 +4664,7 @@ void Parser::ParseEnumSpecifier(SourceLocation StartLoc, 
DeclSpec &DS,
   Tok.isNot(tok::colon)) {
 Diag(Tok, diag::err_expected_either) << tok::identifier << tok::l_brace;
 
+DS.SetTypeSpecError();
 // Skip the rest of this declarator, up until the comma or semicolon.
 SkipUntil(tok::comma, StopAtSemi);
 return;
@@ -4838,6 +4840,7 @@ void Parser::ParseEnumSpecifier(SourceLocation StartLoc, 
DeclSpec &DS,
   if (!Name && TUK != Sema::TUK_Definition) {
 Diag(Tok, diag::err_enumerator_unnamed_no_def);
 
+DS.SetTypeSpecError();
 // Skip the rest of this declarator, up until the comma or semicolon.
 SkipUntil(tok::comma, StopAtSemi);
 return;

diff  --git a/clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct/p9-0x.cpp 
b/clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct/p9-0x.cpp
index 719aeeddebcc0..93fad2ff90ec9 100644
--- a/clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct/p9-0x.cpp
+++ b/clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct/p9-0x.cpp
@@ -4,4 +4,4 @@
 // will necessarily be ill-formed as a trailing return type for a function
 // definition), and recover with a "type cannot be defined in a trailing return
 // type" error.
-auto j() -> enum { e3 }; // expected-error{{unnamed enumeration must be a 
definition}} expected-error {{expected a type}}
+auto j() -> enum { e3 }; // expected-error{{unnamed enumeration must be a 
definition}}

diff  --git a/clang/test/CXX/drs/dr3xx.cpp b/clang/test/CXX/drs/dr3xx.cpp
index 0278a7d7f4974..e6f7328dd4d6c 100644
--- a/clang/test/CXX/drs/dr3xx.cpp
+++ b/clang/test/CXX/drs/dr3xx.cpp
@@ -30,7 +30,7 @@ namespace dr301 { // dr301: yes
 typename T::template operator+ a; // expected-error {{typename 
specifier refers to a non-type template}} expected-error +{{}}
 // FIXME: This shouldn't say (null).
 class T::template operator+ b; // expected-error {{identifier 
followed by '<' indicates a class template specialization but (null) refers to 
a function template}}
-enum T::template operator+ c; // expected-error {{expected 
identifier}} expected-error {{does not declare anything}}
+enum T::template operator+ c; // expected-error {{expected 
identifier}}
 enum T::template operator+::E d; // expected-error {{qualified name 
refers into a specialization of function template 'T::template operator +'}} 
expected-error {{forward reference}}
 enum T::template X::E e;
 T::template operator+::foobar(); // expected-error {{qualified name 
refers into a specialization of function template 'T::template operator +'}}

diff  --git a/clang/test/Parser/cxx20-using-enum.cpp 
b/clang/test/Parser/cxx20-using-enum.cpp
new file mode 100644
index 0..d6744d3a91290
--- /dev/null
+++ b/clang/test/Parser/cxx20-using-enum.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -std=c++20 -verify %s
+
+namespace GH57347 {
+namespace A {}
+
+void f() {
+  using enum A::+; // expected-error {{expected identifier}}
+  using enum; // expected-error {{expected identifier or '{'}}
+  using enum class; // expected-error {{expected identifier or '{'}}
+  using enum : bla

[clang] b9f7678 - [Clang] Fix lambda CheckForDefaultedFunction(...) so that it checks the CXXMethodDecl is a special member function before attempting to call DefineDefaultedFunction(...)

[Shafik Yaghmour via cfe-commits]

Author: Shafik Yaghmour
Date: 2022-08-30T18:08:44-07:00
New Revision: b9f767884669db0b5a56d87b0e8733614d8f884d

URL: 
https://github.com/llvm/llvm-project/commit/b9f767884669db0b5a56d87b0e8733614d8f884d
DIFF: 
https://github.com/llvm/llvm-project/commit/b9f767884669db0b5a56d87b0e8733614d8f884d.diff

LOG: [Clang] Fix lambda CheckForDefaultedFunction(...) so that it checks the 
CXXMethodDecl is a special member function before attempting to call 
DefineDefaultedFunction(...)

In Sema::CheckCompletedCXXClass(...) It used a lambda CheckForDefaultedFunction
the CXXMethodDecl passed to CheckForDefaultedFunction may not be a special
member function and so before attempting to apply functions that only apply to
special member functions it needs to check. It fails to do this before calling
DefineDefaultedFunction(...). This PR adds that check and test to verify we no
longer crash.

This fixes https://github.com/llvm/llvm-project/issues/57431

Differential Revision: https://reviews.llvm.org/D132906

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/SemaCXX/constant-expression-cxx2a.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index f946018b361c..944d88d23017 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -86,6 +86,9 @@ Bug Fixes
   `Issue 57387 `_.
 - Fix a crash when emitting a concept-related diagnostic. This fixes
   `Issue 57415 `_.
+- Fix a crash when attempting to default a virtual constexpr non-special member
+  function in a derived class. This fixes
+  `Issue 57431 `_
 
 
 Improvements to Clang's diagnostics

diff  --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 198de680ae3d..04d06c11e9c0 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -6954,7 +6954,8 @@ void Sema::CheckCompletedCXXClass(Scope *S, CXXRecordDecl 
*Record) {
 // Define defaulted constexpr virtual functions that override a base class
 // function right away.
 // FIXME: We can defer doing this until the vtable is marked as used.
-if (M->isDefaulted() && M->isConstexpr() && M->size_overridden_methods())
+if (CSM != CXXInvalid && M->isDefaulted() && M->isConstexpr() &&
+M->size_overridden_methods())
   DefineDefaultedFunction(*this, M, M->getLocation());
 
 if (!Incomplete)

diff  --git a/clang/test/SemaCXX/constant-expression-cxx2a.cpp 
b/clang/test/SemaCXX/constant-expression-cxx2a.cpp
index 6ebec5dc7e30..63ea42995582 100644
--- a/clang/test/SemaCXX/constant-expression-cxx2a.cpp
+++ b/clang/test/SemaCXX/constant-expression-cxx2a.cpp
@@ -1473,3 +1473,13 @@ namespace PR45879 {
   }
   static_assert(g()); // expected-error {{constant expression}} expected-note 
{{in call}}
 }
+
+namespace GH57431 {
+class B {
+  virtual int constexpr f() = 0;
+};
+
+class D : B {
+  virtual int constexpr f() = default; // expected-error {{only special member 
functions and comparison operators may be defaulted}}
+};
+}



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

[clang] 9f6b319 - [Clang] Fix lambda CheckForDefaultedFunction(...) so that it checks the CXXMethodDecl is not deleted before attempting to call DefineDefaultedFunction(...)

[Shafik Yaghmour via cfe-commits]

Author: Shafik Yaghmour
Date: 2022-09-02T18:59:15-07:00
New Revision: 9f6b3199d33c146937f29feb62e123f34138f770

URL: 
https://github.com/llvm/llvm-project/commit/9f6b3199d33c146937f29feb62e123f34138f770
DIFF: 
https://github.com/llvm/llvm-project/commit/9f6b3199d33c146937f29feb62e123f34138f770.diff

LOG: [Clang] Fix lambda CheckForDefaultedFunction(...) so that it checks the 
CXXMethodDecl is not deleted before attempting to call 
DefineDefaultedFunction(...)

I discovered this additional bug at the end of working on D132906

In Sema::CheckCompletedCXXClass(...)  uses a lambda CheckForDefaultedFunction to
verify each CXXMethodDecl holds to the expected invariants before passing them
on to CheckForDefaultedFunction.

It is currently missing a check that it is not deleted, this adds that check and
a test that crashed without this check.

This fixes: https://github.com/llvm/llvm-project/issues/57516

Differential Revision: https://reviews.llvm.org/D133177

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/SemaCXX/constant-expression-cxx2a.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 41371fe0e04f2..146b2149f9d7e 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -89,6 +89,8 @@ Bug Fixes
 - Fix a crash when attempting to default a virtual constexpr non-special member
   function in a derived class. This fixes
   `Issue 57431 `_
+- Fix a crash where we attempt to define a deleted destructor. This fixes
+  `Issue 57516 `_
 
 
 Improvements to Clang's diagnostics

diff  --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 76db4e5a7a3b9..7c70255fd3e5d 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -6954,8 +6954,8 @@ void Sema::CheckCompletedCXXClass(Scope *S, CXXRecordDecl 
*Record) {
 // Define defaulted constexpr virtual functions that override a base class
 // function right away.
 // FIXME: We can defer doing this until the vtable is marked as used.
-if (CSM != CXXInvalid && M->isDefaulted() && M->isConstexpr() &&
-M->size_overridden_methods())
+if (CSM != CXXInvalid && !M->isDeleted() && M->isDefaulted() &&
+M->isConstexpr() && M->size_overridden_methods())
   DefineDefaultedFunction(*this, M, M->getLocation());
 
 if (!Incomplete)

diff  --git a/clang/test/SemaCXX/constant-expression-cxx2a.cpp 
b/clang/test/SemaCXX/constant-expression-cxx2a.cpp
index 63ea42995582e..34881d7446e58 100644
--- a/clang/test/SemaCXX/constant-expression-cxx2a.cpp
+++ b/clang/test/SemaCXX/constant-expression-cxx2a.cpp
@@ -1483,3 +1483,12 @@ class D : B {
   virtual int constexpr f() = default; // expected-error {{only special member 
functions and comparison operators may be defaulted}}
 };
 }
+
+namespace GH57516 {
+class B{
+  virtual constexpr ~B() = 0; // expected-note {{overridden virtual function 
is here}}
+};
+
+class D : B{}; // expected-error {{deleted function '~D' cannot override a 
non-deleted function}}
+// expected-note@-1 {{destructor of 'D' is implicitly deleted because base 
class 'B' has an inaccessible destructor}}
+}



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

[clang] [Clang] Remove unneeded template keyword (PR #71435)

[Shafik Yaghmour via cfe-commits]

https://github.com/shafik created 
https://github.com/llvm/llvm-project/pull/71435

As noted in this bug report: https://github.com/llvm/llvm-project/issues/37647

Due to this clang bug we added the template keyword in Redeclarable.h. The bug 
has been fixed since then, so removing it.

>From a7435adf0dc86d3bf656e623dbe7b42ea78344ae Mon Sep 17 00:00:00 2001
From: Shafik Yaghmour 
Date: Mon, 6 Nov 2023 11:29:43 -0800
Subject: [PATCH] [Clang] Remove unneeded template keyword

As noted in this bug report: https://github.com/llvm/llvm-project/issues/37647

Due to this clang bug we added the template keyword in Redeclarable.h. The bug
has been fixed since then, so removing it.
---
 clang/include/clang/AST/Redeclarable.h | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/clang/include/clang/AST/Redeclarable.h 
b/clang/include/clang/AST/Redeclarable.h
index 091bb886f2d4962..be022bf5322ad61 100644
--- a/clang/include/clang/AST/Redeclarable.h
+++ b/clang/include/clang/AST/Redeclarable.h
@@ -114,9 +114,7 @@ class Redeclarable {
 
 bool isFirst() const {
   return Link.is() ||
- // FIXME: 'template' is required on the next line due to an
- // apparent clang bug.
- Link.get().template is();
+ Link.get().is();
 }
 
 decl_type *getPrevious(const decl_type *D) const {

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

[clang] [Clang] Remove unneeded template keyword (PR #71435)

[Shafik Yaghmour via cfe-commits]

https://github.com/shafik closed https://github.com/llvm/llvm-project/pull/71435
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Remove unneeded template keyword (PR #71435)

[Shafik Yaghmour via cfe-commits]

shafik wrote:

Sadly this is failing on some build bots: 
https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/62315/consoleFull#-3939952849ba4694-19c4-4d7e-bec5-911270d8a58c

Not sure why at the moment. Will revert for now.

https://github.com/llvm/llvm-project/pull/71435
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Revert "[Clang] Remove unneeded template keyword" (PR #71478)

[Shafik Yaghmour via cfe-commits]

https://github.com/shafik created 
https://github.com/llvm/llvm-project/pull/71478

Reverts llvm/llvm-project#71435

This is failing on some build bots e.g.: 
https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/62315/consoleFull#-3939952849ba4694-19c4-4d7e-bec5-911270d8a58c

>From 61934f800bb6d1a987e214bd7424f4dc1c58d4c3 Mon Sep 17 00:00:00 2001
From: Shafik Yaghmour 
Date: Mon, 6 Nov 2023 18:22:07 -0800
Subject: [PATCH] Revert "[Clang] Remove unneeded template keyword (#71435)"

This reverts commit 9c346780dc1deed1c5362fcfca874e281c53e6e0.
---
 clang/include/clang/AST/Redeclarable.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/clang/include/clang/AST/Redeclarable.h 
b/clang/include/clang/AST/Redeclarable.h
index be022bf5322ad61..091bb886f2d4962 100644
--- a/clang/include/clang/AST/Redeclarable.h
+++ b/clang/include/clang/AST/Redeclarable.h
@@ -114,7 +114,9 @@ class Redeclarable {
 
 bool isFirst() const {
   return Link.is() ||
- Link.get().is();
+ // FIXME: 'template' is required on the next line due to an
+ // apparent clang bug.
+ Link.get().template is();
 }
 
 decl_type *getPrevious(const decl_type *D) const {

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

[clang] Revert "[Clang] Remove unneeded template keyword" (PR #71478)

[Shafik Yaghmour via cfe-commits]

https://github.com/shafik closed https://github.com/llvm/llvm-project/pull/71478
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Avoid -Wshadow warning when init-capture named same as class field (PR #74512)

[Shafik Yaghmour via cfe-commits]

@@ -179,3 +179,21 @@ void f() {
 #endif
 }
 }
+
+namespace GH71976 {
+struct A {
+  int b = 5;
+  int foo() {
+return [b = b]() { return b; }();
+  }
+};
+
+struct B {
+  int a;
+  void foo() {
+auto b = [a = this->a] {

shafik wrote:

```suggestion
auto b = [a = this->a] { // no diagnostic, init-capture does not shadow a
```

https://github.com/llvm/llvm-project/pull/74512
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Avoid -Wshadow warning when init-capture named same as class field (PR #74512)

[Shafik Yaghmour via cfe-commits]

@@ -179,3 +179,21 @@ void f() {
 #endif
 }
 }
+
+namespace GH71976 {
+struct A {
+  int b = 5;
+  int foo() {
+return [b = b]() { return b; }();

shafik wrote:

```suggestion
return [b = b]() { return b; }(); // no diagnostic, init-capture does not 
shadow b
```

https://github.com/llvm/llvm-project/pull/74512
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Avoid -Wshadow warning when init-capture named same as class field (PR #74512)

[Shafik Yaghmour via cfe-commits]

https://github.com/shafik edited https://github.com/llvm/llvm-project/pull/74512
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Avoid -Wshadow warning when init-capture named same as class field (PR #74512)

[Shafik Yaghmour via cfe-commits]

@@ -8395,10 +8395,11 @@ void Sema::CheckShadow(NamedDecl *D, NamedDecl 
*ShadowedDecl,
 
   unsigned WarningDiag = diag::warn_decl_shadow;
   SourceLocation CaptureLoc;
-  if (isa(D) && isa(ShadowedDecl) && NewDC &&
-  isa(NewDC)) {
+  if (isa(D) && NewDC && isa(NewDC)) {
 if (const auto *RD = dyn_cast(NewDC->getParent())) {
   if (RD->isLambda() && OldDC->Encloses(NewDC->getLexicalParent())) {
+if (!isa(ShadowedDecl))

shafik wrote:

Maybe a comment here along the lines of "if it is not VarDecl then it can not 
shadow" I am not sure if that implies it must be a capture but I think it does.

https://github.com/llvm/llvm-project/pull/74512
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Avoid -Wshadow warning when init-capture named same as class field (PR #74512)

[Shafik Yaghmour via cfe-commits]

https://github.com/shafik approved this pull request.

LGTM w/ minor comments

https://github.com/llvm/llvm-project/pull/74512
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Crash when referencing capture in static lambda (PR #74661)

[Shafik Yaghmour via cfe-commits]

https://github.com/shafik commented:

I think this makes sense but @cor3ntin should look at it.

https://github.com/llvm/llvm-project/pull/74661
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] bbb8a0d - [Clang] Fix ResolveConstructorOverload to not select a conversion function if we are going use copy elision

[Shafik Yaghmour via cfe-commits]

Author: Shafik Yaghmour
Date: 2023-12-08T09:38:59-08:00
New Revision: bbb8a0df7367068e1cf2fc54edd376beb976b430

URL: 
https://github.com/llvm/llvm-project/commit/bbb8a0df7367068e1cf2fc54edd376beb976b430
DIFF: 
https://github.com/llvm/llvm-project/commit/bbb8a0df7367068e1cf2fc54edd376beb976b430.diff

LOG: [Clang] Fix ResolveConstructorOverload to not select a conversion function 
if we are going use copy elision

ResolveConstructorOverload needs to check properly if we are going to use copy
elision we can't use a conversion function.

This fixes:

https://github.com/llvm/llvm-project/issues/39319
https://github.com/llvm/llvm-project/issues/60182
https://github.com/llvm/llvm-project/issues/62157
https://github.com/llvm/llvm-project/issues/64885
https://github.com/llvm/llvm-project/issues/65568

Differential Revision: https://reviews.llvm.org/D148474

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/Sema/SemaInit.cpp
clang/lib/Sema/SemaTemplateInstantiate.cpp
clang/test/SemaCXX/cxx1z-copy-omission.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 626585c146914c..28f9393e28437d 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -669,6 +669,12 @@ Bug Fixes in This Version
   Fixes (`#64467 `_)
 - Clang's ``-Wchar-subscripts`` no longer warns on chars whose values are 
known non-negative constants.
   Fixes (`#18763 `_)
+- Fix crash due to incorrectly allowing conversion functions in copy elision.
+  Fixes (`#39319 `_) and
+  (`#60182 `_) and
+  (`#62157 `_) and
+  (`#64885 `_) and
+  (`#65568 `_)
 
 Bug Fixes to Compiler Builtins
 ^^

diff  --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index 042ca61abe44d5..5ca6b232df66a5 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -4085,16 +4085,13 @@ static bool hasCopyOrMoveCtorParam(ASTContext &Ctx,
   return Ctx.hasSameUnqualifiedType(ParmT, ClassT);
 }
 
-static OverloadingResult
-ResolveConstructorOverload(Sema &S, SourceLocation DeclLoc,
-   MultiExprArg Args,
-   OverloadCandidateSet &CandidateSet,
-   QualType DestType,
-   DeclContext::lookup_result Ctors,
-   OverloadCandidateSet::iterator &Best,
-   bool CopyInitializing, bool AllowExplicit,
-   bool OnlyListConstructors, bool IsListInit,
-   bool SecondStepOfCopyInit = false) {
+static OverloadingResult ResolveConstructorOverload(
+Sema &S, SourceLocation DeclLoc, MultiExprArg Args,
+OverloadCandidateSet &CandidateSet, QualType DestType,
+DeclContext::lookup_result Ctors, OverloadCandidateSet::iterator &Best,
+bool CopyInitializing, bool AllowExplicit, bool OnlyListConstructors,
+bool IsListInit, bool RequireActualConstructor,
+bool SecondStepOfCopyInit = false) {
   CandidateSet.clear(OverloadCandidateSet::CSK_InitByConstructor);
   CandidateSet.setDestAS(DestType.getQualifiers().getAddressSpace());
 
@@ -4157,7 +4154,7 @@ ResolveConstructorOverload(Sema &S, SourceLocation 
DeclLoc,
   // Note: SecondStepOfCopyInit is only ever true in this case when
   // evaluating whether to produce a C++98 compatibility warning.
   if (S.getLangOpts().CPlusPlus17 && Args.size() == 1 &&
-  !SecondStepOfCopyInit) {
+  !RequireActualConstructor && !SecondStepOfCopyInit) {
 Expr *Initializer = Args[0];
 auto *SourceRD = Initializer->getType()->getAsCXXRecordDecl();
 if (SourceRD && S.isCompleteType(DeclLoc, Initializer->getType())) {
@@ -4225,6 +4222,12 @@ static void TryConstructorInitialization(Sema &S,
 return;
   }
 
+  bool RequireActualConstructor =
+  !(Entity.getKind() != InitializedEntity::EK_Base &&
+Entity.getKind() != InitializedEntity::EK_Delegating &&
+Entity.getKind() !=
+InitializedEntity::EK_LambdaToBlockConversionBlockElement);
+
   // C++17 [dcl.init]p17:
   // - If the initializer expression is a prvalue and the cv-unqualified
   //   version of the source type is the same class as the class of the
@@ -4234,11 +4237,7 @@ static void TryConstructorInitialization(Sema &S,
   // class or delegating to another constructor from a mem-initializer.
   // ObjC++: Lambda captured by the block in the lambda to block conversion
   // should avoid copy elision.
-  if (S.getLangOpts().CPlusPlus17 &&
-  Entity.getKind() != Initia

[clang] [Clang][Sema] Check the number of lambda non-concept tempate parameters (PR #74885)

[Shafik Yaghmour via cfe-commits]

https://github.com/shafik commented:

This needs at least one test and a Release note.

I expect the code form the issue to be well-formed: 
https://godbolt.org/z/bhdfG34xc

So I am curious why you are adding the diagnostic for?

https://github.com/llvm/llvm-project/pull/74885
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Sema] Print more static_assert exprs (PR #74852)

[Shafik Yaghmour via cfe-commits]

https://github.com/shafik commented:

I don't feel like the changes are an improvement to the diagnostic but I would 
like to hear from others who focus on clang front-end reviews.

https://github.com/llvm/llvm-project/pull/74852
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][ASTImporter] Support Importer of BuiltinBitCastExpr (PR #74813)

[Shafik Yaghmour via cfe-commits]

shafik wrote:

Can you please update the description to explain the problem in more details 
and why these changes fixes the problem. The description is what ends up in the 
git log and it is important that those logs provide enough information to 
understand the changes without having to view the details.

https://github.com/llvm/llvm-project/pull/74813
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][ASTImporter] Support Importer of BuiltinBitCastExpr (PR #74813)

[Shafik Yaghmour via cfe-commits]

@@ -7820,6 +7820,18 @@ ExpectedStmt 
ASTNodeImporter::VisitExplicitCastExpr(ExplicitCastExpr *E) {
 *ToLParenLocOrErr, OCE->getBridgeKind(), E->getCastKind(),
 *ToBridgeKeywordLocOrErr, ToTypeInfoAsWritten, ToSubExpr);
   }
+  case Stmt::BuiltinBitCastExprClass: {
+auto *BBC = cast(E);
+ExpectedSLoc ToLParenLocOrErr = import(BBC->getBeginLoc());

shafik wrote:

So it looks like this returns `KWLoc` so perhaps `ToKwLocOrErr` makes more 
sense?

https://github.com/llvm/llvm-project/pull/74813
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][Interp] Reject static lambdas with captures (PR #74718)

[Shafik Yaghmour via cfe-commits]

shafik wrote:

Can you please add more details to the description so folks reading the git log 
know what the change is without having to do more digging. 

https://github.com/llvm/llvm-project/pull/74718
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][AST] Fix crash in APValue::LValueBase::getType when we have invalid decl (PR #75130)

[Shafik Yaghmour via cfe-commits]

https://github.com/shafik created 
https://github.com/llvm/llvm-project/pull/75130

In some cases when calling APValue::LValueBase::getType() when we have a 
ValueDecl in some cases we don't handle invalid decls. We iterating over 
redeclarations we reset the current decl to the current most recent decl and we 
check the next redeclaration to ensure it is not invalid.

Fixes: https://github.com/llvm/llvm-project/issues/69468

>From 6e056bed30a15a0b7b657f10500ef69196f5366b Mon Sep 17 00:00:00 2001
From: Shafik Yaghmour 
Date: Mon, 11 Dec 2023 18:35:57 -0800
Subject: [PATCH] [Clang][AST] Fix crash in APValue::LValueBase::getType when
 we have invalid decl

In some cases when calling APValue::LValueBase::getType() when we have a
ValueDecl in some cases we don't handle invalid decls. We iterating over
redeclarations we reset the current decl to the current most recent decl and
we check the next redeclaration to ensure it is not invalid.

Fixes: https://github.com/llvm/llvm-project/issues/69468
---
 clang/lib/AST/APValue.cpp  |  4 +++-
 clang/test/AST/gh69468.cpp | 14 ++
 2 files changed, 17 insertions(+), 1 deletion(-)
 create mode 100644 clang/test/AST/gh69468.cpp

diff --git a/clang/lib/AST/APValue.cpp b/clang/lib/AST/APValue.cpp
index 4eae308ef5b34c..2ccd83a1d4823d 100644
--- a/clang/lib/AST/APValue.cpp
+++ b/clang/lib/AST/APValue.cpp
@@ -70,11 +70,13 @@ QualType APValue::LValueBase::getType() const {
 // constexpr int *p = &arr[1]; // valid?
 //
 // For now, we take the most complete type we can find.
-for (auto *Redecl = cast(D->getMostRecentDecl()); Redecl;
+for (auto *Redecl = cast(D->getMostRecentDecl());
+ Redecl && !Redecl->isInvalidDecl();
  Redecl = cast_or_null(Redecl->getPreviousDecl())) {
   QualType T = Redecl->getType();
   if (!T->isIncompleteArrayType())
 return T;
+  D = Redecl;
 }
 return D->getType();
   }
diff --git a/clang/test/AST/gh69468.cpp b/clang/test/AST/gh69468.cpp
new file mode 100644
index 00..8c93fa5e828ac7
--- /dev/null
+++ b/clang/test/AST/gh69468.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -verify %s
+
+
+a[i] = b[i]; // expected-error {{use of undeclared identifier 'i'}} \
+ // expected-error {{a type specifier is required for all 
declarations}} \
+// expected-error {{use of undeclared identifier 'b'}} \
+// expected-error {{use of undeclared identifier 'i'}}
+extern char b[];
+extern char a[];
+
+void foo(int j) {
+  // This used to crash here
+  a[j] = b[j];
+}

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

[clang] [Sema] Check nullness of captured type before use (PR #72230)

[Shafik Yaghmour via cfe-commits]

@@ -19773,6 +19773,8 @@ bool Sema::tryCaptureVariable(
   // declcontext can either capture the variable or have already captured
   // the variable.
   CaptureType = Var->getType();
+  if (CaptureType.isNull())

shafik wrote:

I am not sure this is the right fix. I think by the time we are here we should 
have caught any errors. I made a comment in the bug report describing my 
alternative.

https://github.com/llvm/llvm-project/pull/72230
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][AST] Invalidate DecompositionDecl if it has invalid initializer. (PR #72428)

[Shafik Yaghmour via cfe-commits]

@@ -13540,6 +13540,15 @@ void Sema::AddInitializerToDecl(Decl *RealDecl, Expr 
*Init, bool DirectInit) {
   CreateRecoveryExpr(Init->getBeginLoc(), Init->getEndLoc(), Args);
   if (RecoveryExpr.get())
 VDecl->setInit(RecoveryExpr.get());
+  // In general, for error recovery purposes, the initalizer doesn't play
+  // part in the valid bit of the declaration. There are a few exceptions:
+  //  1) if the var decl has a deduced auto type, and the type cannot be
+  // deduced by an invalid initializer;
+  //  2) if the var decl is decompsition decl with a concrete type (e.g.
+  //`int [a, b] = 1;`), and the initializer is invalid;
+  // Case 1) is already handled earlier in this function.
+  if (llvm::isa(VDecl)) // Case 2)
+VDecl->setInvalidDecl();

shafik wrote:

We should have the comment in both places. This is very helpful to 
understanding the code.

I agree that having the comment far away from the code that it is commenting on 
is less useful. The code could easily be moved in a refactor or in other ways 
and become completely disconnected. 

https://github.com/llvm/llvm-project/pull/72428
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Fix finding instantiated decls for class template specializations during instantiation (PR #72346)

[Shafik Yaghmour via cfe-commits]

https://github.com/shafik edited https://github.com/llvm/llvm-project/pull/72346
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Fix finding instantiated decls for class template specializations during instantiation (PR #72346)

[Shafik Yaghmour via cfe-commits]

https://github.com/shafik commented:

This makes some sense but I am not familiar enough with this area.

https://github.com/llvm/llvm-project/pull/72346
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Fix finding instantiated decls for class template specializations during instantiation (PR #72346)

[Shafik Yaghmour via cfe-commits]

@@ -0,0 +1,33 @@
+// RUN: %clang_cc1 -verify -fsyntax-only %s

shafik wrote:

Normally we put lone tests for a specific bug report in its own file e.g. 
GH70735.cpp for this case or we find a test that covers a similar area and put 
the test in a namespace named after the issue e.g. GH70735.

https://github.com/llvm/llvm-project/pull/72346
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Reapply Handle templated operators with reversed arguments (PR #72213)

[Shafik Yaghmour via cfe-commits]

shafik wrote:

So currently implementations differ here: https://godbolt.org/z/11331KW6e

This feels related to [cwg2804](https://wg21.link/cwg2804) which is not live 
yet but can be found [here](https://cplusplus.github.io/CWG/issues/2804.html). 
I can find the discussion on this from Kona but we ended up sending this to EWG 
b/c it was not clear what the right fixes for all the related issues are and we 
wanted design feedback.

CC @zygoloid @AaronBallman 

https://github.com/llvm/llvm-project/pull/72213
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Emit bad shift warnings (PR #70307)

[Shafik Yaghmour via cfe-commits]

@@ -11773,7 +11786,25 @@ QualType Sema::CheckShiftOperands(ExprResult &LHS, 
ExprResult &RHS,
   isScopedEnumerationType(RHSType)) {
 return InvalidOperands(Loc, LHS, RHS);
   }
-  DiagnoseBadShiftValues(*this, LHS, RHS, Loc, Opc, LHSType);
+
+  BadShiftValueKind BSVKind =
+  DiagnoseBadShiftValues(*this, LHS, RHS, Loc, Opc, LHSType);
+  if (ExprEvalContexts.back().isConstantEvaluated()) {

shafik wrote:

This feels pretty hacky but perhaps I am not fully understanding the solution.

Can you update the description with more details on why this is a good approach 
to the problem.

https://github.com/llvm/llvm-project/pull/70307
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [NFC][Clang] Fix potential deref of end iterator (PR #70193)

[Shafik Yaghmour via cfe-commits]

@@ -65,7 +65,7 @@ class CXXFieldCollector {
 
   /// getCurFields - Pointer to array of fields added to the currently parsed
   /// class.
-  FieldDecl **getCurFields() { return &*(Fields.end() - getCurNumFields()); }

shafik wrote:

I think this is basically [cwg232](https://wg21.link/cwg232) this is ok b/c we 
are not doing an lvalue-to-rvalue conversion but only using it to take the 
address of. 

CC @zygoloid 

https://github.com/llvm/llvm-project/pull/70193
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Fixed faulty shift count warning (PR #69521)

[Shafik Yaghmour via cfe-commits]

@@ -12143,8 +12143,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
&LHS, ExprResult &RHS,
 auto FXSema = S.Context.getFixedPointSemantics(LHSExprType);
 LeftSize = FXSema.getWidth() - (unsigned)FXSema.hasUnsignedPadding();
   }
-  llvm::APInt LeftBits(Right.getBitWidth(), LeftSize);
-  if (Right.uge(LeftBits)) {
+  if (Right.uge(LeftSize)) {

shafik wrote:

I feel like `LeftSizeInBits` would be a more appropriate name but the changes 
looks right.

https://github.com/llvm/llvm-project/pull/69521
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Defer the instantiation of explicit-specifier until constraint checking completes (PR #70548)

[Shafik Yaghmour via cfe-commits]

@@ -3553,6 +3553,49 @@ static unsigned getPackIndexForParam(Sema &S,
   llvm_unreachable("parameter index would not be produced from template");
 }
 
+// if `Specialization` is a `CXXConstructorDecl` or `CXXConversionDecl`
+// we try to instantiate and update its explicit specifier after constraint
+// checking.
+static Sema::TemplateDeductionResult
+resolveExplicitSpecifier(Sema &S, FunctionDecl *Specialization,
+ const MultiLevelTemplateArgumentList &SubstArgs,
+ TemplateDeductionInfo &Info,
+ FunctionTemplateDecl *FunctionTemplate,
+ ArrayRef DeducedArgs) {
+  auto GetExplicitSpecifier = [](FunctionDecl *D) {
+return isa(D)
+   ? cast(D)->getExplicitSpecifier()
+   : cast(D)->getExplicitSpecifier();
+  };
+  auto SetExplicitSpecifier = [](FunctionDecl *D, ExplicitSpecifier ES) {
+isa(D)
+? cast(D)->setExplicitSpecifier(ES)
+: cast(D)->setExplicitSpecifier(ES);
+  };
+
+  ExplicitSpecifier ES = GetExplicitSpecifier(Specialization);
+  Expr *const Expr = ES.getExpr();
+  if (!Expr) {
+return Sema::TDK_Success;
+  }
+  if (!Expr->isValueDependent()) {
+return Sema::TDK_Success;
+  }
+  // TemplateDeclInstantiator::InitFunctionInstantiation set the
+  // ActiveInstType to TemplateInstantiation, but we need
+  // to enable SFINAE when instantiating an explicit specifier.
+  Sema::InstantiatingTemplate Inst(

shafik wrote:

So we don't have to check `Inst.isInvalid()`?

https://github.com/llvm/llvm-project/pull/70548
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Defer the instantiation of explicit-specifier until constraint checking completes (PR #70548)

[Shafik Yaghmour via cfe-commits]

@@ -3682,6 +3725,17 @@ Sema::TemplateDeductionResult 
Sema::FinishTemplateArgumentDeduction(
 }
   }
 
+  // We skipped the instantiation of the explicit-specifier during subst the

shafik wrote:

This sentence is not super clear, can we spell out `subst` and `FD` as well.

https://github.com/llvm/llvm-project/pull/70548
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Fix clang++ crash on assertions when compiling source (PR #70594)

[Shafik Yaghmour via cfe-commits]

@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -fsyntax-only -std=c++11 %s -verify
+// RUN: %clang_cc1 -fsyntax-only -std=c++23 %s -verify
+
+// expected-no-diagnostics
+
+struct A {};
+using CA = const A;
+
+struct S1 : CA {

shafik wrote:

These examples: https://godbolt.org/z/8T3643496 don't crash clang and so don't 
really test the original bug. Please add the example from the bug report that 
does crash: https://godbolt.org/z/4naMG1q6r so we have a proper regression test.

https://github.com/llvm/llvm-project/pull/70594
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Fix clang++ crash on assertions when compiling source (PR #70594)

[Shafik Yaghmour via cfe-commits]

@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -fsyntax-only -std=c++11 %s -verify

shafik wrote:

Normally if we add a new test just for a specific bug report we name it 
something like `GH35603.cpp` so we can readily identify the test is linked to 
github issue 35603. If we add the test to an existing test file we wrap the 
test in namespace of the same name.

https://github.com/llvm/llvm-project/pull/70594
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Fix clang++ crash on assertions when compiling source (PR #70594)

[Shafik Yaghmour via cfe-commits]

@@ -6431,7 +6431,7 @@ static bool HandleConstructorCall(const Expr *E, const 
LValue &This,
   // Non-virtual base classes are initialized in the order in the class
   // definition. We have already checked for virtual base classes.
   assert(!BaseIt->isVirtual() && "virtual base for literal type");
-  assert(Info.Ctx.hasSameType(BaseIt->getType(), BaseType) &&
+  assert(Info.Ctx.hasSameUnqualifiedType(BaseIt->getType(), BaseType) &&

shafik wrote:

This looks right to me but CC @zygoloid who originally added this code.

https://github.com/llvm/llvm-project/pull/70594
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Defer the instantiation of explicit-specifier until constraint checking completes (PR #70548)

[Shafik Yaghmour via cfe-commits]

@@ -3553,6 +3553,48 @@ static unsigned getPackIndexForParam(Sema &S,
   llvm_unreachable("parameter index would not be produced from template");
 }
 
+// if `Specialization` is a `CXXConstructorDecl` or `CXXConversionDecl`,
+// we'll try to instantiate and update its explicit specifier after constraint
+// checking.
+static Sema::TemplateDeductionResult instantiateExplicitSpecifierDeferred(
+Sema &S, FunctionDecl *Specialization,
+const MultiLevelTemplateArgumentList &SubstArgs,
+TemplateDeductionInfo &Info, FunctionTemplateDecl *FunctionTemplate,
+ArrayRef DeducedArgs) {
+  auto GetExplicitSpecifier = [](FunctionDecl *D) {
+return isa(D)
+   ? cast(D)->getExplicitSpecifier()
+   : cast(D)->getExplicitSpecifier();
+  };
+  auto SetExplicitSpecifier = [](FunctionDecl *D, ExplicitSpecifier ES) {
+isa(D)
+? cast(D)->setExplicitSpecifier(ES)
+: cast(D)->setExplicitSpecifier(ES);
+  };
+
+  ExplicitSpecifier ES = GetExplicitSpecifier(Specialization);
+  Expr *ExplicitExpr = ES.getExpr();
+  if (!ExplicitExpr)
+return Sema::TDK_Success;
+  if (!ExplicitExpr->isValueDependent())
+return Sema::TDK_Success;
+
+  Sema::InstantiatingTemplate Inst(
+  S, Info.getLocation(), FunctionTemplate, DeducedArgs,
+  Sema::CodeSynthesisContext::DeducedTemplateArgumentSubstitution, Info);
+  if (Inst.isInvalid())
+return Sema::TDK_InstantiationDepth;
+  Sema::SFINAETrap Trap(S);
+  const ExplicitSpecifier InstantiatedES =
+  S.instantiateExplicitSpecifier(SubstArgs, ES);
+  if (InstantiatedES.isInvalid() || Trap.hasErrorOccurred()) {

shafik wrote:

Do we have a test that checks this case at all? Same for the other failures, if 
we know how to test them we should write a test to verify them.

https://github.com/llvm/llvm-project/pull/70548
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Defer the instantiation of explicit-specifier until constraint checking completes (PR #70548)

[Shafik Yaghmour via cfe-commits]

https://github.com/shafik edited https://github.com/llvm/llvm-project/pull/70548
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Defer the instantiation of explicit-specifier until constraint checking completes (PR #70548)

[Shafik Yaghmour via cfe-commits]

https://github.com/shafik approved this pull request.

I just want to make sure we have enough testing in place for this change but 
overall I think I am happy.

https://github.com/llvm/llvm-project/pull/70548
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [CodeGen] Revamp counted_by calculations (PR #70606)

[Shafik Yaghmour via cfe-commits]

@@ -966,9 +962,65 @@ static llvm::Value *getArrayIndexingBound(CodeGenFunction 
&CGF,
   return nullptr;
 }
 
-FieldDecl *CodeGenFunction::FindCountedByField(
-const Expr *Base,
-LangOptions::StrictFlexArraysLevelKind StrictFlexArraysLevel) {
+const Expr *
+CodeGenFunction::BuildCountedByFieldExpr(const Expr *Base,
+ const ValueDecl *CountedByVD) {
+  // Find the outer struct expr (i.e. p in p->a.b.c.d).
+  Expr *CountedByExpr = const_cast(Base)->IgnoreParenImpCasts();
+
+  // Work our way up the expression until we reach the DeclRefExpr.
+  while (!isa(CountedByExpr))
+if (const auto *ME = dyn_cast(CountedByExpr))
+  CountedByExpr = ME->getBase()->IgnoreParenImpCasts();
+
+  // Add back an implicit cast to create the required pr-value.
+  CountedByExpr = ImplicitCastExpr::Create(
+  getContext(), CountedByExpr->getType(), CK_LValueToRValue, CountedByExpr,
+  nullptr, VK_PRValue, FPOptionsOverride());
+
+  if (const auto *IFD = dyn_cast(CountedByVD)) {
+// The counted_by field is inside an anonymous struct / union. The
+// IndirectFieldDecl has the correct order of FieldDecls to build this
+// easily. (Yay!)
+for (NamedDecl *ND : IFD->chain()) {
+  auto *VD = cast(ND);
+  CountedByExpr =
+  MemberExpr::CreateImplicit(getContext(), CountedByExpr,
+ CountedByExpr->getType()->isPointerType(),
+ VD, VD->getType(), VK_LValue, 
OK_Ordinary);
+}
+  } else {
+CountedByExpr = MemberExpr::CreateImplicit(
+getContext(), const_cast(CountedByExpr),
+CountedByExpr->getType()->isPointerType(),
+const_cast(CountedByVD), CountedByVD->getType(), 
VK_LValue,
+OK_Ordinary);
+  }
+
+  return CountedByExpr;
+}
+
+const ValueDecl *
+CodeGenFunction::FindFlexibleArrayMemberField(ASTContext &Ctx,
+  const RecordDecl *RD) {
+  LangOptions::StrictFlexArraysLevelKind StrictFlexArraysLevel =
+  getLangOpts().getStrictFlexArraysLevel();
+
+  for (const Decl *D : RD->decls()) {
+if (const auto *VD = dyn_cast(D);
+VD && Decl::isFlexibleArrayMemberLike(Ctx, VD, VD->getType(),
+  StrictFlexArraysLevel, true))

shafik wrote:

```suggestion
  StrictFlexArraysLevel, 
/*IgnoreTemplateOrMacroSubstitution=*/true))
```

https://github.com/llvm/llvm-project/pull/70606
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [CodeGen] Revamp counted_by calculations (PR #70606)

[Shafik Yaghmour via cfe-commits]

@@ -966,9 +962,65 @@ static llvm::Value *getArrayIndexingBound(CodeGenFunction 
&CGF,
   return nullptr;
 }
 
-FieldDecl *CodeGenFunction::FindCountedByField(
-const Expr *Base,
-LangOptions::StrictFlexArraysLevelKind StrictFlexArraysLevel) {
+const Expr *
+CodeGenFunction::BuildCountedByFieldExpr(const Expr *Base,
+ const ValueDecl *CountedByVD) {
+  // Find the outer struct expr (i.e. p in p->a.b.c.d).
+  Expr *CountedByExpr = const_cast(Base)->IgnoreParenImpCasts();
+
+  // Work our way up the expression until we reach the DeclRefExpr.
+  while (!isa(CountedByExpr))
+if (const auto *ME = dyn_cast(CountedByExpr))
+  CountedByExpr = ME->getBase()->IgnoreParenImpCasts();
+
+  // Add back an implicit cast to create the required pr-value.
+  CountedByExpr = ImplicitCastExpr::Create(
+  getContext(), CountedByExpr->getType(), CK_LValueToRValue, CountedByExpr,
+  nullptr, VK_PRValue, FPOptionsOverride());
+
+  if (const auto *IFD = dyn_cast(CountedByVD)) {
+// The counted_by field is inside an anonymous struct / union. The
+// IndirectFieldDecl has the correct order of FieldDecls to build this
+// easily. (Yay!)
+for (NamedDecl *ND : IFD->chain()) {
+  auto *VD = cast(ND);
+  CountedByExpr =
+  MemberExpr::CreateImplicit(getContext(), CountedByExpr,
+ CountedByExpr->getType()->isPointerType(),
+ VD, VD->getType(), VK_LValue, 
OK_Ordinary);
+}
+  } else {
+CountedByExpr = MemberExpr::CreateImplicit(
+getContext(), const_cast(CountedByExpr),
+CountedByExpr->getType()->isPointerType(),
+const_cast(CountedByVD), CountedByVD->getType(), 
VK_LValue,
+OK_Ordinary);
+  }
+
+  return CountedByExpr;
+}
+
+const ValueDecl *
+CodeGenFunction::FindFlexibleArrayMemberField(ASTContext &Ctx,
+  const RecordDecl *RD) {
+  LangOptions::StrictFlexArraysLevelKind StrictFlexArraysLevel =
+  getLangOpts().getStrictFlexArraysLevel();
+
+  for (const Decl *D : RD->decls()) {
+if (const auto *VD = dyn_cast(D);
+VD && Decl::isFlexibleArrayMemberLike(Ctx, VD, VD->getType(),
+  StrictFlexArraysLevel, true))

shafik wrote:

nit to be consistent with 
[bugprone-argument-comment](https://clang.llvm.org/extra/clang-tidy/checks/bugprone/argument-comment.html)

https://github.com/llvm/llvm-project/pull/70606
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Fix to attribute plugins reaching an unreachable (PR #70877)

[Shafik Yaghmour via cfe-commits]

shafik wrote:

Can you please add a more detailed description of the problem and what the fix 
actually is. The description is what ends up in the git log and it is important 
that we have enough details there to understand the PR and what changes it 
makes.

I do not see a test, can this fix be tested?

https://github.com/llvm/llvm-project/pull/70877
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][AST] Fix crash in APValue::LValueBase::getType when we have invalid decl (PR #75130)

[Shafik Yaghmour via cfe-commits]

https://github.com/shafik updated 
https://github.com/llvm/llvm-project/pull/75130

>From 8a169838778e333e6bf14a7156a6220d09b5818a Mon Sep 17 00:00:00 2001
From: Shafik Yaghmour 
Date: Mon, 11 Dec 2023 18:35:57 -0800
Subject: [PATCH] [Clang][AST] Fix crash in APValue::LValueBase::getType when
 we have invalid decl

In some cases when calling APValue::LValueBase::getType() when we have a
ValueDecl in some cases we don't handle invalid decls. We iterating over
redeclarations we reset the current decl to the current most recent decl and
we check the next redeclaration to ensure it is not invalid.

Fixes: https://github.com/llvm/llvm-project/issues/69468
---
 clang/docs/ReleaseNotes.rst |  4 
 clang/lib/AST/APValue.cpp   |  4 +++-
 clang/test/AST/gh69468.cpp  | 14 ++
 3 files changed, 21 insertions(+), 1 deletion(-)
 create mode 100644 clang/test/AST/gh69468.cpp

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 783dc7333af7e2..988eec5ed6f995 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -641,6 +641,10 @@ Bug Fixes in This Version
   Fixes (`#67317 `_)
 - Clang now properly diagnoses use of stand-alone OpenMP directives after a
   label (including ``case`` or ``default`` labels).
+- Fix crash when dealing with ill-formed code where we were not handling 
invalid
+  redeclarations properly.
+  Fixes (`#69468 `_)
+  
 
   Before:
 
diff --git a/clang/lib/AST/APValue.cpp b/clang/lib/AST/APValue.cpp
index 4eae308ef5b34c..2ccd83a1d4823d 100644
--- a/clang/lib/AST/APValue.cpp
+++ b/clang/lib/AST/APValue.cpp
@@ -70,11 +70,13 @@ QualType APValue::LValueBase::getType() const {
 // constexpr int *p = &arr[1]; // valid?
 //
 // For now, we take the most complete type we can find.
-for (auto *Redecl = cast(D->getMostRecentDecl()); Redecl;
+for (auto *Redecl = cast(D->getMostRecentDecl());
+ Redecl && !Redecl->isInvalidDecl();
  Redecl = cast_or_null(Redecl->getPreviousDecl())) {
   QualType T = Redecl->getType();
   if (!T->isIncompleteArrayType())
 return T;
+  D = Redecl;
 }
 return D->getType();
   }
diff --git a/clang/test/AST/gh69468.cpp b/clang/test/AST/gh69468.cpp
new file mode 100644
index 00..8c93fa5e828ac7
--- /dev/null
+++ b/clang/test/AST/gh69468.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -verify %s
+
+
+a[i] = b[i]; // expected-error {{use of undeclared identifier 'i'}} \
+ // expected-error {{a type specifier is required for all 
declarations}} \
+// expected-error {{use of undeclared identifier 'b'}} \
+// expected-error {{use of undeclared identifier 'i'}}
+extern char b[];
+extern char a[];
+
+void foo(int j) {
+  // This used to crash here
+  a[j] = b[j];
+}

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

[clang] [Clang][AST] Fix crash in APValue::LValueBase::getType when we have invalid decl (PR #75130)

[Shafik Yaghmour via cfe-commits]

https://github.com/shafik updated 
https://github.com/llvm/llvm-project/pull/75130

>From fb0ea87f212c78b5b39345e534a48b8956a19cc4 Mon Sep 17 00:00:00 2001
From: Shafik Yaghmour 
Date: Mon, 11 Dec 2023 18:35:57 -0800
Subject: [PATCH] [Clang][AST] Fix crash in APValue::LValueBase::getType when
 we have invalid decl

In some cases when calling APValue::LValueBase::getType() when we have a
ValueDecl in some cases we don't handle invalid decls. We iterating over
redeclarations we reset the current decl to the current most recent decl and
we check the next redeclaration to ensure it is not invalid.

Fixes: https://github.com/llvm/llvm-project/issues/69468
---
 clang/docs/ReleaseNotes.rst |  4 
 clang/lib/AST/APValue.cpp   |  4 +++-
 clang/test/AST/gh69468.cpp  | 14 ++
 3 files changed, 21 insertions(+), 1 deletion(-)
 create mode 100644 clang/test/AST/gh69468.cpp

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 783dc7333af7e2..b9415b2275d8ef 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -641,6 +641,10 @@ Bug Fixes in This Version
   Fixes (`#67317 `_)
 - Clang now properly diagnoses use of stand-alone OpenMP directives after a
   label (including ``case`` or ``default`` labels).
+- Fix crash when dealing with ill-formed code where we were not handling 
invalid
+  redeclarations properly.
+  Fixes (`#69468 `_)
+
 
   Before:
 
diff --git a/clang/lib/AST/APValue.cpp b/clang/lib/AST/APValue.cpp
index 4eae308ef5b34c..2ccd83a1d4823d 100644
--- a/clang/lib/AST/APValue.cpp
+++ b/clang/lib/AST/APValue.cpp
@@ -70,11 +70,13 @@ QualType APValue::LValueBase::getType() const {
 // constexpr int *p = &arr[1]; // valid?
 //
 // For now, we take the most complete type we can find.
-for (auto *Redecl = cast(D->getMostRecentDecl()); Redecl;
+for (auto *Redecl = cast(D->getMostRecentDecl());
+ Redecl && !Redecl->isInvalidDecl();
  Redecl = cast_or_null(Redecl->getPreviousDecl())) {
   QualType T = Redecl->getType();
   if (!T->isIncompleteArrayType())
 return T;
+  D = Redecl;
 }
 return D->getType();
   }
diff --git a/clang/test/AST/gh69468.cpp b/clang/test/AST/gh69468.cpp
new file mode 100644
index 00..8c93fa5e828ac7
--- /dev/null
+++ b/clang/test/AST/gh69468.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -verify %s
+
+
+a[i] = b[i]; // expected-error {{use of undeclared identifier 'i'}} \
+ // expected-error {{a type specifier is required for all 
declarations}} \
+// expected-error {{use of undeclared identifier 'b'}} \
+// expected-error {{use of undeclared identifier 'i'}}
+extern char b[];
+extern char a[];
+
+void foo(int j) {
+  // This used to crash here
+  a[j] = b[j];
+}

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

[clang] [clang] Fix CTAD not work for C++ explicit type conversion (functional annotation). (PR #75779)

[Shafik Yaghmour via cfe-commits]

shafik wrote:

Can you please update your summary to explain the problem and how the fix 
addresses the problem.

This is usually what goes into the git log and we want those to be as 
descriptive as possible but also it help code reviewers to understand what they 
are reviewing quicker.

https://github.com/llvm/llvm-project/pull/75779
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Fix CTAD not work for C++ explicit type conversion (functional annotation). (PR #75779)

[Shafik Yaghmour via cfe-commits]

shafik wrote:

> > Why remove the support for ParenListExpr entirely? Could you not fallback 
> > on looking at the init list expression only when ParenListExpr is null?
> 
> We could keep `ParenListExpr`, but I'm not sure the benefit (IMO, removing it 
> seems like an API improvement, and make code simpler). The `ParenListExpr` is 
> redundant with the existing `Inits` parameter to some degree, the only 
> benefit to use it is for the precise `()` location information (as we deduct 
> the template type here, I think it is fine to use a less-precise source 
> location).
> 
> Looking at all usages (total 4) of 
> `DeduceTemplateSpecializationFromInitializer` in clang, only 1 
> [place](https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaDecl.cpp#L12959)
>  passes the `ParenListExpr`.

As long as it does not effect the quality of the diagnostics then this makes 
sense.

https://github.com/llvm/llvm-project/pull/75779
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Fix CTAD not work for C++ explicit type conversion (functional annotation). (PR #75779)

[Shafik Yaghmour via cfe-commits]

https://github.com/shafik approved this pull request.

LGTM

https://github.com/llvm/llvm-project/pull/75779
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Fix CTAD not respect default template arguments that were added after the definition. (PR #75569)

[Shafik Yaghmour via cfe-commits]

shafik wrote:

Can we please add more detailed summaries, we want the git log to be sufficient 
for triaging various issues without having to dig into each commit individually.

https://github.com/llvm/llvm-project/pull/75569
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Fix CTAD not respect default template arguments that were added after the definition. (PR #75569)

[Shafik Yaghmour via cfe-commits]

@@ -2061,13 +2070,13 @@ DeclResult Sema::CheckClassTemplate(
   if (!(TUK == TUK_Friend && CurContext->isDependentContext()) &&
   CheckTemplateParameterList(
   TemplateParams,
-  PrevClassTemplate
-  ? PrevClassTemplate->getMostRecentDecl()->getTemplateParameters()
-  : nullptr,
+  PrevClassTemplate ? GetTemplateParameterList(PrevClassTemplate)
+: nullptr,
   (SS.isSet() && SemanticContext && SemanticContext->isRecord() &&
SemanticContext->isDependentContext())
   ? TPC_ClassTemplateMember
-  : TUK == TUK_Friend ? TPC_FriendClassTemplate : 
TPC_ClassTemplate,
+  : TUK == TUK_Friend ? TPC_FriendClassTemplate

shafik wrote:

This looks like an unrelated change and it looks funny.

https://github.com/llvm/llvm-project/pull/75569
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][Interp] Implement integral->complex casts (PR #75590)

[Shafik Yaghmour via cfe-commits]

shafik wrote:

Can we please have a more detailed summary for this PR? We rely on more 
detailed summaries in git logs.

https://github.com/llvm/llvm-project/pull/75590
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][Interp] Implement integral->complex casts (PR #75590)

[Shafik Yaghmour via cfe-commits]

@@ -283,6 +283,28 @@ bool ByteCodeExprGen::VisitCastExpr(const 
CastExpr *CE) {
   case CK_ToVoid:
 return discard(SubExpr);
 
+  case CK_IntegralRealToComplex:
+  case CK_FloatingRealToComplex: {
+// We're creating a complex value here, so we need to
+// allocate storage for it.
+if (!Initializing) {
+  std::optional LocalIndex =
+  allocateLocal(CE, /*IsExtended=*/true);
+  if (!LocalIndex)
+return false;
+  if (!this->emitGetPtrLocal(*LocalIndex, CE))
+return false;
+}
+
+if (!this->visitArrayElemInit(0, SubExpr))

shafik wrote:

Why the different init for first and second element? Complex is basiclly an 
array underneath: https://eel.is/c++draft/complex.numbers#general-4 but I am 
not sure I understand the difference in how we init the first and second 
element. 

Seems like a comment would be helpful.

https://github.com/llvm/llvm-project/pull/75590
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][AST] Invalidate DecompositionDecl if it has invalid initializer. (PR #72428)

[Shafik Yaghmour via cfe-commits]

https://github.com/shafik commented:

We need a release note and please add a more detailed summary. A description of 
the problem being solved and the solution to the fix provides.

https://github.com/llvm/llvm-project/pull/72428
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] fix issue 73559. (PR #74926)

[Shafik Yaghmour via cfe-commits]

@@ -641,6 +641,8 @@ Bug Fixes in This Version
   Fixes (`#67317 `_)
 - Clang now properly diagnoses use of stand-alone OpenMP directives after a
   label (including ``case`` or ``default`` labels).
+- Fix crash when using C++ only tokens like ``::`` in C compiler clang.

shafik wrote:

It looks like you added your note in the middle of another note that has a 
before and after description of the bug they fixed.

Please make sure you move this to the end of the list. 

After that I think this is good.

https://github.com/llvm/llvm-project/pull/74926
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] fix issue 73559. (PR #74926)

[Shafik Yaghmour via cfe-commits]

shafik wrote:

In the future please add a more description title to your PR. Your summary is 
great though.

Also in this case this is a clang and parser fix so you should prefix your 
title with `[Clang][Parser]`

https://github.com/llvm/llvm-project/pull/74926
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][Sema] Add -Wswitch-default warning option (PR #73077)

[Shafik Yaghmour via cfe-commits]

@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wswitch-default %s
+
+int f1(int a) {
+  switch (a) {// expected-warning {{'switch' missing 'default' 
label}}
+case 1: a++; break;
+case 2: a += 2; break;
+  }
+  return a;
+}

shafik wrote:

Might be worth adding a test case that shows that this warns even for 
completely covered switches e.g. https://godbolt.org/z/zbqr6P5xc

https://github.com/llvm/llvm-project/pull/73077
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Sema] Fix Wswitch-default bad warning in template (PR #76007)

[Shafik Yaghmour via cfe-commits]

shafik wrote:

Can you add more details to you summary 
"https://github.com/llvm/llvm-project/pull/73077 added -Wswitch-default 
diagnostic but it produced false positives in templates. This PR will address 
that issue" 


https://github.com/llvm/llvm-project/pull/76007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Sema] Fix Wswitch-default bad warning in template (PR #76007)

[Shafik Yaghmour via cfe-commits]

@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wswitch-default %s

shafik wrote:

I think we can add these test to `clang/test/Sema/switch-default.c`

https://github.com/llvm/llvm-project/pull/76007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][sema] make sure arguments of __atomic_exchange complete type (PR #75135)

[Shafik Yaghmour via cfe-commits]

https://github.com/shafik commented:

We should add a release note for this change.

https://github.com/llvm/llvm-project/pull/75135
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Lex] Fix parsing of nested requirement to prevent flowing off the end of token stream (PR #73691)

[Shafik Yaghmour via cfe-commits]

https://github.com/shafik created 
https://github.com/llvm/llvm-project/pull/73691

Currently when parsing a nested requirement we attempt to balance parens if we 
have a parameter list. This will fail in some cases of ill-formed code and keep 
going until we fall off the token stream and crash. This fixes the hand parsing 
by using SkipUntil which will properly flag if we don't find the expected 
tokens.

Fixes: https://github.com/llvm/llvm-project/issues/73112

>From 3f8b552ccc78c0d5ca98ed529ce28ac8c95f0656 Mon Sep 17 00:00:00 2001
From: Shafik Yaghmour 
Date: Tue, 28 Nov 2023 11:20:12 -0800
Subject: [PATCH] [Clang][Lex] Fix parsing of nested requirement to prevent
 flowing off the end of token stream

Currently when parsing a nested requirement we attempt to balance parens if we
have a parameter list. This will fail in some cases of ill-formed code and keep
going until we fall off the token stream and crash. This fixes the hand parsing
by using SkipUntil which will properly flag if we don't find the expected
tokens.

Fixes: https://github.com/llvm/llvm-project/issues/73112
---
 clang/lib/Parse/ParseExprCXX.cpp   | 4 
 clang/test/Parser/cxx2a-concepts-requires-expr.cpp | 8 
 2 files changed, 12 insertions(+)

diff --git a/clang/lib/Parse/ParseExprCXX.cpp b/clang/lib/Parse/ParseExprCXX.cpp
index 79db094e098f8e6..a0903eb3a932490 100644
--- a/clang/lib/Parse/ParseExprCXX.cpp
+++ b/clang/lib/Parse/ParseExprCXX.cpp
@@ -3639,6 +3639,10 @@ ExprResult Parser::ParseRequiresExpression() {
 //  TryParseParameterDeclarationClause).
 unsigned Depth = 1;
 while (Depth != 0) {
+  bool FoundParen = SkipUntil(tok::l_paren, tok::r_paren,
+  SkipUntilFlags::StopBeforeMatch);
+  if (!FoundParen)
+break;
   if (Tok.is(tok::l_paren))
 Depth++;
   else if (Tok.is(tok::r_paren))
diff --git a/clang/test/Parser/cxx2a-concepts-requires-expr.cpp 
b/clang/test/Parser/cxx2a-concepts-requires-expr.cpp
index a18a54c7fad0690..971591afb08dba2 100644
--- a/clang/test/Parser/cxx2a-concepts-requires-expr.cpp
+++ b/clang/test/Parser/cxx2a-concepts-requires-expr.cpp
@@ -160,3 +160,11 @@ template 
 requires requires {
  typename BitInt; // ok
 } using r44 = void;
+
+namespace GH73112 {
+void f() {
+requires { requires(int; } // expected-error {{expected ')'}} \
+   // expected-error {{expected expression}} \
+   // expected-note {{to match this '('}}
+}
+}

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

[clang] [Clang][Lex] Fix parsing of nested requirement to prevent flowing off the end of token stream (PR #73691)

[Shafik Yaghmour via cfe-commits]

@@ -3639,6 +3639,10 @@ ExprResult Parser::ParseRequiresExpression() {
 //  TryParseParameterDeclarationClause).

shafik wrote:

I don't think the above fixme above is needed, I don't think there is any other 
way to handle this, at least based on some of the other places 
`TryParseParameterDeclarationClause()` is used e.g. 
`TryParseFunctionDeclarator(...)`

https://github.com/llvm/llvm-project/pull/73691
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Fix a bug with qualified name lookup into current instantiation (PR #73018)

[Shafik Yaghmour via cfe-commits]

@@ -482,8 +481,23 @@ ParsedType Sema::getTypeName(const IdentifierInfo &II, 
SourceLocation NameLoc,
 }
   }
 }
-// If typo correction failed or was not performed, fall through
-[[fallthrough]];
+Result.suppressDiagnostics();
+return nullptr;
+  case LookupResult::NotFoundInCurrentInstantiation: {
+if (AllowImplicitTypename == ImplicitTypenameContext::Yes) {
+  QualType T;
+  T = Context.getDependentNameType(ElaboratedTypeKeyword::None,
+   SS->getScopeRep(), &II);
+  TypeLocBuilder TLB;
+  DependentNameTypeLoc TL = TLB.push(T);
+  TL.setElaboratedKeywordLoc(SourceLocation());
+  TL.setQualifierLoc(SS->getWithLocInContext(Context));
+  TL.setNameLoc(NameLoc);
+  return CreateParsedType(T, TLB.getTypeSourceInfo(Context, T));
+}
+Result.suppressDiagnostics();

shafik wrote:

We could also just do `[[fallthrough]];` at the end. I think that is fine.

https://github.com/llvm/llvm-project/pull/73018
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Fix a bug with qualified name lookup into current instantiation (PR #73018)

[Shafik Yaghmour via cfe-commits]

@@ -482,8 +481,23 @@ ParsedType Sema::getTypeName(const IdentifierInfo &II, 
SourceLocation NameLoc,
 }
   }
 }
-// If typo correction failed or was not performed, fall through
-[[fallthrough]];
+Result.suppressDiagnostics();
+return nullptr;
+  case LookupResult::NotFoundInCurrentInstantiation: {
+if (AllowImplicitTypename == ImplicitTypenameContext::Yes) {
+  QualType T;
+  T = Context.getDependentNameType(ElaboratedTypeKeyword::None,

shafik wrote:

Why not just initialize `T` when we declare it?

https://github.com/llvm/llvm-project/pull/73018
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Lex] Fix parsing of nested requirement to prevent flowing off the end of token stream (PR #73691)

[Shafik Yaghmour via cfe-commits]

https://github.com/shafik updated 
https://github.com/llvm/llvm-project/pull/73691

>From 21d6bbdd1f8676e51b053ec3dd9020270e6b929e Mon Sep 17 00:00:00 2001
From: Shafik Yaghmour 
Date: Tue, 28 Nov 2023 11:20:12 -0800
Subject: [PATCH] [Clang][Lex] Fix parsing of nested requirement to prevent
 flowing off the end of token stream

Currently when parsing a nested requirement we attempt to balance parens if we
have a parameter list. This will fail in some cases of ill-formed code and keep
going until we fall off the token stream and crash. This fixes the hand parsing
by using SkipUntil which will properly flag if we don't find the expected
tokens.

Fixes: https://github.com/llvm/llvm-project/issues/73112
---
 clang/docs/ReleaseNotes.rst| 3 +++
 clang/lib/Parse/ParseExprCXX.cpp   | 6 --
 clang/test/Parser/cxx2a-concepts-requires-expr.cpp | 8 
 3 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 7c909ac3cab6419..f8abc44743da09c 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -783,6 +783,9 @@ Bug Fixes to C++ Support
   completes (except deduction guides). Fixes:
   (`#59827 `_)
 
+- Fix crash when parsing nested requirement. Fixes:
+  (`#73112 `_)
+
 Bug Fixes to AST Handling
 ^
 - Fixed an import failure of recursive friend class template.
diff --git a/clang/lib/Parse/ParseExprCXX.cpp b/clang/lib/Parse/ParseExprCXX.cpp
index 79db094e098f8e6..8b86db1bb8fc5d5 100644
--- a/clang/lib/Parse/ParseExprCXX.cpp
+++ b/clang/lib/Parse/ParseExprCXX.cpp
@@ -3635,10 +3635,12 @@ ExprResult Parser::ParseRequiresExpression() {
   auto Res = TryParseParameterDeclarationClause();
   if (Res != TPResult::False) {
 // Skip to the closing parenthesis
-// FIXME: Don't traverse these tokens twice (here and in
-//  TryParseParameterDeclarationClause).
 unsigned Depth = 1;
 while (Depth != 0) {
+  bool FoundParen = SkipUntil(tok::l_paren, tok::r_paren,
+  SkipUntilFlags::StopBeforeMatch);
+  if (!FoundParen)
+break;
   if (Tok.is(tok::l_paren))
 Depth++;
   else if (Tok.is(tok::r_paren))
diff --git a/clang/test/Parser/cxx2a-concepts-requires-expr.cpp 
b/clang/test/Parser/cxx2a-concepts-requires-expr.cpp
index a18a54c7fad0690..971591afb08dba2 100644
--- a/clang/test/Parser/cxx2a-concepts-requires-expr.cpp
+++ b/clang/test/Parser/cxx2a-concepts-requires-expr.cpp
@@ -160,3 +160,11 @@ template 
 requires requires {
  typename BitInt; // ok
 } using r44 = void;
+
+namespace GH73112 {
+void f() {
+requires { requires(int; } // expected-error {{expected ')'}} \
+   // expected-error {{expected expression}} \
+   // expected-note {{to match this '('}}
+}
+}

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

[clang] [clang][NFC] Refactor expected directives in C++ DRs 1-99 (PR #73879)

[Shafik Yaghmour via cfe-commits]

https://github.com/shafik edited https://github.com/llvm/llvm-project/pull/73879
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][NFC] Refactor expected directives in C++ DRs 1-99 (PR #73879)

[Shafik Yaghmour via cfe-commits]

https://github.com/shafik commented:

I think I mostly like this direction.

https://github.com/llvm/llvm-project/pull/73879
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][NFC] Refactor expected directives in C++ DRs 1-99 (PR #73879)

[Shafik Yaghmour via cfe-commits]

@@ -80,14 +86,21 @@ namespace dr5 { // dr5: 3.1
 
 namespace dr7 { // dr7: 3.4
   class A { public: ~A(); };
-  class B : virtual private A {}; // expected-note 2 {{declared private here}}
-  class C : public B {} c; // expected-error 2 {{inherited virtual base class 
'A' has private destructor}} \
-   // expected-note {{implicit default constructor for 
'dr7::C' first required here}} \
-   // expected-note {{implicit destructor for 'dr7::C' 
first required here}}
+  class B : virtual private A {}; // #dr7-B
+  class C : public B {} c; // #dr7-C
+  // expected-error@#dr7-C {{inherited virtual base class 'A' has private 
destructor}}
+  // expected-note@#dr7-C {{in implicit default constructor for 'dr7::C' first 
required here}}
+  // expected-note@#dr7-B {{declared private here}}

shafik wrote:

I have to say, it is kind of messing with me that we have `-C` before `-B` but 
I then realized that `B` and `C` are the class names here and don't imply some 
sort of ordering.

I wanted to point that out in case others also felt that way.

https://github.com/llvm/llvm-project/pull/73879
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Lex] Fix parsing of nested requirement to prevent flowing off the end of token stream (PR #73691)

[Shafik Yaghmour via cfe-commits]

https://github.com/shafik closed https://github.com/llvm/llvm-project/pull/73691
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Eagerly instantiate used constexpr function upon definition. (PR #73463)

[Shafik Yaghmour via cfe-commits]

https://github.com/shafik approved this pull request.

Thank you, LGTM

https://github.com/llvm/llvm-project/pull/73463
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Accept lambdas in C++03 as an extensions (PR #73376)

[Shafik Yaghmour via cfe-commits]

shafik wrote:

> Was there an RFC asking the community about exposing lambdas in pre C++11 
> modes? This is a sufficiently large language extension that we probably 
> should verify if we haven't already. I believe this is a conforming extension 
> (I can't think of a circumstance under which we'd take correct C++98 code and 
> treat it differently), but I don't think it's as simple as you've done here. 
> For example, `noexcept` is a C++11-ism and can appear on a lambda. 
> `constexpr` as well, trailing return types, etc. So we'd need a better 
> understanding of what features of lambdas you intend to enable and just how 
> much of the syntax you expect to work in older modes.

Also generic lambdas is probably a big one to think about as well. 

https://github.com/llvm/llvm-project/pull/73376
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][NFC] Refactor expected directives in C++ DRs 1-99 (PR #73879)

[Shafik Yaghmour via cfe-commits]

@@ -80,14 +86,21 @@ namespace dr5 { // dr5: 3.1
 
 namespace dr7 { // dr7: 3.4
   class A { public: ~A(); };
-  class B : virtual private A {}; // expected-note 2 {{declared private here}}
-  class C : public B {} c; // expected-error 2 {{inherited virtual base class 
'A' has private destructor}} \
-   // expected-note {{implicit default constructor for 
'dr7::C' first required here}} \
-   // expected-note {{implicit destructor for 'dr7::C' 
first required here}}
+  class B : virtual private A {}; // #dr7-B
+  class C : public B {} c; // #dr7-C
+  // expected-error@#dr7-C {{inherited virtual base class 'A' has private 
destructor}}
+  // expected-note@#dr7-C {{in implicit default constructor for 'dr7::C' first 
required here}}
+  // expected-note@#dr7-B {{declared private here}}

shafik wrote:

This is not a sticking point for me, just an observation and to see how others 
felt.

https://github.com/llvm/llvm-project/pull/73879
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Implement P2308R1 - Template Parameter Initialization. (PR #73103)

[Shafik Yaghmour via cfe-commits]

https://github.com/shafik approved this pull request.

LGTM

https://github.com/llvm/llvm-project/pull/73103
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Fix crash when declaring invalid lambda member (PR #74110)

[Shafik Yaghmour via cfe-commits]

shafik wrote:

Can you add a little more details in the description on the root cause of the 
bug?

https://github.com/llvm/llvm-project/pull/74110
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][ASTImporter] Fix import of variable template redeclarations. (PR #72841)

[Shafik Yaghmour via cfe-commits]

=?utf-8?q?Bal=C3=A1zs_K=C3=A9ri?= 
Message-ID:
In-Reply-To: 



@@ -5050,6 +5050,59 @@ TEST_P(ImportFriendClasses, RecordVarTemplateDecl) {
   EXPECT_EQ(ToTUX, ToX);
 }
 
+TEST_P(ASTImporterOptionSpecificTestBase, VarTemplateDeclConflict) {
+  getToTuDecl(
+  R"(
+  template 
+  constexpr int X = 1;
+  )",
+  Lang_CXX14);
+
+  Decl *FromTU = getTuDecl(
+  R"(
+  template 
+  constexpr int X = 2;
+  )",
+  Lang_CXX14, "input1.cc");
+  auto *FromX = FirstDeclMatcher().match(
+  FromTU, varTemplateDecl(hasName("X")));
+  auto *ToX = Import(FromX, Lang_CXX11);
+  // FIXME: This import should fail.
+  EXPECT_TRUE(ToX);
+}
+
+TEST_P(ASTImporterOptionSpecificTestBase, VarTemplateStaticDefinition) {
+  Decl *ToTU = getToTuDecl(
+  R"(
+  struct A {
+template 
+static int X;
+  };
+  )",
+  Lang_CXX14);
+  auto *ToX = FirstDeclMatcher().match(
+  ToTU, varTemplateDecl(hasName("X")));
+  ASSERT_FALSE(ToX->isThisDeclarationADefinition());
+
+  Decl *FromTU = getTuDecl(
+  R"(
+  struct A {
+template 

shafik wrote:

Did you mean to drop the `class V` here? 

https://github.com/llvm/llvm-project/pull/72841
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Reject incomplete type arguments for __builtin_dump_struct (PR #72749)

[Shafik Yaghmour via cfe-commits]

https://github.com/shafik approved this pull request.

LGTM you should put the bug report in the problem description so it 
automatically links the PR and closes it when you merge it.

https://github.com/llvm/llvm-project/pull/72749
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[flang] [libcxx] [clang] [compiler-rt] [clang-tools-extra] [llvm] [Clang][Sema] Fix qualifier restriction of overriden methods (PR #71696)

[Shafik Yaghmour via cfe-commits]

@@ -18469,9 +18469,22 @@ bool Sema::CheckOverridingFunctionReturnType(const 
CXXMethodDecl *New,
 
 
   // The new class type must have the same or less qualifiers as the old type.
-  if (NewClassTy.isMoreQualifiedThan(OldClassTy)) {
+  if (!OldClassTy.isAtLeastAsQualifiedAs(NewClassTy)) {

shafik wrote:

In the comment right above we can add a standard reference see: 
https://eel.is/c++draft/class.virtual#8

e.g.

```cpp
// C++ [class.virtual]p8:
//   The return type of an overriding function shall be either identical to the 
return type of the overridden function or [covariant]
```

https://github.com/llvm/llvm-project/pull/71696
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[flang] [libcxx] [clang] [compiler-rt] [clang-tools-extra] [llvm] [Clang][Sema] Fix qualifier restriction of overriden methods (PR #71696)

[Shafik Yaghmour via cfe-commits]

@@ -18469,9 +18469,22 @@ bool Sema::CheckOverridingFunctionReturnType(const 
CXXMethodDecl *New,
 
 
   // The new class type must have the same or less qualifiers as the old type.
-  if (NewClassTy.isMoreQualifiedThan(OldClassTy)) {
+  if (!OldClassTy.isAtLeastAsQualifiedAs(NewClassTy)) {

shafik wrote:

and also add a release note.

https://github.com/llvm/llvm-project/pull/71696
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Improve bit-field in ref NTTP diagnostic (PR #71077)

[Shafik Yaghmour via cfe-commits]

https://github.com/shafik approved this pull request.

LGTM

https://github.com/llvm/llvm-project/pull/71077
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Improve bit-field in ref NTTP diagnostic (PR #71077)

[Shafik Yaghmour via cfe-commits]

shafik wrote:

> > Can you add a release note?
> 
> Do you think this small diagnostic wording change is worth noting in relnotes?

Yes, we have a `Improvments to clang diagnostics` section. 

https://github.com/llvm/llvm-project/pull/71077
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] assert fail when number of arguments in pack exceed implement… (PR #74220)

[Shafik Yaghmour via cfe-commits]

shafik wrote:

> Thanks for this PR! Emitting a diag in `TransformTemplateParmRefExpr` would 
> probably a better way to go about it. It's not clear to me that the assert` 
> is an improvement , and adding a diag would not be a lot more work. Would 
> that be something you would be willing to explore?

Agreed.

https://github.com/llvm/llvm-project/pull/74220
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] assert fail when number of arguments in pack exceed implement… (PR #74220)

[Shafik Yaghmour via cfe-commits]

shafik wrote:

Please make sure before you commit that you update the description with more 
details. This is what will show up in the git log and we want to make sure that 
is detailed enough to allow folks to debug build issues etc without having to 
examine each commit in detail.

https://github.com/llvm/llvm-project/pull/74220
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Avoid printing overly large integer. (PR #75902)

[Shafik Yaghmour via cfe-commits]

shafik wrote:

> I agree with @tbaederr, fixing the underlying issue - either by optimizing 
> divide (good luck!) or printing in hexadecimal, seems like a better solution. 
> At the minimum, we should have a fixme comment to explain the restriction
> 
> @tbaederr @erichkeane @shafik opinion?

I agree the hexadecimal option feel like the better solution. Should we also 
have a diagnostic when we hit this case?

Also as mentioned we need some test cases as well. 

https://github.com/llvm/llvm-project/pull/75902
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Parser] Fix crash of clang when using C++ constructs like :: in C code (PR #74926)

[Shafik Yaghmour via cfe-commits]

https://github.com/shafik approved this pull request.

If you have addressed all review comments and no one has asked you to wait to 
land the PR then you can land once you get an approval.

https://github.com/llvm/llvm-project/pull/74926
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Sema] Fix Wswitch-default bad warning in template (PR #76007)

[Shafik Yaghmour via cfe-commits]

shafik wrote:

> For now I guess this is Ok, although I think the better fix would be to 
> diagnose missing or duplicate `default` labels even in the dependent case. 
> Because `default` labels themselves are never dependent.

We should probably add a FIXME for this.

https://github.com/llvm/llvm-project/pull/76007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Add -Wc++11-narrowing-const-reference (PR #76094)

[Shafik Yaghmour via cfe-commits]

@@ -6158,12 +6158,24 @@ def err_illegal_initializer_type : Error<"illegal 
initializer type %0">;
 def ext_init_list_type_narrowing : ExtWarn<
   "type %0 cannot be narrowed to %1 in initializer list">,
   InGroup, DefaultError, SFINAEFailure;
+// *_narrowing_const_reference diagnostics have the same messages, but are
+// controlled by -Wc++11-narrowing-const-reference for narrowing involving a
+// const reference.
+def ext_init_list_type_narrowing_const_reference : ExtWarn<

shafik wrote:

There should be a test for each of these new diagnostics.

https://github.com/llvm/llvm-project/pull/76094
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Fix crash on invalid code with parenthesized aggregate initialization (PR #76232)

[Shafik Yaghmour via cfe-commits]

https://github.com/shafik commented:

This is missing a release note.

https://github.com/llvm/llvm-project/pull/76232
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][ASTImporter] import InstantiatedFromMember of ClassTemplateSpecializationDecl (PR #76493)

[Shafik Yaghmour via cfe-commits]

https://github.com/shafik commented:

LGTM but I will let @balazske approve

https://github.com/llvm/llvm-project/pull/76493
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][ASTImporter] import InstantiatedFromMember of ClassTemplateSpecializationDecl (PR #76493)

[Shafik Yaghmour via cfe-commits]

@@ -9342,6 +9342,38 @@ TEST_P(ASTImporterOptionSpecificTestBase, 
ImportConflictTypeAliasTemplate) {
   EXPECT_FALSE(ImportedCallable);
 }
 
+AST_MATCHER(ClassTemplateSpecializationDecl, hasInstantiatedFromMember) {
+  if (auto Instantiate = Node.getInstantiatedFrom()) {
+if (auto *FromPartialSpecialization =
+Instantiate.get()) {
+  return nullptr != FromPartialSpecialization->getInstantiatedFromMember();
+}
+  }
+  return false;
+}
+
+TEST_P(ASTImporterOptionSpecificTestBase, ImportInstantiatedFromMember) {
+  const char *Code =
+  R"(
+  template  struct B {
+template  union D;
+template  union D {};
+D d;
+  };
+  B b;
+  )";
+  Decl *FromTU = getTuDecl(Code, Lang_CXX17);

shafik wrote:

nitpick, I don't think this requires C++17 even though the original test case 
uses C++17.

https://github.com/llvm/llvm-project/pull/76493
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][ASTImporter] import InstantiatedFromMember of ClassTemplateSpecializationDecl (PR #76493)

[Shafik Yaghmour via cfe-commits]

https://github.com/shafik edited https://github.com/llvm/llvm-project/pull/76493
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][Sema] deleted overriding function can have lax except spec (PR #76248)

[Shafik Yaghmour via cfe-commits]

@@ -979,6 +979,11 @@ bool Sema::CheckOverridingFunctionExceptionSpec(const 
CXXMethodDecl *New,
   if (isa(New) && New->getParent()->isDependentType())
 return false;
 
+  // CWG1351: if either of the old function or the new function is defined as
+  // deleted, we don't need this check.
+  if (Old->isDeleted() || New->isDeleted())

shafik wrote:

Either they both have to be deleted or not. We should diagnose otherwise. So I 
think checking both is redundant. 

https://github.com/llvm/llvm-project/pull/76248
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][AST] Fix crash in APValue::LValueBase::getType when we have invalid decl (PR #75130)

[Shafik Yaghmour via cfe-commits]

@@ -70,11 +70,13 @@ QualType APValue::LValueBase::getType() const {
 // constexpr int *p = &arr[1]; // valid?
 //
 // For now, we take the most complete type we can find.
-for (auto *Redecl = cast(D->getMostRecentDecl()); Redecl;
+for (auto *Redecl = cast(D->getMostRecentDecl());
+ Redecl && !Redecl->isInvalidDecl();

shafik wrote:

We are dealing with ill-formed code here. So there is not "right answer", 
right? Are you saying that we might misdiagnose an error?

Or are you saying the original code is incorrect and you think there is 
well-formed code we will not do the correct thing on?

https://github.com/llvm/llvm-project/pull/75130
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Correctly construct template arguments for file-scope template template parameters (PR #76811)

[Shafik Yaghmour via cfe-commits]

@@ -345,15 +345,19 @@ MultiLevelTemplateArgumentList 
Sema::getTemplateInstantiationArgs(
 
   using namespace TemplateInstArgsHelpers;
   const Decl *CurDecl = ND;
+
+  if (!ND)
+CurDecl = Decl::castFromDeclContext(DC);
+
   if (Innermost) {
 Result.addOuterTemplateArguments(const_cast(ND),
  Innermost->asArray(), Final);
-CurDecl = Response::UseNextDecl(ND).NextDecl;
+if (CurDecl->getDeclContext()->isFileContext())

shafik wrote:

I feel like a comment here would be helpful.

https://github.com/llvm/llvm-project/pull/76811
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Correctly construct template arguments for file-scope template template parameters (PR #76811)

[Shafik Yaghmour via cfe-commits]

@@ -345,15 +345,19 @@ MultiLevelTemplateArgumentList 
Sema::getTemplateInstantiationArgs(
 
   using namespace TemplateInstArgsHelpers;
   const Decl *CurDecl = ND;
+
+  if (!ND)

shafik wrote:

Can we also get a comment here why this makes sense to call 
`Decl::castFromDeclContext(DC)` if `CurrDecl` is not present?

https://github.com/llvm/llvm-project/pull/76811
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Correctly construct template arguments for file-scope template template parameters (PR #76811)

[Shafik Yaghmour via cfe-commits]

@@ -345,15 +345,19 @@ MultiLevelTemplateArgumentList 
Sema::getTemplateInstantiationArgs(
 
   using namespace TemplateInstArgsHelpers;
   const Decl *CurDecl = ND;
+
+  if (!ND)

shafik wrote:

Seems less error prone to check `!CurDecl` here. If the surrounding code get 
modified or refactored the relationship between `ND` and `CurDecl` may break 
down.

https://github.com/llvm/llvm-project/pull/76811
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits