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

2018-10-31 Thread 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=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=345783=345784=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=345783=345784=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


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

2019-03-27 Thread 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=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=357099=357100=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


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

2019-03-04 Thread 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=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=355331=355332=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=355331=355332=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  = 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  = 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 existing MemoryBuffer!
   bool Invalid 

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

2019-01-28 Thread 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=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=352435=352436=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=352435=352436=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 {
 int chf2(int x);
 


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

2019-04-08 Thread 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=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=357939=357940=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


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

2019-05-24 Thread 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=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=361649=361650=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


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

2019-04-26 Thread 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=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=359337=359338=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  : 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=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=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

2019-08-12 Thread 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=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=368590=368591=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=368590=368591=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


r370107 - Debug Info: Support for DW_AT_export_symbols for anonymous structs

2019-08-27 Thread Shafik Yaghmour via cfe-commits
Author: shafik
Date: Tue Aug 27 13:17:35 2019
New Revision: 370107

URL: http://llvm.org/viewvc/llvm-project?rev=370107=rev
Log:
Debug Info: Support for DW_AT_export_symbols for anonymous structs

This implements the DWARF 5 feature described in:

http://dwarfstd.org/ShowIssue.php?issue=141212.1

To support recognizing anonymous structs:

  struct A {
struct { // Anonymous struct
int y;
};
  } a;

This patch adds support in CGDebugInfo::CreateLimitedType(...) for this new 
flag and an accompanying test to verify this feature.

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

Added:
cfe/trunk/test/CodeGenCXX/debug-info-export_symbols.cpp
Modified:
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp

Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=370107=370106=370107=diff
==
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Tue Aug 27 13:17:35 2019
@@ -3121,7 +3121,8 @@ llvm::DICompositeType *CGDebugInfo::Crea
 
   SmallString<256> Identifier = getTypeIdentifier(Ty, CGM, TheCU);
 
-  // Explicitly record the calling convention for C++ records.
+  // Explicitly record the calling convention and export symbols for C++
+  // records.
   auto Flags = llvm::DINode::FlagZero;
   if (auto CXXRD = dyn_cast(RD)) {
 if (CGM.getCXXABI().getRecordArgABI(CXXRD) == CGCXXABI::RAA_Indirect)
@@ -3132,6 +3133,10 @@ llvm::DICompositeType *CGDebugInfo::Crea
 // Record if a C++ record is non-trivial type.
 if (!CXXRD->isTrivial())
   Flags |= llvm::DINode::FlagNonTrivial;
+
+// Record exports it symbols to the containing structure.
+if (CXXRD->isAnonymousStructOrUnion())
+Flags |= llvm::DINode::FlagExportSymbols;
   }
 
   llvm::DICompositeType *RealDecl = DBuilder.createReplaceableCompositeType(

Added: cfe/trunk/test/CodeGenCXX/debug-info-export_symbols.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-export_symbols.cpp?rev=370107=auto
==
--- cfe/trunk/test/CodeGenCXX/debug-info-export_symbols.cpp (added)
+++ cfe/trunk/test/CodeGenCXX/debug-info-export_symbols.cpp Tue Aug 27 13:17:35 
2019
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -emit-llvm -debug-info-kind=standalone -triple 
%itanium_abi_triple %s -o - | FileCheck %s
+
+// CHECK: [[SCOPE:![0-9]+]] = distinct !DICompositeType({{.*}}flags: 
DIFlagTypePassByValue
+// CHECK: !DICompositeType(tag: DW_TAG_structure_type, scope: [[SCOPE]]
+// CHECK-SAME:  DIFlagExportSymbols | 
DIFlagTypePassByValue
+struct A {
+ // Anonymous class exports its symbols into A
+ struct {
+ int y;
+ };
+} a;


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


[clang] f56cb52 - [DEBUGINFO] [LLDB] Add support for generating debug-info for structured bindings of structs and arrays

2022-02-17 Thread Shafik Yaghmour via cfe-commits

Author: Shafik Yaghmour
Date: 2022-02-17T11:14:14-08:00
New Revision: f56cb520d8554ca42a215e82ecfa58d0b6c178e4

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

LOG: [DEBUGINFO] [LLDB] Add support for generating debug-info for structured 
bindings of structs and arrays

Currently we are not emitting debug-info for all cases of structured bindings a
C++17 feature which allows us to bind names to subobjects in an initializer.

A structured binding is represented by a DecompositionDecl AST node and the
binding are represented by a BindingDecl. It looks the original implementation
only covered the tuple like case which be represented by a DeclRefExpr which
contains a VarDecl.

If the binding is to a subobject of the struct the binding will contain a
MemberExpr and in the case of arrays it will contain an ArraySubscriptExpr.
This PR adds support emitting debug-info for the MemberExpr and 
ArraySubscriptExpr
cases as well as llvm and lldb tests for these cases as well as the tuple case.

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

Added: 
clang/test/CodeGenCXX/debug-info-structured-binding.cpp
lldb/test/API/lang/cpp/structured-binding/Makefile
lldb/test/API/lang/cpp/structured-binding/TestStructuredBinding.py
lldb/test/API/lang/cpp/structured-binding/main.cpp

Modified: 
clang/lib/CodeGen/CGDebugInfo.cpp
clang/lib/CodeGen/CGDebugInfo.h

Removed: 




diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp 
b/clang/lib/CodeGen/CGDebugInfo.cpp
index d75b5a1a9d125..2203f0aec5c7c 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -4635,11 +4635,103 @@ llvm::DILocalVariable *CGDebugInfo::EmitDeclare(const 
VarDecl *VD,
   return D;
 }
 
+llvm::DILocalVariable *CGDebugInfo::EmitDeclare(const BindingDecl *BD,
+llvm::Value *Storage,
+llvm::Optional ArgNo,
+CGBuilderTy ,
+const bool UsePointerValue) {
+  assert(CGM.getCodeGenOpts().hasReducedDebugInfo());
+  assert(!LexicalBlockStack.empty() && "Region stack mismatch, stack empty!");
+  if (BD->hasAttr())
+return nullptr;
+
+  // Skip the tuple like case, we don't handle that here
+  if (isa(BD->getBinding()))
+return nullptr;
+
+  llvm::DIFile *Unit = getOrCreateFile(BD->getLocation());
+  llvm::DIType *Ty = getOrCreateType(BD->getType(), Unit);
+
+  // If there is no debug info for this type then do not emit debug info
+  // for this variable.
+  if (!Ty)
+return nullptr;
+
+  auto Align = getDeclAlignIfRequired(BD, CGM.getContext());
+  unsigned AddressSpace = 
CGM.getContext().getTargetAddressSpace(BD->getType());
+
+  SmallVector Expr;
+  AppendAddressSpaceXDeref(AddressSpace, Expr);
+
+  // Clang stores the sret pointer provided by the caller in a static alloca.
+  // Use DW_OP_deref to tell the debugger to load the pointer and treat it as
+  // the address of the variable.
+  if (UsePointerValue) {
+assert(!llvm::is_contained(Expr, llvm::dwarf::DW_OP_deref) &&
+   "Debug info already contains DW_OP_deref.");
+Expr.push_back(llvm::dwarf::DW_OP_deref);
+  }
+
+  unsigned Line = getLineNumber(BD->getLocation());
+  unsigned Column = getColumnNumber(BD->getLocation());
+  StringRef Name = BD->getName();
+  auto *Scope = cast(LexicalBlockStack.back());
+  // Create the descriptor for the variable.
+  llvm::DILocalVariable *D = DBuilder.createAutoVariable(
+  Scope, Name, Unit, Line, Ty, CGM.getLangOpts().Optimize,
+  llvm::DINode::FlagZero, Align);
+
+  if (const MemberExpr *ME = dyn_cast(BD->getBinding())) {
+if (const FieldDecl *FD = dyn_cast(ME->getMemberDecl())) {
+  const unsigned fieldIndex = FD->getFieldIndex();
+  const clang::CXXRecordDecl *parent =
+  (const CXXRecordDecl *)FD->getParent();
+  const ASTRecordLayout  =
+  CGM.getContext().getASTRecordLayout(parent);
+  const uint64_t fieldOffset = layout.getFieldOffset(fieldIndex);
+
+  if (fieldOffset != 0) {
+Expr.push_back(llvm::dwarf::DW_OP_plus_uconst);
+Expr.push_back(
+CGM.getContext().toCharUnitsFromBits(fieldOffset).getQuantity());
+  }
+}
+  } else if (const ArraySubscriptExpr *ASE =
+ dyn_cast(BD->getBinding())) {
+if (const IntegerLiteral *IL = dyn_cast(ASE->getIdx())) {
+  const uint64_t value = IL->getValue().getZExtValue();
+  const uint64_t typeSize = CGM.getContext().getTypeSize(BD->getType());
+
+  if (value != 0) {
+Expr.push_back(llvm::dwarf::DW_OP_plus_uconst);
+Expr.push_back(CGM.getContext()
+   .toCharUnitsFromBits(value * 

[clang] [AST] Fix nested name specifiers printing as NamespaceNamespace (PR #65266)

2023-09-05 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik commented:

LGTM but we should test all changes.

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


[clang] [AST] Fix nested name specifiers printing as NamespaceNamespace (PR #65266)

2023-09-05 Thread Shafik Yaghmour via cfe-commits


@@ -792,11 +793,11 @@ void clang::TextNodeDumper::dumpNestedNameSpecifier(const 
NestedNameSpecifier *N
   OS << " '" << NNS->getAsIdentifier()->getName() << "'";
   break;
 case NestedNameSpecifier::Namespace:
-  OS << " Namespace";
+  OS << " "; // "Namespace" is printed as the decl kind.
   dumpBareDeclRef(NNS->getAsNamespace());
   break;
 case NestedNameSpecifier::NamespaceAlias:
-  OS << " NamespaceAlias";
+  OS << " "; // "NamespaceAlias" is printed as the decl kind.

shafik wrote:

I don't see a test that covers this case.

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


[clang] [AST] Fix nested name specifiers printing as NamespaceNamespace (PR #65266)

2023-09-05 Thread Shafik Yaghmour via cfe-commits

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


[clang] [AST] Only dump desugared type when visibly different (PR #65214)

2023-09-05 Thread Shafik Yaghmour via cfe-commits

shafik wrote:

So it looks like some of these changes undo some of the change introduced by 
@mizvekov in some tests. Maybe @zygoloid or @AaronBallman has some more input 
here.

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


[clang] [Clang] Fix the for statement disappearing in AST when an error occurs in the conditional expression of the for statement (PR #65381)

2023-09-06 Thread Shafik Yaghmour via cfe-commits


@@ -2158,8 +2158,10 @@ StmtResult Parser::ParseForStatement(SourceLocation 
*TrailingElseLoc) {
 // for-range-declaration next.
 bool MightBeForRangeStmt = !ForRangeInfo.ParsedForRangeDecl();
 ColonProtectionRAIIObject ColonProtection(*this, MightBeForRangeStmt);
+SourceLocation SecondPartStart = Tok.getLocation();
+Sema::ConditionKind CK = Sema::ConditionKind::Boolean;
 SecondPart = ParseCXXCondition(
-nullptr, ForLoc, Sema::ConditionKind::Boolean,
+nullptr, ForLoc, CK,
 // FIXME: recovery if we don't see another semi!
 /*MissingOK=*/true, MightBeForRangeStmt ?  : nullptr,
 /*EnterForConditionScope*/ true);

shafik wrote:

```suggestion
/*InitStmt=*/nullptr, ForLoc, CK,
// FIXME: recovery if we don't see another semi!
/*MissingOK=*/true, MightBeForRangeStmt ?  : nullptr,
/*EnterForConditionScope=*/true);
```

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


[clang] [Clang] Fix the for statement disappearing in AST when an error occurs in the conditional expression of the for statement (PR #65381)

2023-09-06 Thread Shafik Yaghmour via cfe-commits

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


[clang] [Clang] Fix the for statement disappearing in AST when an error occurs in the conditional expression of the for statement (PR #65381)

2023-09-06 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik commented:

This looks good to me but I would like another set of eyes on it.

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


[clang] [AST] Only dump desugared type when visibly different (PR #65214)

2023-09-06 Thread Shafik Yaghmour via cfe-commits

shafik wrote:

> > So it looks like some of these changes undo some of the change introduced 
> > by @mizvekov in some tests. Maybe @zygoloid or @AaronBallman has some more 
> > input here.
> 
> Do you have a pointer to such changes?

One of the commits is here: 15f3cd6bfc670 

I know he did a lot of work in this area and I am not familiar with it in 
detail but I want to make sure interested parties are not stepping on each 
others work.

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


[clang] Fix out of line Concept-comparisons of NestedNameSpecifiers (PR #65993)

2023-09-11 Thread Shafik Yaghmour via cfe-commits


@@ -231,14 +231,18 @@ Response HandleFunctionTemplateDecl(const 
FunctionTemplateDecl *FTD,
 MultiLevelTemplateArgumentList ) {
   if (!isa(FTD->getDeclContext())) {
 NestedNameSpecifier *NNS = FTD->getTemplatedDecl()->getQualifier();
-const Type *Ty;
-const TemplateSpecializationType *TSTy;
-if (NNS && (Ty = NNS->getAsType()) &&
-(TSTy = Ty->getAs()))
-  Result.addOuterTemplateArguments(const_cast(FTD),
-   TSTy->template_arguments(),
-   /*Final=*/false);
+
+while (const Type *Ty = NNS ? NNS->getAsType() : nullptr) {

shafik wrote:

Yeah, I just realized I was looking at the code a moment ago 
`FTD->getTemplatedDecl()->getQualifier()`

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


[clang] Fix out of line Concept-comparisons of NestedNameSpecifiers (PR #65993)

2023-09-11 Thread Shafik Yaghmour via cfe-commits


@@ -231,14 +231,18 @@ Response HandleFunctionTemplateDecl(const 
FunctionTemplateDecl *FTD,
 MultiLevelTemplateArgumentList ) {
   if (!isa(FTD->getDeclContext())) {
 NestedNameSpecifier *NNS = FTD->getTemplatedDecl()->getQualifier();
-const Type *Ty;
-const TemplateSpecializationType *TSTy;
-if (NNS && (Ty = NNS->getAsType()) &&
-(TSTy = Ty->getAs()))
-  Result.addOuterTemplateArguments(const_cast(FTD),
-   TSTy->template_arguments(),
-   /*Final=*/false);
+
+while (const Type *Ty = NNS ? NNS->getAsType() : nullptr) {

shafik wrote:

It does not look like there is a simple way of getting like w/ `CXXRecordDecl` 
so some work would be involved.

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


[clang] Fix out of line Concept-comparisons of NestedNameSpecifiers (PR #65993)

2023-09-11 Thread Shafik Yaghmour via cfe-commits


@@ -231,14 +231,18 @@ Response HandleFunctionTemplateDecl(const 
FunctionTemplateDecl *FTD,
 MultiLevelTemplateArgumentList ) {
   if (!isa(FTD->getDeclContext())) {
 NestedNameSpecifier *NNS = FTD->getTemplatedDecl()->getQualifier();
-const Type *Ty;
-const TemplateSpecializationType *TSTy;
-if (NNS && (Ty = NNS->getAsType()) &&
-(TSTy = Ty->getAs()))
-  Result.addOuterTemplateArguments(const_cast(FTD),
-   TSTy->template_arguments(),
-   /*Final=*/false);
+
+while (const Type *Ty = NNS ? NNS->getAsType() : nullptr) {

shafik wrote:

This make sense, I was just looking at the AST: https://godbolt.org/z/P3brjcsMG

and I see that `InnerClass` has:

```console
| |-NestedNameSpecifier TypeSpec 'Base'
```

and I wondering what the prefix is there? Maybe the AST is not telling the 
whole story?

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


[clang] [Clang] Handle consteval expression in array bounds expressions (PR #66222)

2023-09-13 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik commented:

If the function is `constexpr` do we treat it as a VLA? 

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


[clang] [Clang] Handle consteval expression in array bounds expressions (PR #66222)

2023-09-13 Thread Shafik Yaghmour via cfe-commits


@@ -13928,7 +13930,7 @@ static bool CheckTautologicalComparison(Sema , 
BinaryOperator *E,
 return false;
 
   IntRange OtherValueRange = GetExprRange(
-  S.Context, Other, S.isConstantEvaluated(), /*Approximate*/ false);
+  S.Context, Other, S.isConstantEvaluatedContext(), /*Approximate*/ false);

shafik wrote:

```suggestion
  S.Context, Other, S.isConstantEvaluatedContext(), /*Approximate=*/ false);
```

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


[clang] [Clang] Handle consteval expression in array bounds expressions (PR #66222)

2023-09-13 Thread Shafik Yaghmour via cfe-commits


@@ -13928,7 +13930,7 @@ static bool CheckTautologicalComparison(Sema , 
BinaryOperator *E,
 return false;
 
   IntRange OtherValueRange = GetExprRange(
-  S.Context, Other, S.isConstantEvaluated(), /*Approximate*/ false);
+  S.Context, Other, S.isConstantEvaluatedContext(), /*Approximate*/ false);

shafik wrote:

```suggestion
  S.Context, Other, S.isConstantEvaluatedContext(), /*Approximate=*/ false);
```

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


[clang] [Clang] Handle consteval expression in array bounds expressions (PR #66222)

2023-09-13 Thread Shafik Yaghmour via cfe-commits

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


[clang] [Clang] Handle consteval expression in array bounds expressions (PR #66222)

2023-09-13 Thread Shafik Yaghmour via cfe-commits


@@ -15159,16 +15163,16 @@ static void CheckImplicitConversion(Sema , Expr *E, 
QualType T,
 
   IntRange SourceTypeRange =
   IntRange::forTargetOfCanonicalType(S.Context, Source);
-  IntRange LikelySourceRange =
-  GetExprRange(S.Context, E, S.isConstantEvaluated(), /*Approximate*/ 
true);
+  IntRange LikelySourceRange = GetExprRange(
+  S.Context, E, S.isConstantEvaluatedContext(), /*Approximate*/ true);

shafik wrote:

```suggestion
  S.Context, E, S.isConstantEvaluatedContext(), /*Approximate=*/ true);
```

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


[clang] [Clang] Handle consteval expression in array bounds expressions (PR #66222)

2023-09-13 Thread Shafik Yaghmour via cfe-commits


@@ -14143,8 +14145,9 @@ static void AnalyzeComparison(Sema , BinaryOperator 
*E) {
   }
 
   // Otherwise, calculate the effective range of the signed operand.
-  IntRange signedRange = GetExprRange(
-  S.Context, signedOperand, S.isConstantEvaluated(), /*Approximate*/ true);
+  IntRange signedRange =
+  GetExprRange(S.Context, signedOperand, S.isConstantEvaluatedContext(),
+   /*Approximate*/ true);

shafik wrote:

```suggestion
   /*Approximate=*/ true);
```

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


[clang] [Clang] Handle consteval expression in array bounds expressions (PR #66222)

2023-09-13 Thread Shafik Yaghmour via cfe-commits


@@ -15090,8 +15093,9 @@ static void CheckImplicitConversion(Sema , Expr *E, 
QualType T,
   if (SourceBT && TargetBT && SourceBT->isIntegerType() &&
   TargetBT->isFloatingType() && !IsListInit) {
 // Determine the number of precision bits in the source integer type.
-IntRange SourceRange = GetExprRange(S.Context, E, S.isConstantEvaluated(),
-/*Approximate*/ true);
+IntRange SourceRange =
+GetExprRange(S.Context, E, S.isConstantEvaluatedContext(),
+ /*Approximate*/ true);

shafik wrote:

```suggestion
 /*Approximate=*/ true);
```

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


[clang] [Clang] Handle consteval expression in array bounds expressions (PR #66222)

2023-09-13 Thread Shafik Yaghmour via cfe-commits


@@ -9843,30 +9833,44 @@ class Sema final {
   /// diagnostics that will be suppressed.
   std::optional isSFINAEContext() const;
 
-  /// Determines whether we are currently in a context that
-  /// is not evaluated as per C++ [expr] p5.
-  bool isUnevaluatedContext() const {
+  /// Whether the AST is currently being rebuilt to correct immediate
+  /// invocations. Immediate invocation candidates and references to consteval
+  /// functions aren't tracked when this is set.
+  bool RebuildingImmediateInvocation = false;
+
+  /// Used to change context to isConstantEvaluated without pushing a heavy
+  /// ExpressionEvaluationContextRecord object.
+  bool isConstantEvaluatedOverride;

shafik wrote:

Is there no good default value?

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


[clang] Introduce paged vector (PR #66430)

2023-09-15 Thread Shafik Yaghmour via cfe-commits


@@ -0,0 +1,84 @@
+//===- llvm/unittest/ADT/PagedVectorTest.cpp 
--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// PagedVector unit tests.
+//
+//===--===//
+
+#include "llvm/ADT/PagedVector.h"
+#include "gtest/gtest.h"
+
+namespace llvm {
+TEST(PagedVectorTest, FunctionalityTest) {

shafik wrote:

I don't see tests for `clear()`, `empty()` or `at(..)` and I think we should 
have `EXPECT_DEATH` tests for assertions that are testable.

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


[clang] Introduce paged vector (PR #66430)

2023-09-15 Thread Shafik Yaghmour via cfe-commits

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


[clang] Introduce paged vector (PR #66430)

2023-09-15 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik commented:

Thank you for this contributions, I just have one comment so far on testing. We 
should make sure any new ADT is very well tested before landing it. 

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


[clang] [clang] Fix wrong warning about missing init for flexible array members (PR #66341)

2023-09-14 Thread Shafik Yaghmour via cfe-commits

shafik wrote:

Thank you for the quick fix

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


[clang] [Clang] Fix CXXRewrittenBinaryOperator::getDecomposedForm to handle case when spaceship operator returns comparison category by reference (PR #66270)

2023-09-14 Thread Shafik Yaghmour via cfe-commits

shafik wrote:

> Perhaps it needs a release note?

You would think I would remember this by now.

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


[clang] [Clang] Fix CXXRewrittenBinaryOperator::getDecomposedForm to handle case when spaceship operator returns comparison category by reference (PR #66270)

2023-09-14 Thread Shafik Yaghmour via cfe-commits

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

>From b934841d7fc3d3447b23a9718a38742943f76916 Mon Sep 17 00:00:00 2001
From: Shafik Yaghmour 
Date: Wed, 13 Sep 2023 11:09:28 -0700
Subject: [PATCH 1/2] [Clang] Fix CXXRewrittenBinaryOperator::getDecomposedForm
 to handle case when spaceship operator returns comparison category by
 reference

Currently CXXRewrittenBinaryOperator::getDecomposedForm(...) may crash if the
spaceship operator returns a comparison category by reference. This because
IgnoreImplicitAsWritten() does not look through CXXConstructExpr. The fix is to
use IgnoreUnlessSpelledInSource() which will look though CXXConstructExpr.

This fixes: https://github.com/llvm/llvm-project/issues/64162
---
 clang/lib/AST/ExprCXX.cpp|  2 +-
 clang/test/SemaCXX/compare-cxx2a.cpp | 23 +++
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/clang/lib/AST/ExprCXX.cpp b/clang/lib/AST/ExprCXX.cpp
index efb64842fb6d96d..06163255f9b5e54 100644
--- a/clang/lib/AST/ExprCXX.cpp
+++ b/clang/lib/AST/ExprCXX.cpp
@@ -111,7 +111,7 @@ CXXRewrittenBinaryOperator::getDecomposedForm() const {
 return Result;
 
   // Otherwise, we expect a <=> to now be on the LHS.
-  E = Result.LHS->IgnoreImplicitAsWritten();
+  E = Result.LHS->IgnoreUnlessSpelledInSource();
   if (auto *BO = dyn_cast(E)) {
 assert(BO->getOpcode() == BO_Cmp);
 Result.LHS = BO->getLHS();
diff --git a/clang/test/SemaCXX/compare-cxx2a.cpp 
b/clang/test/SemaCXX/compare-cxx2a.cpp
index 619e16aa7458179..15a0baccfca17a2 100644
--- a/clang/test/SemaCXX/compare-cxx2a.cpp
+++ b/clang/test/SemaCXX/compare-cxx2a.cpp
@@ -479,3 +479,26 @@ void DoSomething() {
   // expected-note  
{{undefined function 'operator++' cannot be used in a constant expression}}
 }
 }
+
+namespace GH64162 {
+struct S {
+const std::strong_ordering& operator<=>(const S&) const = default;
+};
+bool test(S s) {
+return s < s; // We expect this not to crash anymore
+}
+
+// Following example never crashed but worth adding in because it is related
+struct A {};
+bool operator<(A, int);
+
+struct B {
+operator A();
+};
+
+struct C {
+B operator<=>(C);
+};
+
+bool f(C c) { return c < c; }
+}

>From f91332e6e51fe442ecec2db2d203aa7650d9d7ab Mon Sep 17 00:00:00 2001
From: Shafik Yaghmour 
Date: Wed, 13 Sep 2023 11:09:28 -0700
Subject: [PATCH 2/2] [Clang] Fix CXXRewrittenBinaryOperator::getDecomposedForm
 to handle case when spaceship operator returns comparison category by
 reference

Currently CXXRewrittenBinaryOperator::getDecomposedForm(...) may crash if the
spaceship operator returns a comparison category by reference. This because
IgnoreImplicitAsWritten() does not look through CXXConstructExpr. The fix is to
use IgnoreUnlessSpelledInSource() which will look though CXXConstructExpr.

This fixes: https://github.com/llvm/llvm-project/issues/64162
---
 clang/docs/ReleaseNotes.rst  |  4 
 clang/lib/AST/ExprCXX.cpp|  2 +-
 clang/test/SemaCXX/compare-cxx2a.cpp | 23 +++
 3 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 3cdad2f7b9f0e5a..e35e7ed941bc79c 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -281,6 +281,10 @@ Bug Fixes to C++ Support
   a non-template inner-class between the function and the class template.
   (`#65810 `_)
 
+- Fix crash caused by a spaceship operator returning a comparision category by
+  reference. Fixes:
+  (`#64162 `_)
+
 Bug Fixes to AST Handling
 ^
 - Fixed an import failure of recursive friend class template.
diff --git a/clang/lib/AST/ExprCXX.cpp b/clang/lib/AST/ExprCXX.cpp
index efb64842fb6d96d..06163255f9b5e54 100644
--- a/clang/lib/AST/ExprCXX.cpp
+++ b/clang/lib/AST/ExprCXX.cpp
@@ -111,7 +111,7 @@ CXXRewrittenBinaryOperator::getDecomposedForm() const {
 return Result;
 
   // Otherwise, we expect a <=> to now be on the LHS.
-  E = Result.LHS->IgnoreImplicitAsWritten();
+  E = Result.LHS->IgnoreUnlessSpelledInSource();
   if (auto *BO = dyn_cast(E)) {
 assert(BO->getOpcode() == BO_Cmp);
 Result.LHS = BO->getLHS();
diff --git a/clang/test/SemaCXX/compare-cxx2a.cpp 
b/clang/test/SemaCXX/compare-cxx2a.cpp
index 619e16aa7458179..15a0baccfca17a2 100644
--- a/clang/test/SemaCXX/compare-cxx2a.cpp
+++ b/clang/test/SemaCXX/compare-cxx2a.cpp
@@ -479,3 +479,26 @@ void DoSomething() {
   // expected-note  
{{undefined function 'operator++' cannot be used in a constant expression}}
 }
 }
+
+namespace GH64162 {
+struct S {
+const std::strong_ordering& operator<=>(const S&) const = default;
+};
+bool test(S s) {
+return s < s; // We expect this not to crash anymore
+}
+
+// Following example 

[clang] 6eadc8f - [Clang] Fix crash in Parser::ParseDirectDeclarator by adding check that token is not an annotation token

2023-09-12 Thread Shafik Yaghmour via cfe-commits

Author: Shafik Yaghmour
Date: 2023-09-12T11:06:06-07:00
New Revision: 6eadc8f16e03f6aa3b1b1c178c308ac452eabeac

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

LOG: [Clang] Fix crash in Parser::ParseDirectDeclarator by adding check that 
token is not an annotation token

In Parser::ParseDirectDeclarator(...) in some cases ill-formed code can cause an
annotation token to end up where it was not expected. The fix is to add a
!Tok.isAnnotation() guard before attempting to access identifier info.

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

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

Added: 
clang/test/Parser/gh64836.cpp

Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/Parse/ParseDecl.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 768b322ca5e721d..3cdad2f7b9f0e5a 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -218,6 +218,8 @@ Bug Fixes in This Version
   (`#65156 `_)
 - Clang no longer considers the loss of ``__unaligned`` qualifier from objects 
as
   an invalid conversion during method function overload resolution.
+- Fix parser crash when dealing with ill-formed objective C++ header code. 
Fixes
+  (`#64836 `_)
 
 Bug Fixes to Compiler Builtins
 ^^

diff  --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index 7c27a02ee4af625..748b9d53c9f5b33 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -6667,7 +6667,7 @@ void Parser::ParseDirectDeclarator(Declarator ) {
   // Objective-C++: Detect C++ keywords and try to prevent further errors 
by
   // treating these keyword as valid member names.
   if (getLangOpts().ObjC && getLangOpts().CPlusPlus &&
-  Tok.getIdentifierInfo() &&
+  !Tok.isAnnotation() && Tok.getIdentifierInfo() &&
   Tok.getIdentifierInfo()->isCPlusPlusKeyword(getLangOpts())) {
 Diag(getMissingDeclaratorIdLoc(D, Tok.getLocation()),
  diag::err_expected_member_name_or_semi_objcxx_keyword)

diff  --git a/clang/test/Parser/gh64836.cpp b/clang/test/Parser/gh64836.cpp
new file mode 100644
index 000..12b726060cb3fd9
--- /dev/null
+++ b/clang/test/Parser/gh64836.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -xobjective-c++-header %s
+
+template 
+class C {};
+
+class B {
+  p // expected-error {{unknown type name 'p'}}
+ private: // expected-error {{'private' is a keyword in Objective-C++}}
+  void f() {} // expected-error {{expected '(' for function-style cast or type 
construction}}
+  C c; // expected-error {{use of undeclared identifier 'f'}}
+  // expected-error@-1 {{expected member name}}
+};



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


[clang] [ASTImport]improve ast comparation (PR #66110)

2023-09-12 Thread Shafik Yaghmour via cfe-commits


@@ -1295,6 +1306,21 @@ static bool 
IsStructurallyEquivalent(StructuralEquivalenceContext ,
   return true;
 }
 
+static bool IsStructurallyEquivalent(StructuralEquivalenceContext ,

shafik wrote:

Should we also be checking the storage class and the various bits for example 
`CXXForRangeDecl` etc? 

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


[clang] [ASTImport]improve ast comparation (PR #66110)

2023-09-12 Thread Shafik Yaghmour via cfe-commits

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


[clang] [Clang] Fix CXXRewrittenBinaryOperator::getDecomposedForm to handle case when spaceship operator returns comparison category by reference (PR #66270)

2023-09-13 Thread Shafik Yaghmour via cfe-commits

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

Currently CXXRewrittenBinaryOperator::getDecomposedForm(...) may crash if the
spaceship operator returns a comparison category by reference. This because
IgnoreImplicitAsWritten() does not look through CXXConstructExpr. The fix is to
use IgnoreUnlessSpelledInSource() which will look though CXXConstructExpr.

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


>From ed43ed6c693bcc06ffb637972d3938d8d7f9db25 Mon Sep 17 00:00:00 2001
From: Shafik Yaghmour 
Date: Wed, 13 Sep 2023 11:09:28 -0700
Subject: [PATCH] [Clang] Fix CXXRewrittenBinaryOperator::getDecomposedForm to
 handle case when spaceship operator returns comparison category by reference

Currently CXXRewrittenBinaryOperator::getDecomposedForm(...) may crash if the
spaceship operator returns a comparison category by reference. This because
IgnoreImplicitAsWritten() does not look through CXXConstructExpr. The fix is to
use IgnoreUnlessSpelledInSource() which will look though CXXConstructExpr.

This fixes: https://github.com/llvm/llvm-project/issues/64162
---
 clang/lib/AST/ExprCXX.cpp|  2 +-
 clang/test/SemaCXX/compare-cxx2a.cpp | 23 +++
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/clang/lib/AST/ExprCXX.cpp b/clang/lib/AST/ExprCXX.cpp
index efb64842fb6d96d..06163255f9b5e54 100644
--- a/clang/lib/AST/ExprCXX.cpp
+++ b/clang/lib/AST/ExprCXX.cpp
@@ -111,7 +111,7 @@ CXXRewrittenBinaryOperator::getDecomposedForm() const {
 return Result;
 
   // Otherwise, we expect a <=> to now be on the LHS.
-  E = Result.LHS->IgnoreImplicitAsWritten();
+  E = Result.LHS->IgnoreUnlessSpelledInSource();
   if (auto *BO = dyn_cast(E)) {
 assert(BO->getOpcode() == BO_Cmp);
 Result.LHS = BO->getLHS();
diff --git a/clang/test/SemaCXX/compare-cxx2a.cpp 
b/clang/test/SemaCXX/compare-cxx2a.cpp
index 619e16aa7458179..15a0baccfca17a2 100644
--- a/clang/test/SemaCXX/compare-cxx2a.cpp
+++ b/clang/test/SemaCXX/compare-cxx2a.cpp
@@ -479,3 +479,26 @@ void DoSomething() {
   // expected-note  
{{undefined function 'operator++' cannot be used in a constant expression}}
 }
 }
+
+namespace GH64162 {
+struct S {
+const std::strong_ordering& operator<=>(const S&) const = default;
+};
+bool test(S s) {
+return s < s; // We expect this not to crash anymore
+}
+
+// Following example never crashed but worth adding in because it is related
+struct A {};
+bool operator<(A, int);
+
+struct B {
+operator A();
+};
+
+struct C {
+B operator<=>(C);
+};
+
+bool f(C c) { return c < c; }
+}

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


[clang] [Clang] Fix CXXRewrittenBinaryOperator::getDecomposedForm to handle case when spaceship operator returns comparison category by reference (PR #66270)

2023-09-13 Thread Shafik Yaghmour via cfe-commits

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


[clang] [clang] Introduce copy-on-write `CompilerInvocation` (PR #65412)

2023-09-07 Thread Shafik Yaghmour via cfe-commits

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


[clang] [clang] Introduce copy-on-write `CompilerInvocation` (PR #65412)

2023-09-07 Thread Shafik Yaghmour via cfe-commits


@@ -4588,28 +4641,29 @@ std::string CompilerInvocation::getModuleHash() const {
   return toString(llvm::APInt(64, Hash), 36, /*Signed=*/false);
 }
 
-void CompilerInvocation::generateCC1CommandLine(
+void CompilerInvocationBase::generateCC1CommandLine(
 ArgumentConsumer Consumer) const {
-  llvm::Triple T(TargetOpts->Triple);
-
-  GenerateFileSystemArgs(FileSystemOpts, Consumer);
-  GenerateMigratorArgs(MigratorOpts, Consumer);
-  GenerateAnalyzerArgs(*AnalyzerOpts, Consumer);
-  GenerateDiagnosticArgs(*DiagnosticOpts, Consumer, false);
-  GenerateFrontendArgs(FrontendOpts, Consumer, LangOpts->IsHeaderFile);
-  GenerateTargetArgs(*TargetOpts, Consumer);
-  GenerateHeaderSearchArgs(*HeaderSearchOpts, Consumer);
-  GenerateLangArgs(*LangOpts, Consumer, T, FrontendOpts.DashX);
-  GenerateCodeGenArgs(CodeGenOpts, Consumer, T, FrontendOpts.OutputFile,
-  &*LangOpts);
-  GeneratePreprocessorArgs(*PreprocessorOpts, Consumer, *LangOpts, 
FrontendOpts,
-   CodeGenOpts);
-  GeneratePreprocessorOutputArgs(PreprocessorOutputOpts, Consumer,
- FrontendOpts.ProgramAction);
-  GenerateDependencyOutputArgs(DependencyOutputOpts, Consumer);
+  llvm::Triple T(getTargetOpts().Triple);
+
+  GenerateFileSystemArgs(getFileSystemOpts(), Consumer);
+  GenerateMigratorArgs(getMigratorOpts(), Consumer);
+  GenerateAnalyzerArgs(getAnalyzerOpts(), Consumer);
+  GenerateDiagnosticArgs(getDiagnosticOpts(), Consumer, false);

shafik wrote:

```suggestion
  GenerateDiagnosticArgs(getDiagnosticOpts(), Consumer, 
/*DefaultDiagColor=*/false);
```

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


[clang] [clang] Introduce copy-on-write `CompilerInvocation` (PR #65412)

2023-09-07 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik commented:

nit

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


[clang] [sema] Improve -Wsign-compare (PR #65684)

2023-09-08 Thread Shafik Yaghmour via cfe-commits

shafik wrote:

> Yes, there's this way and also when you compare integers smaller than `int`, 
> but those don't trigger -Wsign-compare. -Wsign-compare only triggers if a 
> signed operand is converted to an unsigned operand. This is a PR description 
> bug :)

I did not understand that. I can see why the unsigned case is more problematic 
but I don't know if I like the asymmetry in the diagnostic. The singed case can 
still result in really confusing results.  

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


[clang] [sema] Improve -Wsign-compare (PR #65684)

2023-09-08 Thread Shafik Yaghmour via cfe-commits

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


[clang] [sema] Improve -Wsign-compare (PR #65684)

2023-09-08 Thread Shafik Yaghmour via cfe-commits


@@ -14043,28 +14039,61 @@ static bool CheckTautologicalComparison(Sema , 
BinaryOperator *E,
   return true;
 }
 
+namespace {
+struct AnalyzeImplicitConversionsWorkItem {
+  Expr *E;
+  SourceLocation CC;
+  unsigned IsListInit : 1;
+  unsigned IsTopLevelExpr : 1;
+};
+
+class ImplicitConversionChecker {
+  Sema 
+  Expr *TopLevelExpr;
+
+  void AnalyzeCompoundAssignment(BinaryOperator *E);
+  void AnalyzeImplicitConversions(
+  AnalyzeImplicitConversionsWorkItem Item,
+  llvm::SmallVectorImpl );
+  void AnalyzeImpConvsInComparison(BinaryOperator *E);
+  void AnalyzeComparison(BinaryOperator *E);
+  void AnalyzeAssignment(BinaryOperator *E);
+  void CheckConditionalOperand(Expr *E, QualType T, SourceLocation CC,
+   bool );
+  void CheckConditionalOperator(AbstractConditionalOperator *E,
+SourceLocation CC, QualType T);
+
+public:
+  ImplicitConversionChecker(Sema ) : S(S), TopLevelExpr(nullptr) {}
+
+  void AnalyzeImplicitConversions(Expr *E, SourceLocation CC,
+  bool IsInitList = false,
+  bool IsTopLevelExpr = false);
+};
+} // namespace
+
 /// Analyze the operands of the given comparison.  Implements the
 /// fallback case from AnalyzeComparison.
-static void AnalyzeImpConvsInComparison(Sema , BinaryOperator *E) {
-  AnalyzeImplicitConversions(S, E->getLHS(), E->getOperatorLoc());
-  AnalyzeImplicitConversions(S, E->getRHS(), E->getOperatorLoc());
+void ImplicitConversionChecker::AnalyzeImpConvsInComparison(BinaryOperator *E) 
{
+  AnalyzeImplicitConversions(E->getLHS(), E->getOperatorLoc(), false, false);
+  AnalyzeImplicitConversions(E->getRHS(), E->getOperatorLoc(), false, false);

shafik wrote:

```suggestion
  AnalyzeImplicitConversions(E->getLHS(), E->getOperatorLoc(), 
/*IsInitList=*/false, /*IsTopLevelExpr=*/false);
  AnalyzeImplicitConversions(E->getRHS(), E->getOperatorLoc(), 
/*IsInitList=*/false, /*IsTopLevelExpr=*/false);
```

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


[clang] [sema] Improve -Wsign-compare (PR #65684)

2023-09-08 Thread Shafik Yaghmour via cfe-commits


@@ -14308,22 +14382,22 @@ static bool AnalyzeBitFieldAssignment(Sema , 
FieldDecl *Bitfield, Expr *Init,
 
 /// Analyze the given simple or compound assignment for warning-worthy
 /// operations.
-static void AnalyzeAssignment(Sema , BinaryOperator *E) {
+void ImplicitConversionChecker::AnalyzeAssignment(BinaryOperator *E) {
   // Just recurse on the LHS.
-  AnalyzeImplicitConversions(S, E->getLHS(), E->getOperatorLoc());
+  AnalyzeImplicitConversions(E->getLHS(), E->getOperatorLoc(), false, true);
 
   // We want to recurse on the RHS as normal unless we're assigning to
   // a bitfield.
   if (FieldDecl *Bitfield = E->getLHS()->getSourceBitField()) {
 if (AnalyzeBitFieldAssignment(S, Bitfield, E->getRHS(),
   E->getOperatorLoc())) {
   // Recurse, ignoring any implicit conversions on the RHS.
-  return AnalyzeImplicitConversions(S, E->getRHS()->IgnoreParenImpCasts(),
-E->getOperatorLoc());
+  return AnalyzeImplicitConversions(E->getRHS()->IgnoreParenImpCasts(),
+E->getOperatorLoc(), false, true);

shafik wrote:

```suggestion
E->getOperatorLoc(),  
/*IsInitList=*/false, /*IsTopLevelExpr=*/true);
```

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


[clang] [sema] Improve -Wsign-compare (PR #65684)

2023-09-08 Thread Shafik Yaghmour via cfe-commits


@@ -14135,8 +14164,8 @@ static void AnalyzeComparison(Sema , BinaryOperator 
*E) {
 
   // Go ahead and analyze implicit conversions in the operands.  Note
   // that we skip the implicit conversions on both sides.
-  AnalyzeImplicitConversions(S, LHS, E->getOperatorLoc());
-  AnalyzeImplicitConversions(S, RHS, E->getOperatorLoc());
+  AnalyzeImplicitConversions(LHS, E->getOperatorLoc(), false, false);
+  AnalyzeImplicitConversions(RHS, E->getOperatorLoc(), false, false);

shafik wrote:

```suggestion
  AnalyzeImplicitConversions(LHS, E->getOperatorLoc(), /*IsInitList=*/false, 
/*IsTopLevelExpr=*/false);
  AnalyzeImplicitConversions(RHS, E->getOperatorLoc(), /*IsInitList=*/false, 
/*IsTopLevelExpr=*/false);
```

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


[clang] [sema] Improve -Wsign-compare (PR #65684)

2023-09-08 Thread Shafik Yaghmour via cfe-commits

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


[clang] [sema] Improve -Wsign-compare (PR #65684)

2023-09-08 Thread Shafik Yaghmour via cfe-commits


@@ -14308,22 +14382,22 @@ static bool AnalyzeBitFieldAssignment(Sema , 
FieldDecl *Bitfield, Expr *Init,
 
 /// Analyze the given simple or compound assignment for warning-worthy
 /// operations.
-static void AnalyzeAssignment(Sema , BinaryOperator *E) {
+void ImplicitConversionChecker::AnalyzeAssignment(BinaryOperator *E) {
   // Just recurse on the LHS.
-  AnalyzeImplicitConversions(S, E->getLHS(), E->getOperatorLoc());
+  AnalyzeImplicitConversions(E->getLHS(), E->getOperatorLoc(), false, true);

shafik wrote:

```suggestion
  AnalyzeImplicitConversions(E->getLHS(), E->getOperatorLoc(), 
/*IsInitList=*/false, /*IsTopLevelExpr=*/true);
```

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


[clang] [sema] Improve -Wsign-compare (PR #65684)

2023-09-08 Thread Shafik Yaghmour via cfe-commits


@@ -0,0 +1,129 @@
+// RUN: %clang_cc1 -fsyntax-only -Wsign-compare -Wno-unused-comparison 
-Wno-empty-body -Wno-unused-value -verify %s
+// RUN: cp %s %t
+// RUN: %clang_cc1 -fsyntax-only -Wsign-compare -fixit -x c %t 2> /dev/null
+// RUN: grep -v CHECK %t | FileCheck %s
+
+unsigned Uf(void);
+int Sf(void);
+
+void test(signed S, unsigned U) {

shafik wrote:

I would like to see what you do w/ the `-1L < 1U` case for both `-m32` and not.

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


[clang] [sema] Improve -Wsign-compare (PR #65684)

2023-09-08 Thread Shafik Yaghmour via cfe-commits


@@ -15423,29 +15489,29 @@ static void AnalyzeImplicitConversions(
 // FIXME: Use a more uniform representation for this.
 for (auto *SE : POE->semantics())
   if (auto *OVE = dyn_cast(SE))
-WorkList.push_back({OVE->getSourceExpr(), CC, IsListInit});
+WorkList.push_back({OVE->getSourceExpr(), CC, IsListInit, false});
   }
 
   // Skip past explicit casts.
   if (auto *CE = dyn_cast(E)) {
 E = CE->getSubExpr()->IgnoreParenImpCasts();
 if (!CE->getType()->isVoidType() && E->getType()->isAtomicType())
   S.Diag(E->getBeginLoc(), diag::warn_atomic_implicit_seq_cst);
-WorkList.push_back({E, CC, IsListInit});
+WorkList.push_back({E, CC, IsListInit, false});

shafik wrote:

```suggestion
WorkList.push_back({E, CC, IsListInit, /*IsTopLevelExpr=*/false});
```

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


[clang] [sema] Improve -Wsign-compare (PR #65684)

2023-09-08 Thread Shafik Yaghmour via cfe-commits


@@ -15277,35 +15351,33 @@ static void CheckImplicitConversion(Sema , Expr *E, 
QualType T,
   }
 }
 
-static void CheckConditionalOperator(Sema , AbstractConditionalOperator *E,
- SourceLocation CC, QualType T);
-
-static void CheckConditionalOperand(Sema , Expr *E, QualType T,
-SourceLocation CC, bool ) {
+void ImplicitConversionChecker::CheckConditionalOperand(Expr *E, QualType T,
+SourceLocation CC,
+bool ) {
   E = E->IgnoreParenImpCasts();
   // Diagnose incomplete type for second or third operand in C.
   if (!S.getLangOpts().CPlusPlus && E->getType()->isRecordType())
 S.RequireCompleteExprType(E, diag::err_incomplete_type);
 
   if (auto *CO = dyn_cast(E))
-return CheckConditionalOperator(S, CO, CC, T);
+return CheckConditionalOperator(CO, CC, T);
 
-  AnalyzeImplicitConversions(S, E, CC);
+  AnalyzeImplicitConversions(E, CC, false, true);

shafik wrote:

```suggestion
  AnalyzeImplicitConversions(E, CC,  /*IsInitList=*/false, 
/*IsTopLevelExpr=*/true);
```

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


[clang] [sema] Improve -Wsign-compare (PR #65684)

2023-09-08 Thread Shafik Yaghmour via cfe-commits


@@ -14308,22 +14382,22 @@ static bool AnalyzeBitFieldAssignment(Sema , 
FieldDecl *Bitfield, Expr *Init,
 
 /// Analyze the given simple or compound assignment for warning-worthy
 /// operations.
-static void AnalyzeAssignment(Sema , BinaryOperator *E) {
+void ImplicitConversionChecker::AnalyzeAssignment(BinaryOperator *E) {
   // Just recurse on the LHS.
-  AnalyzeImplicitConversions(S, E->getLHS(), E->getOperatorLoc());
+  AnalyzeImplicitConversions(E->getLHS(), E->getOperatorLoc(), false, true);
 
   // We want to recurse on the RHS as normal unless we're assigning to
   // a bitfield.
   if (FieldDecl *Bitfield = E->getLHS()->getSourceBitField()) {
 if (AnalyzeBitFieldAssignment(S, Bitfield, E->getRHS(),
   E->getOperatorLoc())) {
   // Recurse, ignoring any implicit conversions on the RHS.
-  return AnalyzeImplicitConversions(S, E->getRHS()->IgnoreParenImpCasts(),
-E->getOperatorLoc());
+  return AnalyzeImplicitConversions(E->getRHS()->IgnoreParenImpCasts(),
+E->getOperatorLoc(), false, true);
 }
   }
 
-  AnalyzeImplicitConversions(S, E->getRHS(), E->getOperatorLoc());
+  AnalyzeImplicitConversions(E->getRHS(), E->getOperatorLoc(), false, true);

shafik wrote:

```suggestion
  AnalyzeImplicitConversions(E->getRHS(), E->getOperatorLoc(),  
/*IsInitList=*/false, /*IsTopLevelExpr=*/true);
```

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


[clang] [sema] Improve -Wsign-compare (PR #65684)

2023-09-08 Thread Shafik Yaghmour via cfe-commits


@@ -14490,12 +14564,12 @@ static void DiagnoseFloatingImpCast(Sema , Expr *E, 
QualType T,
 
 /// Analyze the given compound assignment for the possible losing of
 /// floating-point precision.
-static void AnalyzeCompoundAssignment(Sema , BinaryOperator *E) {
+void ImplicitConversionChecker::AnalyzeCompoundAssignment(BinaryOperator *E) {
   assert(isa(E) &&
  "Must be compound assignment operation");
   // Recurse on the LHS and RHS in here
-  AnalyzeImplicitConversions(S, E->getLHS(), E->getOperatorLoc());
-  AnalyzeImplicitConversions(S, E->getRHS(), E->getOperatorLoc());
+  AnalyzeImplicitConversions(E->getLHS(), E->getOperatorLoc(), false, true);
+  AnalyzeImplicitConversions(E->getRHS(), E->getOperatorLoc(), false, true);

shafik wrote:

```suggestion
  AnalyzeImplicitConversions(E->getLHS(), E->getOperatorLoc(),  
/*IsInitList=*/false, /*IsTopLevelExpr=*/true);
  AnalyzeImplicitConversions(E->getRHS(), E->getOperatorLoc(),  
/*IsInitList=*/false, /*IsTopLevelExpr=*/true);
```

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


[clang] [sema] Improve -Wsign-compare (PR #65684)

2023-09-08 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik commented:

Looks good overall, just the one side case I mentioned should be tested and 
mostly small nits.

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


[clang] [sema] Improve -Wsign-compare (PR #65684)

2023-09-08 Thread Shafik Yaghmour via cfe-commits


@@ -15423,29 +15489,29 @@ static void AnalyzeImplicitConversions(
 // FIXME: Use a more uniform representation for this.
 for (auto *SE : POE->semantics())
   if (auto *OVE = dyn_cast(SE))
-WorkList.push_back({OVE->getSourceExpr(), CC, IsListInit});
+WorkList.push_back({OVE->getSourceExpr(), CC, IsListInit, false});

shafik wrote:

```suggestion
WorkList.push_back({OVE->getSourceExpr(), CC, IsListInit, 
/*IsTopLevelExpr=*/false});
```

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


[clang] [sema] Improve -Wsign-compare (PR #65684)

2023-09-08 Thread Shafik Yaghmour via cfe-commits


@@ -15277,35 +15351,33 @@ static void CheckImplicitConversion(Sema , Expr *E, 
QualType T,
   }
 }
 
-static void CheckConditionalOperator(Sema , AbstractConditionalOperator *E,
- SourceLocation CC, QualType T);
-
-static void CheckConditionalOperand(Sema , Expr *E, QualType T,
-SourceLocation CC, bool ) {
+void ImplicitConversionChecker::CheckConditionalOperand(Expr *E, QualType T,
+SourceLocation CC,
+bool ) {
   E = E->IgnoreParenImpCasts();
   // Diagnose incomplete type for second or third operand in C.
   if (!S.getLangOpts().CPlusPlus && E->getType()->isRecordType())
 S.RequireCompleteExprType(E, diag::err_incomplete_type);
 
   if (auto *CO = dyn_cast(E))
-return CheckConditionalOperator(S, CO, CC, T);
+return CheckConditionalOperator(CO, CC, T);
 
-  AnalyzeImplicitConversions(S, E, CC);
+  AnalyzeImplicitConversions(E, CC, false, true);
   if (E->getType() != T)
 return CheckImplicitConversion(S, E, T, CC, );
 }
 
-static void CheckConditionalOperator(Sema , AbstractConditionalOperator *E,
- SourceLocation CC, QualType T) {
-  AnalyzeImplicitConversions(S, E->getCond(), E->getQuestionLoc());
+void ImplicitConversionChecker::CheckConditionalOperator(
+AbstractConditionalOperator *E, SourceLocation CC, QualType T) {
+  AnalyzeImplicitConversions(E->getCond(), E->getQuestionLoc(), false, true);

shafik wrote:

```suggestion
  AnalyzeImplicitConversions(E->getCond(), E->getQuestionLoc(),  
/*IsInitList=*/false, /*IsTopLevelExpr=*/true);
```

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


[clang] [sema] Improve -Wsign-compare (PR #65684)

2023-09-08 Thread Shafik Yaghmour via cfe-commits


@@ -15478,7 +15544,7 @@ static void AnalyzeImplicitConversions(
   // Ignore checking string literals that are in logical and operators.
   // This is a common pattern for asserts.
   continue;
-WorkList.push_back({ChildExpr, CC, IsListInit});
+WorkList.push_back({ChildExpr, CC, IsListInit, false});

shafik wrote:

```suggestion
WorkList.push_back({ChildExpr, CC, IsListInit, /*IsTopLevelExpr=*/false});
```

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


[clang] [sema] Improve -Wsign-compare (PR #65684)

2023-09-08 Thread Shafik Yaghmour via cfe-commits

shafik wrote:

> form when the user doesn't know what happens when integers of different signs 
> are compared (the signed one is cast to an unsigned value).

I just wanted to point out that although this is very much an edge case, but 
mixed sign comparison can result in conversion to signed type. See the last 
example in the "The Rules" section. 

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


[clang] [AST] Add dump() method to TypeLoc (PR #65484)

2023-09-07 Thread Shafik Yaghmour via cfe-commits


@@ -96,6 +96,21 @@ void JSONNodeDumper::Visit(QualType T) {
   JOS.attribute("qualifiers", T.split().Quals.getAsString());
 }
 
+void JSONNodeDumper::Visit(TypeLoc TL) {
+  if (TL.isNull())
+return;
+  JOS.attribute("kind",
+(llvm::Twine(TL.getTypeLocClass() == TypeLoc::Qualified
+ ? "Qualified"
+ : TL.getTypePtr()->getTypeClassName()) +
+ "TypeLoc")
+.str());
+  JOS.attribute("type",
+createQualType(QualType(TL.getType()), /*Desugar*/ false));

shafik wrote:

```suggestion
createQualType(QualType(TL.getType()), /*Desugar=*/false));
```

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


[clang] [clang][VarDecl] Reset un-evaluated constant for all C++ modes (PR #65818)

2023-09-08 Thread Shafik Yaghmour via cfe-commits

shafik wrote:

LGTM, thank you for the quick fix!

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


[clang] 6f30ef3 - [Clang] Modify Parser::ParseLambdaExpressionAfterIntroducer to check whether the lambda-declarator is valid

2023-08-29 Thread Shafik Yaghmour via cfe-commits

Author: Shafik Yaghmour
Date: 2023-08-29T11:28:57-07:00
New Revision: 6f30ef360127ffb3346fd3d3e60a229ed44dc667

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

LOG: [Clang] Modify Parser::ParseLambdaExpressionAfterIntroducer to check 
whether the lambda-declarator is valid

We had a couple of crashes due to invalid lambda trailing return types that
were diagnosed but not treated as errors during parsing. So now in
Parser::ParseLambdaExpressionAfterIntroducer(...) after 
ActOnStartOfLambdaDefinition(...)
we also check if the lambda-declarator is invalid and if so we end up in 
ActOnLambdaError(...).

Fixes: https://github.com/llvm/llvm-project/issues/64962
https://github.com/llvm/llvm-project/issues/28679

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/Parse/ParseExprCXX.cpp
clang/test/Parser/cxx2a-template-lambdas.cpp
clang/test/SemaCXX/subst-func-type-invalid-ret-type.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 6f97acc09bc934..dc7569cf79f43e 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -234,6 +234,10 @@ Bug Fixes to C++ Support
   and appear in an implicit cast.
   (`#64949 `_).
 
+- Fix crash when parsing ill-formed lambda trailing return type. Fixes:
+  (`#64962 `_) and
+  (`#28679 `_).
+
 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 678ecad19ecd82..d2715db95f5285 100644
--- a/clang/lib/Parse/ParseExprCXX.cpp
+++ b/clang/lib/Parse/ParseExprCXX.cpp
@@ -1546,7 +1546,8 @@ ExprResult Parser::ParseLambdaExpressionAfterIntroducer(
   TemplateParamScope.Exit();
   LambdaScope.Exit();
 
-  if (!Stmt.isInvalid() && !TrailingReturnType.isInvalid())
+  if (!Stmt.isInvalid() && !TrailingReturnType.isInvalid() &&
+  !D.isInvalidType())
 return Actions.ActOnLambdaExpr(LambdaBeginLoc, Stmt.get(), getCurScope());
 
   Actions.ActOnLambdaError(LambdaBeginLoc, getCurScope());

diff  --git a/clang/test/Parser/cxx2a-template-lambdas.cpp 
b/clang/test/Parser/cxx2a-template-lambdas.cpp
index 5cf1a862d878cb..98c74a247b535f 100644
--- a/clang/test/Parser/cxx2a-template-lambdas.cpp
+++ b/clang/test/Parser/cxx2a-template-lambdas.cpp
@@ -32,3 +32,11 @@ auto XL1 = [] requires true noexcept requires true {}; 
// expected-error {
 // expected-warning@-3 {{is a C++23 extension}}
 // expected-warning@-3 {{is a C++23 extension}}
 #endif
+
+namespace GH64962 {
+void f() {
+  [] (T i) -> int[] // expected-error {{function cannot return 
array type 'int[]'}}
+// extension-warning {{explicit template 
parameter list for lambdas is a C++20 extension}}
+{ return 3; } (v); // expected-error {{use of undeclared identifier 'v'}}
+}
+}

diff  --git a/clang/test/SemaCXX/subst-func-type-invalid-ret-type.cpp 
b/clang/test/SemaCXX/subst-func-type-invalid-ret-type.cpp
index c783ff50ee..565e06b101a7b2 100644
--- a/clang/test/SemaCXX/subst-func-type-invalid-ret-type.cpp
+++ b/clang/test/SemaCXX/subst-func-type-invalid-ret-type.cpp
@@ -5,12 +5,11 @@
 template T declval();
 
 template 
-auto Call(T x) -> decltype(declval()(0)) {} // expected-note{{candidate 
template ignored}}
+auto Call(T x) -> decltype(declval()(0)) {}
 
 class Status {};
 
 void fun() {
   // The Status() (instead of Status) here used to cause a crash.
   Call([](auto x) -> Status() {}); // expected-error{{function cannot return 
function type 'Status ()}}
-  // expected-error@-1{{no matching function for call to 'Call'}}
 }



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


[clang] [clang][doc] Add documentation for the ASTs used to represent C++ templates (PR #66436)

2023-09-14 Thread Shafik Yaghmour via cfe-commits

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


[clang] [clang][doc] Add documentation for the ASTs used to represent C++ templates (PR #66436)

2023-09-14 Thread Shafik Yaghmour via cfe-commits

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


[clang] [clang][doc] Add documentation for the ASTs used to represent C++ templates (PR #66436)

2023-09-14 Thread Shafik Yaghmour via cfe-commits

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


[clang] [Clang] Fix CXXRewrittenBinaryOperator::getDecomposedForm to handle case when spaceship operator returns comparison category by reference (PR #66270)

2023-09-13 Thread Shafik Yaghmour via cfe-commits

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


[clang] [Clang] Mark declarators invalid in the presence of ill-formed explicit parameters. (PR #70018)

2023-10-26 Thread Shafik Yaghmour via cfe-commits


@@ -542,3 +542,46 @@ void foo(C c) {
 }
 
 }
+
+
+namespace GH69838 {
+struct S {
+  S(this auto ) {} // expected-error {{an explicit object parameter 
cannot appear in a constructor}}
+  virtual void f(this S self) {} // expected-error {{an explicit object 
parameter cannot appear in a virtual function}}
+  void g(this auto ) const {} // expected-error {{explicit object member 
function cannot have 'const' qualifier}}
+  void h(this S self = S{}) {} // expected-error {{the explicit object 
parameter cannot have a default argument}}
+  void i(int i, this S self = S{}) {} // expected-error {{an explicit object 
parameter can only appear as the first parameter of the function}}
+  ~S(this S &); // expected-error {{an explicit object parameter cannot 
appear in a destructor}} \
+ // expected-error {{destructor cannot have any 
parameters}}
+
+  static void j(this S s); // expected-error {{an explicit object parameter 
cannot appear in a static function}}
+};
+
+void nonmember(this S s); // expected-error {{an explicit object parameter 
cannot appear in a non-member function}}

shafik wrote:

You could add `static` here for extra fun.

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


[clang] [Clang] Ensure zero-init is not overridden when initializing a base class in a constant expression context (PR #70150)

2023-10-25 Thread Shafik Yaghmour via cfe-commits

shafik wrote:

The clang-format error is a false positive.

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


[clang-tools-extra] [Clang] Ensure zero-init is not overridden when initializing a base class in a constant expression context (PR #70150)

2023-10-25 Thread Shafik Yaghmour via cfe-commits

shafik wrote:

The clang-format error is a false positive.

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


[clang] [Clang] Ensure zero-init is not overridden when initializing a base class in a constant expression context (PR #70150)

2023-10-25 Thread Shafik Yaghmour via cfe-commits

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


[clang] [clang] Enable Wenum-constexpr-conversion also in system headers and … (PR #67528)

2023-10-25 Thread Shafik Yaghmour via cfe-commits
Carlos =?utf-8?q?G=C3=A1lvez?= 
Message-ID:
In-Reply-To: 


shafik wrote:

Thank you for doing this work.

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


[clang] [Clang] Fix dependence handling of nttp for variable templates (PR #69075)

2023-10-25 Thread Shafik Yaghmour via cfe-commits
=?utf-8?b?5YiY6Zuo5Z+5?= 
Message-ID:
In-Reply-To: 



@@ -1299,8 +1299,9 @@ static bool checkTupleLikeDecomposition(Sema ,
   //   in the associated namespaces.
   Expr *Get = UnresolvedLookupExpr::Create(
   S.Context, nullptr, NestedNameSpecifierLoc(), SourceLocation(),
-  DeclarationNameInfo(GetDN, Loc), /*RequiresADL*/true, ,
-  UnresolvedSetIterator(), UnresolvedSetIterator());
+  DeclarationNameInfo(GetDN, Loc), /*RequiresADL*/ true, ,

shafik wrote:

`/*RequiresADL=*/true`

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


[clang] [Clang] Fix dependence handling of nttp for variable templates (PR #69075)

2023-10-25 Thread Shafik Yaghmour via cfe-commits
=?utf-8?b?5YiY6Zuo5Z+5?= 
Message-ID:
In-Reply-To: 


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


[clang] [Clang] Fix dependence handling of nttp for variable templates (PR #69075)

2023-10-25 Thread Shafik Yaghmour via cfe-commits
=?utf-8?b?5YiY6Zuo5Z+5?= 
Message-ID:
In-Reply-To: 


https://github.com/shafik commented:

A couple of nitpicks

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


[clang] [Clang] Fix dependence handling of nttp for variable templates (PR #69075)

2023-10-25 Thread Shafik Yaghmour via cfe-commits
=?utf-8?b?5YiY6Zuo5Z+5?= 
Message-ID:
In-Reply-To: 



@@ -354,10 +354,10 @@ UnresolvedLookupExpr::UnresolvedLookupExpr(
 NestedNameSpecifierLoc QualifierLoc, SourceLocation TemplateKWLoc,
 const DeclarationNameInfo , bool RequiresADL, bool Overloaded,
 const TemplateArgumentListInfo *TemplateArgs, UnresolvedSetIterator Begin,
-UnresolvedSetIterator End)
+UnresolvedSetIterator End, bool KnownDependent)
 : OverloadExpr(UnresolvedLookupExprClass, Context, QualifierLoc,
-   TemplateKWLoc, NameInfo, TemplateArgs, Begin, End, false,
-   false, false),
+   TemplateKWLoc, NameInfo, TemplateArgs, Begin, End,
+   KnownDependent, false, false),

shafik wrote:

`/*KnownInstantiationDependent=*/false, 
/*KnownContainsUnexpandedParameterPack=*/false` 

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/69075
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2023-10-25 Thread Shafik Yaghmour via cfe-commits


@@ -7688,7 +7688,7 @@ bool Sema::CheckNonDependentConversions(
 QualType ParamType = ParamTypes[I + Offset];
 if (!ParamType->isDependentType()) {
   unsigned ConvIdx = PO == OverloadCandidateParamOrder::Reversed
- ? 0
+ ? Args.size() - 1 - (ThisConversions + I)

shafik wrote:

I think this warrants a comment as to why this computation is correct. It is 
certainly not obvious and if anyone ever needs to update this code it would be 
helpful.

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


[clang-tools-extra] [Clang] Ensure zero-init is not overridden when initializing a base class in a constant expression context (PR #70150)

2023-10-25 Thread Shafik Yaghmour via cfe-commits

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


[clang] Remove warnings from -Wchar-subscripts for known positive constants (PR #69061)

2023-10-25 Thread Shafik Yaghmour via cfe-commits

https://github.com/shafik commented:

You need to make sure you tests have newlines at the end

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


[clang] Remove warnings from -Wchar-subscripts for known positive constants (PR #69061)

2023-10-25 Thread Shafik Yaghmour via cfe-commits


@@ -0,0 +1,103 @@
+// RUN: %clang_cc1 -Wchar-subscripts -fsyntax-only -verify %s
+
+void t1(void) {
+  int array[1] = { 0 };
+  char subscript = 0;
+  int val = array[subscript]; // expected-warning{{array subscript is of type 
'char'}}
+}
+
+void t2(void) {
+  int array[1] = { 0 };
+  char subscript = 0;
+  int val = subscript[array]; // expected-warning{{array subscript is of type 
'char'}}
+}
+
+void t3(void) {
+  int *array = 0;
+  char subscript = 0;
+  int val = array[subscript]; // expected-warning{{array subscript is of type 
'char'}}
+}
+
+void t4(void) {
+  int *array = 0;
+  char subscript = 0;
+  int val = subscript[array]; // expected-warning{{array subscript is of type 
'char'}}
+}
+
+char returnsChar(void);
+void t5(void) {
+  int *array = 0;
+  int val = array[returnsChar()]; // expected-warning{{array subscript is of 
type 'char'}}
+}
+
+void t6(void) {
+  int array[1] = { 0 };
+  signed char subscript = 0;
+  int val = array[subscript]; // no warning for explicit signed char

shafik wrote:

Why do we exclude the `signed char` case?

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


[clang] Remove warnings from -Wchar-subscripts for known positive constants (PR #69061)

2023-10-25 Thread Shafik Yaghmour via cfe-commits

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


[clang] Fix crash with modules and constexpr destructor (PR #69076)

2023-10-25 Thread Shafik Yaghmour via cfe-commits


@@ -15604,10 +15604,13 @@ bool Expr::EvaluateAsInitializer(APValue , 
const ASTContext ,
 LValue LVal;
 LVal.set(VD);
 
-if (!EvaluateInPlace(Value, Info, LVal, this,
- /*AllowNonLiteralTypes=*/true) ||
-EStatus.HasSideEffects)
-  return false;
+{
+  FullExpressionRAII Scope(Info);

shafik wrote:

I think we need to add a comment here similar to the one in 
`EvaluateAsConstexpr(...)` in this case it would be `A full-expression is ... 
an init-declarator ([dcl.decl]) or a mem-initializer` see 
https://eel.is/c++draft/intro.execution#5.4

CC @cor3ntin for second look

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


[clang] Fix crash with modules and constexpr destructor (PR #69076)

2023-10-25 Thread Shafik Yaghmour via cfe-commits

shafik wrote:

> While the change itself looks neat, I am curious about the reason how this 
> interact with modules.

IIUC modules is incidental to the problem, it is just a way we run into an 
underlying issue that we are not dealing with full-expressions properly in this 
case.

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


[clang] [clang] Bail out if the result of function template instantiation is not a function type. (PR #69459)

2023-10-25 Thread Shafik Yaghmour via cfe-commits

shafik wrote:

Please next time add a detailed description to your PR. This is key for 
reviewers to fully understand at a glance what the fix is for.

This is also key for users of git log to be able to understand changes at a 
glance without having to dig in. Folks often use git log to track down recent 
changes to explain changes in behavior.

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


[clang] Fix crash with modules and constexpr destructor (PR #69076)

2023-10-25 Thread Shafik Yaghmour via cfe-commits

shafik wrote:

I think this change makes sense but I am not crazy about how we deal w/ 
full-expressions right now with these `FullExpressionRAII`, it feels very 
ad-hoc and it takes a bunch of time to understand why they are needed where 
they are.

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


[clang] [clang] Bail out if the result of function template instantiation is not a function type. (PR #69459)

2023-10-25 Thread Shafik Yaghmour via cfe-commits


@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -x c++ -std=c++14 -fsyntax-only -verify %s
+
+template 
+using Type = typename A::NestedType; // expected-error {{type 'float' cannot 
be used prior to '::' because it has no members}}
+
+template 
+void Func() {
+  using MyType = Type(); // expected-note {{in instantiation of template 
type alias 'Type' requested here}}
+  // This is a function declaration, not a variable declaration!
+  // After substitution, we do not have a valid function type, and used to 
crash.
+  MyType var;
+}
+
+void Test() {
+  Func(); // expected-note {{in instantiation of function template 
specialization 'Func' requested here}}
+}

shafik wrote:

Files should have newlines at the end.

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


[clang] Fix crash with modules and constexpr destructor (PR #69076)

2023-10-25 Thread Shafik Yaghmour via cfe-commits

shafik wrote:

Please make sure you write a more complete description of the problem and why 
this solves the problem. The description is usually what goes into the git log 
and that is very useful for quickly understanding commits and tracking down 
problems. 

I know some folks edit the description on commit but honestly it is very 
helpful for the reviewers of your PR to have a full description up front.

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


[clang] [Clang][Sema] Skip RecordDecl when checking scope of declarations (PR #69432)

2023-10-25 Thread Shafik Yaghmour via cfe-commits


@@ -19,4 +19,8 @@ void f2(void) {
   struct U u;
 }
 
-
+void f3(void) {

shafik wrote:

So unlike the original test case this one does not seem to cause a crash. Can 
we add a test case for that as well to verify we are fixing that issue as well.

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


[clang] [clang] Language to String function (PR #69487)

2023-10-25 Thread Shafik Yaghmour via cfe-commits

shafik wrote:

What is the intended use for this?

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


[clang] [Clang] Diagnose defaulted assignment operator with incompatible object parameter (PR #70176)

2023-10-27 Thread Shafik Yaghmour via cfe-commits


@@ -585,3 +585,34 @@ class Server : public Thing {
 S name_;
 };
 }
+
+namespace GH69233 {
+struct Base {};
+struct S : Base {
+int j;
+S& operator=(this Base& self, const S&) = default;
+// expected-warning@-1 {{explicitly defaulted copy assignment operator is 
implicitly deleted}}
+// expected-note@-2 {{function is implicitly deleted because its declared 
type does not match the type of an implicit copy assignment operator}}
+// expected-note@-3 {{explicitly defaulted function was implicitly deleted 
here}}
+};
+
+struct S2 {
+S2& operator=(this int&& self, const S2&);
+S2& operator=(this int&& self, S2&&);
+operator int();
+};
+
+S2& S2::operator=(this int&& self, const S2&) = default;
+// expected-error@-1 {{the type of the explicit object parameter of an 
explicitly-defaulted copy assignment operator should match the type of the 
class 'S2'}}
+
+S2& S2::operator=(this int&& self, S2&&) = default;
+// expected-error@-1 {{the type of the explicit object parameter of an 
explicitly-defaulted move assignment operator should match the type of the 
class 'S2'}}
+
+void test() {
+S s;
+s = s; // expected-error {{object of type 'S' cannot be assigned because 
its copy assignment operator is implicitly deleted}}

shafik wrote:

Can we get an example where move assignment fails as well.

https://github.com/llvm/llvm-project/pull/70176
___
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)

2023-10-27 Thread Shafik Yaghmour via cfe-commits


@@ -11773,7 +11786,25 @@ QualType Sema::CheckShiftOperands(ExprResult , 
ExprResult ,
   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)

2023-10-27 Thread 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] [Clang] Defer the instantiation of explicit-specifier until constraint checking completes (PR #70548)

2023-10-31 Thread Shafik Yaghmour via cfe-commits


@@ -3553,6 +3553,48 @@ static unsigned getPackIndexForParam(Sema ,
   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 , FunctionDecl *Specialization,
+const MultiLevelTemplateArgumentList ,
+TemplateDeductionInfo , 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)

2023-10-31 Thread 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)

2023-10-31 Thread 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] [Clang] Defer the instantiation of explicit-specifier until constraint checking completes (PR #70548)

2023-10-30 Thread Shafik Yaghmour via cfe-commits


@@ -3553,6 +3553,49 @@ static unsigned getPackIndexForParam(Sema ,
   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 , FunctionDecl *Specialization,
+ const MultiLevelTemplateArgumentList ,
+ TemplateDeductionInfo ,
+ 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)

2023-10-30 Thread 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] [CodeGen] Revamp counted_by calculations (PR #70606)

2023-11-02 Thread Shafik Yaghmour via cfe-commits


@@ -966,9 +962,65 @@ static llvm::Value *getArrayIndexingBound(CodeGenFunction 
,
   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 ,
+  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)

2023-11-02 Thread Shafik Yaghmour via cfe-commits


@@ -966,9 +962,65 @@ static llvm::Value *getArrayIndexingBound(CodeGenFunction 
,
   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 ,
+  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)

2023-11-02 Thread 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


  1   2   3   4   5   6   7   >