[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc

[Kristof Beyls via Phabricator via cfe-commits]

kristof.beyls added inline comments.



Comment at: llvm/test/CodeGen/ARM/gnu_mcount_nc.ll:1-6
+; RUN: llc -mtriple=armv7a-linux-gnueabihf %s -o - | FileCheck %s 
--check-prefix=CHECK-ARM
+; RUN: llc -mtriple=armv7a-linux-gnueabihf %s -o - | FileCheck %s 
--check-prefix=CHECK-ARM-FAST-ISEL
+; RUN: llc -mtriple=armv7a-linux-gnueabihf %s -o - | FileCheck %s 
--check-prefix=CHECK-ARM-GLOBAL-ISEL
+; RUN: llc -mtriple=thumbv7a-linux-gnueabihf %s -o - | FileCheck %s 
--check-prefix=CHECK-THUMB
+; RUN: llc -mtriple=thumbv7a-linux-gnueabihf %s -o - | FileCheck %s 
--check-prefix=CHECK-THUMB-FAST-ISEL
+; RUN: llc -mtriple=thumbv7a-linux-gnueabihf %s -o - | FileCheck %s 
--check-prefix=CHECK-THUMB-GLOBAL-ISEL

It seems the -fast-isel/-global-isel command line options are missing in the 
RUN lines aiming to test fast and global isel do the right thing?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65019



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

[PATCH] D66054: [CrossTU] Fix problem with CrossTU AST load limit and progress messages.

[Balázs Kéri via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL368545: [CrossTU] Fix problem with CrossTU AST load limit 
and progress messages. (authored by balazske, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66054?vs=214532&id=214581#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66054

Files:
  cfe/trunk/include/clang/CrossTU/CrossTranslationUnit.h
  cfe/trunk/lib/CrossTU/CrossTranslationUnit.cpp

Index: cfe/trunk/include/clang/CrossTU/CrossTranslationUnit.h
===
--- cfe/trunk/include/clang/CrossTU/CrossTranslationUnit.h
+++ cfe/trunk/include/clang/CrossTU/CrossTranslationUnit.h
@@ -219,8 +219,27 @@
 const CompilerInstance &CI;
   };
 
-  /// Storage for ASTUnits, cached access, and providing searchability are the
-  /// concerns of ASTUnitStorage class.
+  /// Maintain number of AST loads and check for reaching the load limit.
+  class ASTLoadGuard {
+  public:
+ASTLoadGuard(unsigned Limit) : Limit(Limit) {}
+
+/// Indicates, whether a new load operation is permitted, it is within the
+/// threshold.
+operator bool() const { return Count < Limit; }
+
+/// Tell that a new AST was loaded successfully.
+void indicateLoadSuccess() { ++Count; }
+
+  private:
+/// The number of ASTs actually imported.
+unsigned Count{0u};
+/// The limit (threshold) value for number of loaded ASTs.
+const unsigned Limit;
+  };
+
+  /// Storage and load of ASTUnits, cached access, and providing searchability
+  /// are the concerns of ASTUnitStorage class.
   class ASTUnitStorage {
   public:
 ASTUnitStorage(const CompilerInstance &CI);
@@ -231,12 +250,14 @@
 /// \param IndexName Name of the file inside \p CrossTUDir which maps
 /// function USR names to file paths. These files contain the corresponding
 /// AST-dumps.
+/// \param DisplayCTUProgress Display a message about loading new ASTs.
 ///
 /// \return An Expected instance which contains the ASTUnit pointer or the
 /// error occured during the load.
 llvm::Expected getASTUnitForFunction(StringRef FunctionName,
 StringRef CrossTUDir,
-StringRef IndexName);
+StringRef IndexName,
+bool DisplayCTUProgress);
 /// Identifies the path of the file which can be used to load the ASTUnit
 /// for a given function.
 ///
@@ -253,7 +274,8 @@
 
   private:
 llvm::Error ensureCTUIndexLoaded(StringRef CrossTUDir, StringRef IndexName);
-llvm::Expected getASTUnitForFile(StringRef FileName);
+llvm::Expected getASTUnitForFile(StringRef FileName,
+bool DisplayCTUProgress);
 
 template  using BaseMapTy = llvm::StringMap;
 using OwningMapTy = BaseMapTy>;
@@ -266,48 +288,17 @@
 IndexMapTy NameFileMap;
 
 ASTFileLoader FileAccessor;
+
+/// Limit the number of loaded ASTs. Used to limit the  memory usage of the
+/// CrossTranslationUnitContext.
+/// The ASTUnitStorage has the knowledge about if the AST to load is
+/// actually loaded or returned from cache. This information is needed to
+/// maintain the counter.
+ASTLoadGuard LoadGuard;
   };
 
   ASTUnitStorage ASTStorage;
 
-  /// \p CTULoadTreshold should serve as an upper limit to the number of TUs
-  /// imported in order to reduce the memory footprint of CTU analysis.
-  const unsigned CTULoadThreshold;
-
-  /// The number successfully loaded ASTs. Used to indicate, and  - with the
-  /// appropriate threshold value - limit the  memory usage of the
-  /// CrossTranslationUnitContext.
-  unsigned NumASTLoaded{0u};
-
-  /// RAII counter to signal 'threshold reached' condition, and to increment the
-  /// NumASTLoaded counter upon a successful load.
-  class LoadGuard {
-  public:
-LoadGuard(unsigned Limit, unsigned &Counter)
-: Counter(Counter), Enabled(Counter < Limit), StoreSuccess(false) {}
-~LoadGuard() {
-  if (StoreSuccess)
-++Counter;
-}
-/// Flag the LoadGuard instance as successful, meaning that the load
-/// operation succeeded, and the memory footprint of the AST storage
-/// actually increased. In this case, \p Counter should be incremented upon
-/// destruction.
-void storedSuccessfully() { StoreSuccess = true; }
-/// Indicates, whether a new load operation is permitted, it is within the
-/// threshold.
-operator bool() const { return Enabled; }
-
-  private:
-/// The number of ASTs actually imported. LoadGuard does not own the
-/// counter, just uses on given to it at construction time.
-unsigned &Coun

r368545 - [CrossTU] Fix problem with CrossTU AST load limit and progress messages.

[Balazs Keri via cfe-commits]

Author: balazske
Date: Mon Aug 12 00:15:29 2019
New Revision: 368545

URL: http://llvm.org/viewvc/llvm-project?rev=368545&view=rev
Log:
[CrossTU] Fix problem with CrossTU AST load limit and progress messages.

Summary:
Number of loaded ASTs is to be incremented only if the AST was really loaded
but not if it was returned from cache. At the same place the message about
a loaded AST is displayed.

Reviewers: martong, gamesh411

Reviewed By: martong

Subscribers: rnkovacs, dkrupp, Szelethus, gamesh411, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/include/clang/CrossTU/CrossTranslationUnit.h
cfe/trunk/lib/CrossTU/CrossTranslationUnit.cpp

Modified: cfe/trunk/include/clang/CrossTU/CrossTranslationUnit.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/CrossTU/CrossTranslationUnit.h?rev=368545&r1=368544&r2=368545&view=diff
==
--- cfe/trunk/include/clang/CrossTU/CrossTranslationUnit.h (original)
+++ cfe/trunk/include/clang/CrossTU/CrossTranslationUnit.h Mon Aug 12 00:15:29 
2019
@@ -219,8 +219,27 @@ private:
 const CompilerInstance &CI;
   };
 
-  /// Storage for ASTUnits, cached access, and providing searchability are the
-  /// concerns of ASTUnitStorage class.
+  /// Maintain number of AST loads and check for reaching the load limit.
+  class ASTLoadGuard {
+  public:
+ASTLoadGuard(unsigned Limit) : Limit(Limit) {}
+
+/// Indicates, whether a new load operation is permitted, it is within the
+/// threshold.
+operator bool() const { return Count < Limit; }
+
+/// Tell that a new AST was loaded successfully.
+void indicateLoadSuccess() { ++Count; }
+
+  private:
+/// The number of ASTs actually imported.
+unsigned Count{0u};
+/// The limit (threshold) value for number of loaded ASTs.
+const unsigned Limit;
+  };
+
+  /// Storage and load of ASTUnits, cached access, and providing searchability
+  /// are the concerns of ASTUnitStorage class.
   class ASTUnitStorage {
   public:
 ASTUnitStorage(const CompilerInstance &CI);
@@ -231,12 +250,14 @@ private:
 /// \param IndexName Name of the file inside \p CrossTUDir which maps
 /// function USR names to file paths. These files contain the corresponding
 /// AST-dumps.
+/// \param DisplayCTUProgress Display a message about loading new ASTs.
 ///
 /// \return An Expected instance which contains the ASTUnit pointer or the
 /// error occured during the load.
 llvm::Expected getASTUnitForFunction(StringRef FunctionName,
 StringRef CrossTUDir,
-StringRef IndexName);
+StringRef IndexName,
+bool DisplayCTUProgress);
 /// Identifies the path of the file which can be used to load the ASTUnit
 /// for a given function.
 ///
@@ -253,7 +274,8 @@ private:
 
   private:
 llvm::Error ensureCTUIndexLoaded(StringRef CrossTUDir, StringRef 
IndexName);
-llvm::Expected getASTUnitForFile(StringRef FileName);
+llvm::Expected getASTUnitForFile(StringRef FileName,
+bool DisplayCTUProgress);
 
 template  using BaseMapTy = llvm::StringMap;
 using OwningMapTy = BaseMapTy>;
@@ -266,48 +288,17 @@ private:
 IndexMapTy NameFileMap;
 
 ASTFileLoader FileAccessor;
+
+/// Limit the number of loaded ASTs. Used to limit the  memory usage of the
+/// CrossTranslationUnitContext.
+/// The ASTUnitStorage has the knowledge about if the AST to load is
+/// actually loaded or returned from cache. This information is needed to
+/// maintain the counter.
+ASTLoadGuard LoadGuard;
   };
 
   ASTUnitStorage ASTStorage;
 
-  /// \p CTULoadTreshold should serve as an upper limit to the number of TUs
-  /// imported in order to reduce the memory footprint of CTU analysis.
-  const unsigned CTULoadThreshold;
-
-  /// The number successfully loaded ASTs. Used to indicate, and  - with the
-  /// appropriate threshold value - limit the  memory usage of the
-  /// CrossTranslationUnitContext.
-  unsigned NumASTLoaded{0u};
-
-  /// RAII counter to signal 'threshold reached' condition, and to increment 
the
-  /// NumASTLoaded counter upon a successful load.
-  class LoadGuard {
-  public:
-LoadGuard(unsigned Limit, unsigned &Counter)
-: Counter(Counter), Enabled(Counter < Limit), StoreSuccess(false) {}
-~LoadGuard() {
-  if (StoreSuccess)
-++Counter;
-}
-/// Flag the LoadGuard instance as successful, meaning that the load
-/// operation succeeded, and the memory footprint of the AST storage
-/// actually increased. In this case, \p Counter should be incremented upon
-/// destruction.
-void storedSuccessfully() { StoreSuccess

[clang-tools-extra] r368546 - [clangd] Highlighting auto variables as the deduced type.

[Johan Vikstrom via cfe-commits]

Author: jvikstrom
Date: Mon Aug 12 00:45:12 2019
New Revision: 368546

URL: http://llvm.org/viewvc/llvm-project?rev=368546&view=rev
Log:
[clangd] Highlighting auto variables as the deduced type.

Summary:
Done in VisitDeclaratorDecl as the AutoTypeLoc is not deduced.
Scoped to only work for variables.
auto function return values need to be handled in a special way (separate patch 
for that).
auto's that are in lambdas ar enot highlighted as we don't highlight their 
underlying type (it's a RecordDecl, but the name is not an identifier so it 
returns in addToken).

Reviewers: hokein, ilya-biryukov

Subscribers: MaskRay, jkorous, arphaman, kadircet, cfe-commits

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp

Modified: clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp?rev=368546&r1=368545&r2=368546&view=diff
==
--- clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp (original)
+++ clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp Mon Aug 12 00:45:12 
2019
@@ -97,8 +97,8 @@ public:
   }
 
   bool VisitTypedefNameDecl(TypedefNameDecl *TD) {
-if(const auto *TSI = TD->getTypeSourceInfo())
-  addTypeLoc(TD->getLocation(), TSI->getTypeLoc());
+if (const auto *TSI = TD->getTypeSourceInfo())
+  addType(TD->getLocation(), TSI->getTypeLoc().getTypePtr());
 return true;
   }
 
@@ -120,10 +120,8 @@ public:
 // structs. It also makes us not highlight certain namespace qualifiers
 // twice. For elaborated types the actual type is highlighted as an inner
 // TypeLoc.
-if (TL.getTypeLocClass() == TypeLoc::TypeLocClass::Elaborated)
-  return true;
-
-addTypeLoc(TL.getBeginLoc(), TL);
+if (TL.getTypeLocClass() != TypeLoc::TypeLocClass::Elaborated)
+  addType(TL.getBeginLoc(), TL.getTypePtr());
 return true;
   }
 
@@ -144,15 +142,27 @@ public:
 HighlightingTokenCollector>::TraverseConstructorInitializer(CI);
   }
 
-private:
-  void addTypeLoc(SourceLocation Loc, const TypeLoc &TL) {
-if (const Type *TP = TL.getTypePtr()) {
-  if (const TagDecl *TD = TP->getAsTagDecl())
-addToken(Loc, TD);
-  if (TP->isBuiltinType())
-// Builtins must be special cased as they do not have a TagDecl.
-addToken(Loc, HighlightingKind::Primitive);
+  bool VisitDeclaratorDecl(DeclaratorDecl *D) {
+if ((!D->getTypeSourceInfo()))
+  return true;
+
+if (auto *AT = D->getType()->getContainedAutoType()) {
+  const auto Deduced = AT->getDeducedType();
+  if (!Deduced.isNull())
+addType(D->getTypeSpecStartLoc(), Deduced.getTypePtr());
 }
+return true;
+  }
+
+private:
+  void addType(SourceLocation Loc, const Type *TP) {
+if (!TP)
+  return;
+if (TP->isBuiltinType())
+  // Builtins must be special cased as they do not have a TagDecl.
+  addToken(Loc, HighlightingKind::Primitive);
+if (const TagDecl *TD = TP->getAsTagDecl())
+  addToken(Loc, TD);
   }
 
   void addToken(SourceLocation Loc, const NamedDecl *D) {

Modified: clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp?rev=368546&r1=368545&r2=368546&view=diff
==
--- clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp 
(original)
+++ clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp Mon 
Aug 12 00:45:12 2019
@@ -99,9 +99,9 @@ TEST(SemanticHighlighting, GetsCorrectTo
   struct {
   } $Variable[[S]];
   $Primitive[[void]] $Function[[foo]]($Primitive[[int]] $Variable[[A]], 
$Class[[AS]] $Variable[[As]]) {
-auto $Variable[[VeryLongVariableName]] = 12312;
+$Primitive[[auto]] $Variable[[VeryLongVariableName]] = 12312;
 $Class[[AS]] $Variable[[AA]];
-auto $Variable[[L]] = $Variable[[AA]].$Field[[SomeMember]] + 
$Variable[[A]];
+$Primitive[[auto]] $Variable[[L]] = 
$Variable[[AA]].$Field[[SomeMember]] + $Variable[[A]];
 auto $Variable[[FN]] = [ $Variable[[AA]]]($Primitive[[int]] 
$Variable[[A]]) -> $Primitive[[void]] {};
 $Variable[[FN]](12312);
   }
@@ -303,6 +303,20 @@ TEST(SemanticHighlighting, GetsCorrectTo
   class $Class[[Bar2]] : public $Class[[Bar]] {
 $Class[[Bar2]]() : $Class[[Bar]]($Class[[Foo]](), $EnumConstant[[EC]]) 
{}
   };
+)cpp",
+R"cpp(
+  enum $Enum[[E]] {
+$EnumConstant[[E]],
+  };
+  class $Class[[Foo]] {};
+  $Enum[[auto]] $Variable[[AE]] = $Enum[[E]]::$EnumConstant[[E]];
+  $Class[[auto]] $Variable[[AF]] = 

[PATCH] D65996: [clangd] Highlighting auto variables as the deduced type.

[Johan Vikström via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL368546: [clangd] Highlighting auto variables as the deduced 
type. (authored by jvikstrom, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65996?vs=214373&id=214582#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65996

Files:
  clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
  clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp

Index: clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
@@ -97,8 +97,8 @@
   }
 
   bool VisitTypedefNameDecl(TypedefNameDecl *TD) {
-if(const auto *TSI = TD->getTypeSourceInfo())
-  addTypeLoc(TD->getLocation(), TSI->getTypeLoc());
+if (const auto *TSI = TD->getTypeSourceInfo())
+  addType(TD->getLocation(), TSI->getTypeLoc().getTypePtr());
 return true;
   }
 
@@ -120,10 +120,8 @@
 // structs. It also makes us not highlight certain namespace qualifiers
 // twice. For elaborated types the actual type is highlighted as an inner
 // TypeLoc.
-if (TL.getTypeLocClass() == TypeLoc::TypeLocClass::Elaborated)
-  return true;
-
-addTypeLoc(TL.getBeginLoc(), TL);
+if (TL.getTypeLocClass() != TypeLoc::TypeLocClass::Elaborated)
+  addType(TL.getBeginLoc(), TL.getTypePtr());
 return true;
   }
 
@@ -144,15 +142,27 @@
 HighlightingTokenCollector>::TraverseConstructorInitializer(CI);
   }
 
-private:
-  void addTypeLoc(SourceLocation Loc, const TypeLoc &TL) {
-if (const Type *TP = TL.getTypePtr()) {
-  if (const TagDecl *TD = TP->getAsTagDecl())
-addToken(Loc, TD);
-  if (TP->isBuiltinType())
-// Builtins must be special cased as they do not have a TagDecl.
-addToken(Loc, HighlightingKind::Primitive);
+  bool VisitDeclaratorDecl(DeclaratorDecl *D) {
+if ((!D->getTypeSourceInfo()))
+  return true;
+
+if (auto *AT = D->getType()->getContainedAutoType()) {
+  const auto Deduced = AT->getDeducedType();
+  if (!Deduced.isNull())
+addType(D->getTypeSpecStartLoc(), Deduced.getTypePtr());
 }
+return true;
+  }
+
+private:
+  void addType(SourceLocation Loc, const Type *TP) {
+if (!TP)
+  return;
+if (TP->isBuiltinType())
+  // Builtins must be special cased as they do not have a TagDecl.
+  addToken(Loc, HighlightingKind::Primitive);
+if (const TagDecl *TD = TP->getAsTagDecl())
+  addToken(Loc, TD);
   }
 
   void addToken(SourceLocation Loc, const NamedDecl *D) {
Index: clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp
@@ -99,9 +99,9 @@
   struct {
   } $Variable[[S]];
   $Primitive[[void]] $Function[[foo]]($Primitive[[int]] $Variable[[A]], $Class[[AS]] $Variable[[As]]) {
-auto $Variable[[VeryLongVariableName]] = 12312;
+$Primitive[[auto]] $Variable[[VeryLongVariableName]] = 12312;
 $Class[[AS]] $Variable[[AA]];
-auto $Variable[[L]] = $Variable[[AA]].$Field[[SomeMember]] + $Variable[[A]];
+$Primitive[[auto]] $Variable[[L]] = $Variable[[AA]].$Field[[SomeMember]] + $Variable[[A]];
 auto $Variable[[FN]] = [ $Variable[[AA]]]($Primitive[[int]] $Variable[[A]]) -> $Primitive[[void]] {};
 $Variable[[FN]](12312);
   }
@@ -303,6 +303,20 @@
   class $Class[[Bar2]] : public $Class[[Bar]] {
 $Class[[Bar2]]() : $Class[[Bar]]($Class[[Foo]](), $EnumConstant[[EC]]) {}
   };
+)cpp",
+R"cpp(
+  enum $Enum[[E]] {
+$EnumConstant[[E]],
+  };
+  class $Class[[Foo]] {};
+  $Enum[[auto]] $Variable[[AE]] = $Enum[[E]]::$EnumConstant[[E]];
+  $Class[[auto]] $Variable[[AF]] = $Class[[Foo]]();
+  $Class[[decltype]](auto) $Variable[[AF2]] = $Class[[Foo]]();
+  $Class[[auto]] *$Variable[[AFP]] = &$Variable[[AF]];
+  $Enum[[auto]] &$Variable[[AER]] = $Variable[[AE]];
+  $Primitive[[auto]] $Variable[[Form]] = 10.2 + 2 * 4;
+  $Primitive[[decltype]]($Variable[[Form]]) $Variable[[F]] = 10;
+  auto $Variable[[Fun]] = []()->$Primitive[[void]]{};
 )cpp"};
   for (const auto &TestCase : TestCases) {
 checkHighlightings(TestCase);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65999: [ASTImporter] Import additional flags for functions.

[Balázs Kéri via Phabricator via cfe-commits]

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



Comment at: clang/unittests/AST/ASTImporterTest.cpp:5239
 
+TEST_P(ASTImporterOptionSpecificTestBase, ImportOfDefaultImplicitFunctions) {
+  // Test that import of implicit functions works and the functions

a_sidorin wrote:
> balazske wrote:
> > martong wrote:
> > > I don't exactly see how this test is related.
> > I do not remember exactly why this test was added but probably the problem 
> > is structural equivalence related: The flags are not imported correctly for 
> > the first time, and at the second import structural match fails and a new 
> > Decl is created instead of returning the existing one. This test fails if 
> > the change is not applied.
> Should we consider isExplicitlyDefaulted() when computing structural 
> equivalence?
We may use `isExplicitlyDefaulted` and `isDeletedAsWritten` and 
`isVirtualAsWritten` but in another patch.



Comment at: clang/unittests/AST/ASTImporterTest.cpp:5246
+struct X { };
+void f() { X x1, x2; x1 = x2; X *x3 = new X; delete x3; }
+)",

a_sidorin wrote:
> Can we remove the function body or reduce it to 'X x'?
The `delete x3` is needed to get a destructor for `X` generated. But the test 
is about testing the implicit functions and I think it is better to have more 
of them. This code is there to have these functions generated so I do not like 
remove of it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65999



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

[PATCH] D65999: [ASTImporter] Import additional flags for functions.

[Balázs Kéri via Phabricator via cfe-commits]

balazske updated this revision to Diff 214585.
balazske added a comment.

- Using StringRef, added comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65999

Files:
  clang/lib/AST/ASTImporter.cpp
  clang/unittests/AST/ASTImporterTest.cpp


Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -5270,9 +5270,53 @@
   }
 }
 
-INSTANTIATE_TEST_CASE_P(ParameterizedTests, SVEBuiltins,
-::testing::Values(ArgVector{"-target",
-"aarch64-linux-gnu"}), );
+TEST_P(ASTImporterOptionSpecificTestBase, ImportOfDefaultImplicitFunctions) {
+  // Test that import of implicit functions works and the functions
+  // are merged into one chain.
+  auto GetDeclToImport = [this](StringRef File) {
+Decl *FromTU = getTuDecl(
+R"(
+struct X { };
+// Force generating some implicit operator definitions for X.
+void f() { X x1, x2; x1 = x2; X *x3 = new X; delete x3; }
+)",
+Lang_CXX11, File);
+auto *FromD = FirstDeclMatcher().match(
+FromTU, cxxRecordDecl(hasName("X"), unless(isImplicit(;
+// Destructor is picked as one example of implicit function.
+return FromD->getDestructor();
+  };
+
+  auto *ToD1 = Import(GetDeclToImport("input1.cc"), Lang_CXX11);
+  ASSERT_TRUE(ToD1);
+
+  auto *ToD2 = Import(GetDeclToImport("input2.cc"), Lang_CXX11);
+  ASSERT_TRUE(ToD2);
+
+  EXPECT_EQ(ToD1->getCanonicalDecl(), ToD2->getCanonicalDecl());
+}
+
+TEST_P(ASTImporterOptionSpecificTestBase,
+   ImportOfExplicitlyDefaultedOrDeleted) {
+  Decl *FromTU = getTuDecl(
+  R"(
+struct X { X() = default; X(const X&) = delete; };
+  )",
+  Lang_CXX11);
+  auto *FromX = FirstDeclMatcher().match(
+  FromTU, cxxRecordDecl(hasName("X")));
+  auto *ImportedX = Import(FromX, Lang_CXX11);
+  auto *Constr1 = FirstDeclMatcher().match(
+  ImportedX, cxxConstructorDecl(hasName("X"), unless(isImplicit(;
+  auto *Constr2 = LastDeclMatcher().match(
+  ImportedX, cxxConstructorDecl(hasName("X"), unless(isImplicit(;
+
+  ASSERT_TRUE(ImportedX);
+  EXPECT_TRUE(Constr1->isDefaulted());
+  EXPECT_TRUE(Constr1->isExplicitlyDefaulted());
+  EXPECT_TRUE(Constr2->isDeletedAsWritten());
+  EXPECT_EQ(ImportedX->isAggregate(), FromX->isAggregate());
+}
 
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, ASTImporterLookupTableTest,
 DefaultTestValuesForRunOptions, );
@@ -5326,5 +5370,9 @@
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, LLDBLookupTest,
 DefaultTestValuesForRunOptions, );
 
+INSTANTIATE_TEST_CASE_P(ParameterizedTests, SVEBuiltins,
+::testing::Values(ArgVector{"-target",
+"aarch64-linux-gnu"}), );
+
 } // end namespace ast_matchers
 } // end namespace clang
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -3292,6 +3292,9 @@
   ToFunction->setVirtualAsWritten(D->isVirtualAsWritten());
   ToFunction->setTrivial(D->isTrivial());
   ToFunction->setPure(D->isPure());
+  ToFunction->setDefaulted(D->isDefaulted());
+  ToFunction->setExplicitlyDefaulted(D->isExplicitlyDefaulted());
+  ToFunction->setDeletedAsWritten(D->isDeletedAsWritten());
   ToFunction->setRangeEnd(ToEndLoc);
 
   // Set the parameters.


Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -5270,9 +5270,53 @@
   }
 }
 
-INSTANTIATE_TEST_CASE_P(ParameterizedTests, SVEBuiltins,
-::testing::Values(ArgVector{"-target",
-"aarch64-linux-gnu"}), );
+TEST_P(ASTImporterOptionSpecificTestBase, ImportOfDefaultImplicitFunctions) {
+  // Test that import of implicit functions works and the functions
+  // are merged into one chain.
+  auto GetDeclToImport = [this](StringRef File) {
+Decl *FromTU = getTuDecl(
+R"(
+struct X { };
+// Force generating some implicit operator definitions for X.
+void f() { X x1, x2; x1 = x2; X *x3 = new X; delete x3; }
+)",
+Lang_CXX11, File);
+auto *FromD = FirstDeclMatcher().match(
+FromTU, cxxRecordDecl(hasName("X"), unless(isImplicit(;
+// Destructor is picked as one example of implicit function.
+return FromD->getDestructor();
+  };
+
+  auto *ToD1 = Import(GetDeclToImport("input1.cc"), Lang_CXX11);
+  ASSERT_TRUE(ToD1);
+
+  auto *ToD2 = Import(GetDeclToImport("input2.cc"), Lang_CXX11);
+  ASSERT_TRUE(ToD2);
+
+  EXPECT_

[PATCH] D65877: [libTooling] In Transformer, generalize `applyFirst` to admit rules with incompatible matchers.

[Dmitri Gribenko via Phabricator via cfe-commits]

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

Nice simplification, thanks!




Comment at: clang/lib/Tooling/Refactoring/Transformer.cpp:127
+  // in `taggedMatchers`.
+  std::map, 1>>
+  Buckets;

`unordered_map`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65877



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

[PATCH] D65373: [clangd] Update features table in the docs with links to LSP extension proposals

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang-tools-extra/docs/clangd/Features.rst:260
 +-++--+
-| Syntax and Semantic Coloring| No |   No |
+| Syntax and Semantic Coloring|`Proposed`__|   No |
 +-++--+

Could you mention that semantic coloring is implemented in clangd? (with a 
footnote that it only works in `Theia` atm)



Comment at: clang-tools-extra/docs/clangd/Features.rst:269
+
+__ https://github.com/microsoft/language-server-protocol/issues/18
+__ https://github.com/microsoft/language-server-protocol/issues/468

Having the same marker for three links makes is hard to read. WDYT about using 
the footnote syntax here?
```
Proposed [1]_
Proposed [2]_


.. [1] https://github.com/microsoft/language-server-protocol/issues/468
.. [2] https://github.com/microsoft/language-server-protocol/issues/136
```

Just a suggestion, not sure which of the two versions renders nicer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65373



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

[PATCH] D66074: [clangd] Drop diags from non-written #include.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: ilya-biryukov.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay.
Herald added a project: clang.

This would fix that we show weird diagnostics on random lines of the main file.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66074

Files:
  clang-tools-extra/clangd/Diagnostics.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -948,6 +948,13 @@
   EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre());
 }
 
+TEST(IgnoreDiags, FromNonWrittenInclude) {
+  TestTU TU = TestTU::withCode("");
+  TU.ExtraArgs.push_back("--include=a.h");
+  TU.AdditionalFiles = {{"a.h", "void main();"}};
+  EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre());
+}
+
 } // namespace
 
 } // namespace clangd
Index: clang-tools-extra/clangd/Diagnostics.cpp
===
--- clang-tools-extra/clangd/Diagnostics.cpp
+++ clang-tools-extra/clangd/Diagnostics.cpp
@@ -122,8 +122,12 @@
 return SM.getIncludeLoc(SM.getFileID(SLoc));
   };
   for (auto IncludeLocation = GetIncludeLoc(DiagLoc); 
IncludeLocation.isValid();
-   IncludeLocation = GetIncludeLoc(IncludeLocation))
-IncludeInMainFile = IncludeLocation;
+   IncludeLocation = GetIncludeLoc(IncludeLocation)) {
+if (clangd::isInsideMainFile(IncludeLocation, SM)) {
+  IncludeInMainFile = IncludeLocation;
+  break;
+}
+  }
   if (IncludeInMainFile.isInvalid())
 return false;
 


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -948,6 +948,13 @@
   EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre());
 }
 
+TEST(IgnoreDiags, FromNonWrittenInclude) {
+  TestTU TU = TestTU::withCode("");
+  TU.ExtraArgs.push_back("--include=a.h");
+  TU.AdditionalFiles = {{"a.h", "void main();"}};
+  EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre());
+}
+
 } // namespace
 
 } // namespace clangd
Index: clang-tools-extra/clangd/Diagnostics.cpp
===
--- clang-tools-extra/clangd/Diagnostics.cpp
+++ clang-tools-extra/clangd/Diagnostics.cpp
@@ -122,8 +122,12 @@
 return SM.getIncludeLoc(SM.getFileID(SLoc));
   };
   for (auto IncludeLocation = GetIncludeLoc(DiagLoc); IncludeLocation.isValid();
-   IncludeLocation = GetIncludeLoc(IncludeLocation))
-IncludeInMainFile = IncludeLocation;
+   IncludeLocation = GetIncludeLoc(IncludeLocation)) {
+if (clangd::isInsideMainFile(IncludeLocation, SM)) {
+  IncludeInMainFile = IncludeLocation;
+  break;
+}
+  }
   if (IncludeInMainFile.isInvalid())
 return false;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66074: [clangd] Drop diags from non-written #include.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM, thanks!
We should also merge this into the release branch if it's not too late yet.




Comment at: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp:952
+TEST(IgnoreDiags, FromNonWrittenInclude) {
+  TestTU TU = TestTU::withCode("");
+  TU.ExtraArgs.push_back("--include=a.h");

Could you add a comment describing which diagnostic we do **not** want to see 
here and why?
I assume it's `main should return int`, right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66074



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

[PATCH] D65999: [ASTImporter] Import additional flags for functions.

[Balázs Kéri via Phabricator via cfe-commits]

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



Comment at: clang/unittests/AST/ASTImporterTest.cpp:5239
 
+TEST_P(ASTImporterOptionSpecificTestBase, ImportOfDefaultImplicitFunctions) {
+  // Test that import of implicit functions works and the functions

balazske wrote:
> a_sidorin wrote:
> > balazske wrote:
> > > martong wrote:
> > > > I don't exactly see how this test is related.
> > > I do not remember exactly why this test was added but probably the 
> > > problem is structural equivalence related: The flags are not imported 
> > > correctly for the first time, and at the second import structural match 
> > > fails and a new Decl is created instead of returning the existing one. 
> > > This test fails if the change is not applied.
> > Should we consider isExplicitlyDefaulted() when computing structural 
> > equivalence?
> We may use `isExplicitlyDefaulted` and `isDeletedAsWritten` and 
> `isVirtualAsWritten` but in another patch.
It can be good to add check for `isExplicitlyDefaulted` because it is a 
separate bit and not checked yet. Probably in another patch that has a test for 
this too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65999



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

[PATCH] D66074: [clangd] Drop diags from non-written #include.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 214590.
hokein marked an inline comment as done.
hokein added a comment.

Add a comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66074

Files:
  clang-tools-extra/clangd/Diagnostics.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -948,6 +948,15 @@
   EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre());
 }
 
+TEST(IgnoreDiags, FromNonWrittenInclude) {
+  TestTU TU = TestTU::withCode("");
+  TU.ExtraArgs.push_back("--include=a.h");
+  TU.AdditionalFiles = {{"a.h", "void main();"}};
+  // The diagnostic "main must return int" is from the header, we don't attempt
+  // to render it in the main file as there is no written location there.
+  EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre());
+}
+
 } // namespace
 
 } // namespace clangd
Index: clang-tools-extra/clangd/Diagnostics.cpp
===
--- clang-tools-extra/clangd/Diagnostics.cpp
+++ clang-tools-extra/clangd/Diagnostics.cpp
@@ -122,8 +122,12 @@
 return SM.getIncludeLoc(SM.getFileID(SLoc));
   };
   for (auto IncludeLocation = GetIncludeLoc(DiagLoc); 
IncludeLocation.isValid();
-   IncludeLocation = GetIncludeLoc(IncludeLocation))
-IncludeInMainFile = IncludeLocation;
+   IncludeLocation = GetIncludeLoc(IncludeLocation)) {
+if (clangd::isInsideMainFile(IncludeLocation, SM)) {
+  IncludeInMainFile = IncludeLocation;
+  break;
+}
+  }
   if (IncludeInMainFile.isInvalid())
 return false;
 


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -948,6 +948,15 @@
   EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre());
 }
 
+TEST(IgnoreDiags, FromNonWrittenInclude) {
+  TestTU TU = TestTU::withCode("");
+  TU.ExtraArgs.push_back("--include=a.h");
+  TU.AdditionalFiles = {{"a.h", "void main();"}};
+  // The diagnostic "main must return int" is from the header, we don't attempt
+  // to render it in the main file as there is no written location there.
+  EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre());
+}
+
 } // namespace
 
 } // namespace clangd
Index: clang-tools-extra/clangd/Diagnostics.cpp
===
--- clang-tools-extra/clangd/Diagnostics.cpp
+++ clang-tools-extra/clangd/Diagnostics.cpp
@@ -122,8 +122,12 @@
 return SM.getIncludeLoc(SM.getFileID(SLoc));
   };
   for (auto IncludeLocation = GetIncludeLoc(DiagLoc); IncludeLocation.isValid();
-   IncludeLocation = GetIncludeLoc(IncludeLocation))
-IncludeInMainFile = IncludeLocation;
+   IncludeLocation = GetIncludeLoc(IncludeLocation)) {
+if (clangd::isInsideMainFile(IncludeLocation, SM)) {
+  IncludeInMainFile = IncludeLocation;
+  break;
+}
+  }
   if (IncludeInMainFile.isInvalid())
 return false;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r368549 - [clangd] Drop diags from non-written #include.

[Haojian Wu via cfe-commits]

Author: hokein
Date: Mon Aug 12 02:35:04 2019
New Revision: 368549

URL: http://llvm.org/viewvc/llvm-project?rev=368549&view=rev
Log:
[clangd] Drop diags from non-written #include.

Summary: This would fix that we show weird diagnostics on random lines of the 
main file.

Reviewers: ilya-biryukov

Subscribers: MaskRay, jkorous, arphaman, kadircet, cfe-commits

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/Diagnostics.cpp
clang-tools-extra/trunk/clangd/unittests/DiagnosticsTests.cpp

Modified: clang-tools-extra/trunk/clangd/Diagnostics.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Diagnostics.cpp?rev=368549&r1=368548&r2=368549&view=diff
==
--- clang-tools-extra/trunk/clangd/Diagnostics.cpp (original)
+++ clang-tools-extra/trunk/clangd/Diagnostics.cpp Mon Aug 12 02:35:04 2019
@@ -122,8 +122,12 @@ bool adjustDiagFromHeader(Diag &D, const
 return SM.getIncludeLoc(SM.getFileID(SLoc));
   };
   for (auto IncludeLocation = GetIncludeLoc(DiagLoc); 
IncludeLocation.isValid();
-   IncludeLocation = GetIncludeLoc(IncludeLocation))
-IncludeInMainFile = IncludeLocation;
+   IncludeLocation = GetIncludeLoc(IncludeLocation)) {
+if (clangd::isInsideMainFile(IncludeLocation, SM)) {
+  IncludeInMainFile = IncludeLocation;
+  break;
+}
+  }
   if (IncludeInMainFile.isInvalid())
 return false;
 

Modified: clang-tools-extra/trunk/clangd/unittests/DiagnosticsTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/DiagnosticsTests.cpp?rev=368549&r1=368548&r2=368549&view=diff
==
--- clang-tools-extra/trunk/clangd/unittests/DiagnosticsTests.cpp (original)
+++ clang-tools-extra/trunk/clangd/unittests/DiagnosticsTests.cpp Mon Aug 12 
02:35:04 2019
@@ -948,6 +948,15 @@ TEST(IgnoreDiags, FromNonWrittenSources)
   EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre());
 }
 
+TEST(IgnoreDiags, FromNonWrittenInclude) {
+  TestTU TU = TestTU::withCode("");
+  TU.ExtraArgs.push_back("--include=a.h");
+  TU.AdditionalFiles = {{"a.h", "void main();"}};
+  // The diagnostic "main must return int" is from the header, we don't attempt
+  // to render it in the main file as there is no written location there.
+  EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre());
+}
+
 } // namespace
 
 } // namespace clangd


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

[PATCH] D66074: [clangd] Drop diags from non-written #include.

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL368549: [clangd] Drop diags from non-written #include. 
(authored by hokein, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66074?vs=214590&id=214591#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66074

Files:
  clang-tools-extra/trunk/clangd/Diagnostics.cpp
  clang-tools-extra/trunk/clangd/unittests/DiagnosticsTests.cpp


Index: clang-tools-extra/trunk/clangd/Diagnostics.cpp
===
--- clang-tools-extra/trunk/clangd/Diagnostics.cpp
+++ clang-tools-extra/trunk/clangd/Diagnostics.cpp
@@ -122,8 +122,12 @@
 return SM.getIncludeLoc(SM.getFileID(SLoc));
   };
   for (auto IncludeLocation = GetIncludeLoc(DiagLoc); 
IncludeLocation.isValid();
-   IncludeLocation = GetIncludeLoc(IncludeLocation))
-IncludeInMainFile = IncludeLocation;
+   IncludeLocation = GetIncludeLoc(IncludeLocation)) {
+if (clangd::isInsideMainFile(IncludeLocation, SM)) {
+  IncludeInMainFile = IncludeLocation;
+  break;
+}
+  }
   if (IncludeInMainFile.isInvalid())
 return false;
 
Index: clang-tools-extra/trunk/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/trunk/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/trunk/clangd/unittests/DiagnosticsTests.cpp
@@ -948,6 +948,15 @@
   EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre());
 }
 
+TEST(IgnoreDiags, FromNonWrittenInclude) {
+  TestTU TU = TestTU::withCode("");
+  TU.ExtraArgs.push_back("--include=a.h");
+  TU.AdditionalFiles = {{"a.h", "void main();"}};
+  // The diagnostic "main must return int" is from the header, we don't attempt
+  // to render it in the main file as there is no written location there.
+  EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre());
+}
+
 } // namespace
 
 } // namespace clangd


Index: clang-tools-extra/trunk/clangd/Diagnostics.cpp
===
--- clang-tools-extra/trunk/clangd/Diagnostics.cpp
+++ clang-tools-extra/trunk/clangd/Diagnostics.cpp
@@ -122,8 +122,12 @@
 return SM.getIncludeLoc(SM.getFileID(SLoc));
   };
   for (auto IncludeLocation = GetIncludeLoc(DiagLoc); IncludeLocation.isValid();
-   IncludeLocation = GetIncludeLoc(IncludeLocation))
-IncludeInMainFile = IncludeLocation;
+   IncludeLocation = GetIncludeLoc(IncludeLocation)) {
+if (clangd::isInsideMainFile(IncludeLocation, SM)) {
+  IncludeInMainFile = IncludeLocation;
+  break;
+}
+  }
   if (IncludeInMainFile.isInvalid())
 return false;
 
Index: clang-tools-extra/trunk/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/trunk/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/trunk/clangd/unittests/DiagnosticsTests.cpp
@@ -948,6 +948,15 @@
   EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre());
 }
 
+TEST(IgnoreDiags, FromNonWrittenInclude) {
+  TestTU TU = TestTU::withCode("");
+  TU.ExtraArgs.push_back("--include=a.h");
+  TU.AdditionalFiles = {{"a.h", "void main();"}};
+  // The diagnostic "main must return int" is from the header, we don't attempt
+  // to render it in the main file as there is no written location there.
+  EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre());
+}
+
 } // namespace
 
 } // namespace clangd
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66074: [clangd] Drop diags from non-written #include.

[Haojian Wu via Phabricator via cfe-commits]

hokein marked an inline comment as done.
hokein added a subscriber: hans.
hokein added a comment.

@hans is there still any chance to merge this patch into the release?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66074



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

[PATCH] D65752: [Sema] Refactor LookupVisibleDecls. NFC

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a subscriber: doug.gregor.
ilya-biryukov added a comment.

In D65752#1623914 , @sammccall wrote:

> This looks reasonable to me, there are a couple of variations you might think 
> about:
>
> - also treat QualifiedNameLookup as an option, and override in places with an 
> RAII pattern like `ScopedOverride Unqual(QualifiedNameLookup, false)` (why 
> don't we have a class like that?). This would further reduce args.


I actually think it's good to have it as an explicit option. Qualified and 
unqualified lookup are so vastly different use-cases, that having the flag that 
discriminates them as a function parameter looks like a better trade-off.
Besides, RAII objects are more code and more complicated.

> - put the options in a `LookupOpts` struct and pass it by const reference - 
> this is a less invasive change

Also an option. I personally like the current version more even though it's 
more invasive; it adds more structure IMO.

Those decisions are personal preferences, I don't have strong opinions there.
@doug.gregor as an original author of the code, do you have any opinions on 
where this code should go?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65752



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

r368551 - [ASTImporter] Fix for import of friend class template with definition.

[Balazs Keri via cfe-commits]

Author: balazske
Date: Mon Aug 12 03:07:38 2019
New Revision: 368551

URL: http://llvm.org/viewvc/llvm-project?rev=368551&view=rev
Log:
[ASTImporter] Fix for import of friend class template with definition.

Summary:
If there is a friend class template "prototype" (forward declaration)
and later a definition for it in the existing code, this existing
definition may be not found by ASTImporter because it is not linked
to the prototype (under the friend AST node). The problem is fixed by
looping over all found matching decls instead of break after the first
found one.

Reviewers: martong, a.sidorin, shafik, a_sidorin

Reviewed By: a_sidorin

Subscribers: rnkovacs, dkrupp, Szelethus, gamesh411, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/lib/AST/ASTImporter.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=368551&r1=368550&r2=368551&view=diff
==
--- cfe/trunk/lib/AST/ASTImporter.cpp (original)
+++ cfe/trunk/lib/AST/ASTImporter.cpp Mon Aug 12 03:07:38 2019
@@ -5082,11 +5082,13 @@ ExpectedDecl ASTNodeImporter::VisitClass
 if (IsStructuralMatch(D, FoundTemplate)) {
   ClassTemplateDecl *TemplateWithDef =
   getTemplateDefinition(FoundTemplate);
-  if (D->isThisDeclarationADefinition() && TemplateWithDef) {
+  if (D->isThisDeclarationADefinition() && TemplateWithDef)
 return Importer.MapImported(D, TemplateWithDef);
-  }
-  FoundByLookup = FoundTemplate;
-  break;
+  if (!FoundByLookup)
+FoundByLookup = FoundTemplate;
+  // Search in all matches because there may be multiple decl chains,
+  // see ASTTests test ImportExistingFriendClassTemplateDef.
+  continue;
 }
   }
 

Modified: cfe/trunk/unittests/AST/ASTImporterTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/ASTImporterTest.cpp?rev=368551&r1=368550&r2=368551&view=diff
==
--- cfe/trunk/unittests/AST/ASTImporterTest.cpp (original)
+++ cfe/trunk/unittests/AST/ASTImporterTest.cpp Mon Aug 12 03:07:38 2019
@@ -5191,6 +5191,50 @@ TEST_P(ASTImporterOptionSpecificTestBase
   EXPECT_TRUE(ToF);
 }
 
+TEST_P(ASTImporterOptionSpecificTestBase,
+   ImportExistingFriendClassTemplateDef) {
+  auto Code =
+  R"(
+template 
+struct Base {
+  template 
+  friend struct Class;
+};
+template 
+struct Class { };
+)";
+
+  TranslationUnitDecl *ToTU = getToTuDecl(Code, Lang_CXX);
+  TranslationUnitDecl *FromTU = getTuDecl(Code, Lang_CXX, "input.cc");
+
+  auto *ToClassProto = FirstDeclMatcher().match(
+  ToTU, classTemplateDecl(hasName("Class")));
+  auto *ToClassDef = LastDeclMatcher().match(
+  ToTU, classTemplateDecl(hasName("Class")));
+  ASSERT_FALSE(ToClassProto->isThisDeclarationADefinition());
+  ASSERT_TRUE(ToClassDef->isThisDeclarationADefinition());
+  // Previous friend decl is not linked to it!
+  ASSERT_FALSE(ToClassDef->getPreviousDecl());
+  ASSERT_EQ(ToClassDef->getMostRecentDecl(), ToClassDef);
+  ASSERT_EQ(ToClassProto->getMostRecentDecl(), ToClassProto);
+
+  auto *FromClassProto = FirstDeclMatcher().match(
+  FromTU, classTemplateDecl(hasName("Class")));
+  auto *FromClassDef = LastDeclMatcher().match(
+  FromTU, classTemplateDecl(hasName("Class")));
+  ASSERT_FALSE(FromClassProto->isThisDeclarationADefinition());
+  ASSERT_TRUE(FromClassDef->isThisDeclarationADefinition());
+  ASSERT_FALSE(FromClassDef->getPreviousDecl());
+  ASSERT_EQ(FromClassDef->getMostRecentDecl(), FromClassDef);
+  ASSERT_EQ(FromClassProto->getMostRecentDecl(), FromClassProto);
+
+  auto *ImportedDef = Import(FromClassDef, Lang_CXX);
+  // At import we should find the definition for 'Class' even if the
+  // prototype (inside 'friend') for it comes first in the AST and is not
+  // linked to the definition.
+  EXPECT_EQ(ImportedDef, ToClassDef);
+}  
+  
 struct LLDBLookupTest : ASTImporterOptionSpecificTestBase {
   LLDBLookupTest() {
 Creator = [](ASTContext &ToContext, FileManager &ToFileManager,


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

[PATCH] D65269: [ASTImporter] Fix for import of friend class template with definition.

[Balázs Kéri via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL368551: [ASTImporter] Fix for import of friend class 
template with definition. (authored by balazske, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65269

Files:
  cfe/trunk/lib/AST/ASTImporter.cpp
  cfe/trunk/unittests/AST/ASTImporterTest.cpp


Index: cfe/trunk/lib/AST/ASTImporter.cpp
===
--- cfe/trunk/lib/AST/ASTImporter.cpp
+++ cfe/trunk/lib/AST/ASTImporter.cpp
@@ -5082,11 +5082,13 @@
 if (IsStructuralMatch(D, FoundTemplate)) {
   ClassTemplateDecl *TemplateWithDef =
   getTemplateDefinition(FoundTemplate);
-  if (D->isThisDeclarationADefinition() && TemplateWithDef) {
+  if (D->isThisDeclarationADefinition() && TemplateWithDef)
 return Importer.MapImported(D, TemplateWithDef);
-  }
-  FoundByLookup = FoundTemplate;
-  break;
+  if (!FoundByLookup)
+FoundByLookup = FoundTemplate;
+  // Search in all matches because there may be multiple decl chains,
+  // see ASTTests test ImportExistingFriendClassTemplateDef.
+  continue;
 }
   }
 
Index: cfe/trunk/unittests/AST/ASTImporterTest.cpp
===
--- cfe/trunk/unittests/AST/ASTImporterTest.cpp
+++ cfe/trunk/unittests/AST/ASTImporterTest.cpp
@@ -5191,6 +5191,50 @@
   EXPECT_TRUE(ToF);
 }
 
+TEST_P(ASTImporterOptionSpecificTestBase,
+   ImportExistingFriendClassTemplateDef) {
+  auto Code =
+  R"(
+template 
+struct Base {
+  template 
+  friend struct Class;
+};
+template 
+struct Class { };
+)";
+
+  TranslationUnitDecl *ToTU = getToTuDecl(Code, Lang_CXX);
+  TranslationUnitDecl *FromTU = getTuDecl(Code, Lang_CXX, "input.cc");
+
+  auto *ToClassProto = FirstDeclMatcher().match(
+  ToTU, classTemplateDecl(hasName("Class")));
+  auto *ToClassDef = LastDeclMatcher().match(
+  ToTU, classTemplateDecl(hasName("Class")));
+  ASSERT_FALSE(ToClassProto->isThisDeclarationADefinition());
+  ASSERT_TRUE(ToClassDef->isThisDeclarationADefinition());
+  // Previous friend decl is not linked to it!
+  ASSERT_FALSE(ToClassDef->getPreviousDecl());
+  ASSERT_EQ(ToClassDef->getMostRecentDecl(), ToClassDef);
+  ASSERT_EQ(ToClassProto->getMostRecentDecl(), ToClassProto);
+
+  auto *FromClassProto = FirstDeclMatcher().match(
+  FromTU, classTemplateDecl(hasName("Class")));
+  auto *FromClassDef = LastDeclMatcher().match(
+  FromTU, classTemplateDecl(hasName("Class")));
+  ASSERT_FALSE(FromClassProto->isThisDeclarationADefinition());
+  ASSERT_TRUE(FromClassDef->isThisDeclarationADefinition());
+  ASSERT_FALSE(FromClassDef->getPreviousDecl());
+  ASSERT_EQ(FromClassDef->getMostRecentDecl(), FromClassDef);
+  ASSERT_EQ(FromClassProto->getMostRecentDecl(), FromClassProto);
+
+  auto *ImportedDef = Import(FromClassDef, Lang_CXX);
+  // At import we should find the definition for 'Class' even if the
+  // prototype (inside 'friend') for it comes first in the AST and is not
+  // linked to the definition.
+  EXPECT_EQ(ImportedDef, ToClassDef);
+}  
+  
 struct LLDBLookupTest : ASTImporterOptionSpecificTestBase {
   LLDBLookupTest() {
 Creator = [](ASTContext &ToContext, FileManager &ToFileManager,


Index: cfe/trunk/lib/AST/ASTImporter.cpp
===
--- cfe/trunk/lib/AST/ASTImporter.cpp
+++ cfe/trunk/lib/AST/ASTImporter.cpp
@@ -5082,11 +5082,13 @@
 if (IsStructuralMatch(D, FoundTemplate)) {
   ClassTemplateDecl *TemplateWithDef =
   getTemplateDefinition(FoundTemplate);
-  if (D->isThisDeclarationADefinition() && TemplateWithDef) {
+  if (D->isThisDeclarationADefinition() && TemplateWithDef)
 return Importer.MapImported(D, TemplateWithDef);
-  }
-  FoundByLookup = FoundTemplate;
-  break;
+  if (!FoundByLookup)
+FoundByLookup = FoundTemplate;
+  // Search in all matches because there may be multiple decl chains,
+  // see ASTTests test ImportExistingFriendClassTemplateDef.
+  continue;
 }
   }
 
Index: cfe/trunk/unittests/AST/ASTImporterTest.cpp
===
--- cfe/trunk/unittests/AST/ASTImporterTest.cpp
+++ cfe/trunk/unittests/AST/ASTImporterTest.cpp
@@ -5191,6 +5191,50 @@
   EXPECT_TRUE(ToF);
 }
 
+TEST_P(ASTImporterOptionSpecificTestBase,
+   ImportExistingFriendClassTemplateDef) {
+  auto Code =
+  R"(
+template 
+struct Base {
+  template 
+  friend struct Class;
+};
+template 
+str

[PATCH] D65050: [SemaTemplate] Mark a function type as dependent when its parameter list contains pack expansion

[S. B. Tam via Phabricator via cfe-commits]

cpplearner added a comment.

ping


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

https://reviews.llvm.org/D65050



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

[PATCH] D47419: [SemaDeclCXX] Allow inheriting constructor declaration that specify a cv-qualified type

[S. B. Tam via Phabricator via cfe-commits]

cpplearner added a comment.
Herald added a project: clang.

ping


Repository:
  rC Clang

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

https://reviews.llvm.org/D47419



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

r368552 - [OpenCL] Fix lang mode predefined macros for C++ mode.

[Anastasia Stulova via cfe-commits]

Author: stulova
Date: Mon Aug 12 03:44:07 2019
New Revision: 368552

URL: http://llvm.org/viewvc/llvm-project?rev=368552&view=rev
Log:
[OpenCL] Fix lang mode predefined macros for C++ mode.

In C++ mode we should only avoid adding __OPENCL_C_VERSION__,
all other predefined macros about the language mode are still
valid.

This change also fixes the language version check in the
headers accordingly.

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


Modified:
cfe/trunk/lib/Frontend/InitPreprocessor.cpp
cfe/trunk/lib/Headers/opencl-c-base.h
cfe/trunk/lib/Headers/opencl-c.h

Modified: cfe/trunk/lib/Frontend/InitPreprocessor.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/InitPreprocessor.cpp?rev=368552&r1=368551&r2=368552&view=diff
==
--- cfe/trunk/lib/Frontend/InitPreprocessor.cpp (original)
+++ cfe/trunk/lib/Frontend/InitPreprocessor.cpp Mon Aug 12 03:44:07 2019
@@ -437,17 +437,17 @@ static void InitializeStandardPredefined
   default:
 llvm_unreachable("Unsupported OpenCL version");
   }
-  Builder.defineMacro("CL_VERSION_1_0", "100");
-  Builder.defineMacro("CL_VERSION_1_1", "110");
-  Builder.defineMacro("CL_VERSION_1_2", "120");
-  Builder.defineMacro("CL_VERSION_2_0", "200");
+}
+Builder.defineMacro("CL_VERSION_1_0", "100");
+Builder.defineMacro("CL_VERSION_1_1", "110");
+Builder.defineMacro("CL_VERSION_1_2", "120");
+Builder.defineMacro("CL_VERSION_2_0", "200");
 
-  if (TI.isLittleEndian())
-Builder.defineMacro("__ENDIAN_LITTLE__");
+if (TI.isLittleEndian())
+  Builder.defineMacro("__ENDIAN_LITTLE__");
 
-  if (LangOpts.FastRelaxedMath)
-Builder.defineMacro("__FAST_RELAXED_MATH__");
-}
+if (LangOpts.FastRelaxedMath)
+  Builder.defineMacro("__FAST_RELAXED_MATH__");
   }
   // Not "standard" per se, but available even with the -undef flag.
   if (LangOpts.AsmPreprocessor)

Modified: cfe/trunk/lib/Headers/opencl-c-base.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/opencl-c-base.h?rev=368552&r1=368551&r2=368552&view=diff
==
--- cfe/trunk/lib/Headers/opencl-c-base.h (original)
+++ cfe/trunk/lib/Headers/opencl-c-base.h Mon Aug 12 03:44:07 2019
@@ -126,7 +126,7 @@ typedef double double8 __attribute__((ex
 typedef double double16 __attribute__((ext_vector_type(16)));
 #endif
 
-#if __OPENCL_C_VERSION__ >= CL_VERSION_2_0
+#if defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ >= CL_VERSION_2_0)
 #define NULL ((void*)0)
 #endif
 
@@ -276,7 +276,7 @@ typedef uint cl_mem_fence_flags;
  */
 #define CLK_GLOBAL_MEM_FENCE   0x02
 
-#if __OPENCL_C_VERSION__ >= CL_VERSION_2_0
+#if defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ >= CL_VERSION_2_0)
 
 typedef enum memory_scope {
   memory_scope_work_item = __OPENCL_MEMORY_SCOPE_WORK_ITEM,
@@ -288,9 +288,6 @@ typedef enum memory_scope {
 #endif
 } memory_scope;
 
-#endif //__OPENCL_C_VERSION__ >= CL_VERSION_2_0
-
-#if __OPENCL_C_VERSION__ >= CL_VERSION_2_0
 /**
  * Queue a memory fence to ensure correct ordering of memory
  * operations between work-items of a work-group to
@@ -313,7 +310,7 @@ typedef enum memory_order
   memory_order_seq_cst = __ATOMIC_SEQ_CST
 } memory_order;
 
-#endif //__OPENCL_C_VERSION__ >= CL_VERSION_2_0
+#endif // defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ >= 
CL_VERSION_2_0)
 
 // OpenCL v1.1 s6.11.3, v1.2 s6.12.14, v2.0 s6.13.14 - Image Read and Write 
Functions
 
@@ -389,14 +386,10 @@ typedef enum memory_order
 #endif //__OPENCL_C_VERSION__ >= CL_VERSION_2_0
 
 // OpenCL v2.0 s6.13.16 - Pipe Functions
-#if __OPENCL_C_VERSION__ >= CL_VERSION_2_0
+#if defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ >= CL_VERSION_2_0)
 #define CLK_NULL_RESERVE_ID (__builtin_astype(((void*)(__SIZE_MAX__)), 
reserve_id_t))
-#endif //__OPENCL_C_VERSION__ >= CL_VERSION_2_0
-
 
 // OpenCL v2.0 s6.13.17 - Enqueue Kernels
-#if __OPENCL_C_VERSION__ >= CL_VERSION_2_0
-
 #define CL_COMPLETE 0x0
 #define CL_RUNNING  0x1
 #define CL_SUBMITTED0x2
@@ -435,7 +428,7 @@ typedef struct {
   size_t localWorkSize[MAX_WORK_DIM];
 } ndrange_t;
 
-#endif //__OPENCL_C_VERSION__ >= CL_VERSION_2_0
+#endif // defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ >= 
CL_VERSION_2_0)
 
 #ifdef cl_intel_device_side_avc_motion_estimation
 #pragma OPENCL EXTENSION cl_intel_device_side_avc_motion_estimation : begin

Modified: cfe/trunk/lib/Headers/opencl-c.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/opencl-c.h?rev=368552&r1=368551&r2=368552&view=diff
==
--- cfe/trunk/lib/Headers/opencl-c.h (original)
+++ cfe/trunk/lib/Headers/opencl-c.h Mon Aug 12 03:44:07 2019
@@

[PATCH] D65941: [OpenCL] Fix lang mode predefined macros for C++ mode

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL368552: [OpenCL] Fix lang mode predefined macros for C++ 
mode. (authored by stulova, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65941?vs=214114&id=214597#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65941

Files:
  cfe/trunk/lib/Frontend/InitPreprocessor.cpp
  cfe/trunk/lib/Headers/opencl-c-base.h
  cfe/trunk/lib/Headers/opencl-c.h

Index: cfe/trunk/lib/Frontend/InitPreprocessor.cpp
===
--- cfe/trunk/lib/Frontend/InitPreprocessor.cpp
+++ cfe/trunk/lib/Frontend/InitPreprocessor.cpp
@@ -437,17 +437,17 @@
   default:
 llvm_unreachable("Unsupported OpenCL version");
   }
-  Builder.defineMacro("CL_VERSION_1_0", "100");
-  Builder.defineMacro("CL_VERSION_1_1", "110");
-  Builder.defineMacro("CL_VERSION_1_2", "120");
-  Builder.defineMacro("CL_VERSION_2_0", "200");
+}
+Builder.defineMacro("CL_VERSION_1_0", "100");
+Builder.defineMacro("CL_VERSION_1_1", "110");
+Builder.defineMacro("CL_VERSION_1_2", "120");
+Builder.defineMacro("CL_VERSION_2_0", "200");
 
-  if (TI.isLittleEndian())
-Builder.defineMacro("__ENDIAN_LITTLE__");
+if (TI.isLittleEndian())
+  Builder.defineMacro("__ENDIAN_LITTLE__");
 
-  if (LangOpts.FastRelaxedMath)
-Builder.defineMacro("__FAST_RELAXED_MATH__");
-}
+if (LangOpts.FastRelaxedMath)
+  Builder.defineMacro("__FAST_RELAXED_MATH__");
   }
   // Not "standard" per se, but available even with the -undef flag.
   if (LangOpts.AsmPreprocessor)
Index: cfe/trunk/lib/Headers/opencl-c.h
===
--- cfe/trunk/lib/Headers/opencl-c.h
+++ cfe/trunk/lib/Headers/opencl-c.h
@@ -11,11 +11,11 @@
 
 #include "opencl-c-base.h"
 
-#if __OPENCL_C_VERSION__ >= CL_VERSION_2_0
+#if defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ >= CL_VERSION_2_0)
 #ifndef cl_khr_depth_images
 #define cl_khr_depth_images
 #endif //cl_khr_depth_images
-#endif //__OPENCL_C_VERSION__ >= CL_VERSION_2_0
+#endif //defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ >= CL_VERSION_2_0)
 
 #if __OPENCL_C_VERSION__ < CL_VERSION_2_0
 #ifdef cl_khr_3d_image_writes
@@ -23,10 +23,10 @@
 #endif //cl_khr_3d_image_writes
 #endif //__OPENCL_C_VERSION__ < CL_VERSION_2_0
 
-#if __OPENCL_C_VERSION__ >= CL_VERSION_1_2
+#if defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ >= CL_VERSION_1_2)
 #pragma OPENCL EXTENSION cl_intel_planar_yuv : begin
 #pragma OPENCL EXTENSION cl_intel_planar_yuv : end
-#endif // __OPENCL_C_VERSION__ >= CL_VERSION_1_2
+#endif // defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ >= CL_VERSION_1_2)
 
 #define __ovld __attribute__((overloadable))
 #define __conv __attribute__((convergent))
@@ -6517,11 +6517,11 @@
  */
 size_t __ovld __cnfn get_global_offset(uint dimindx);
 
-#if __OPENCL_C_VERSION__ >= CL_VERSION_2_0
+#if defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ >= CL_VERSION_2_0)
 size_t __ovld get_enqueued_local_size(uint dimindx);
 size_t __ovld get_global_linear_id(void);
 size_t __ovld get_local_linear_id(void);
-#endif //__OPENCL_C_VERSION__ >= CL_VERSION_2_0
+#endif //defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ >= CL_VERSION_2_0)
 
 // OpenCL v1.1 s6.11.2, v1.2 s6.12.2, v2.0 s6.13.2 - Math functions
 
@@ -7352,7 +7352,7 @@
  * Returns fmin(x - floor (x), 0x1.fep-1f ).
  * floor(x) is returned in iptr.
  */
-#if __OPENCL_C_VERSION__ >= CL_VERSION_2_0
+#if defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ >= CL_VERSION_2_0)
 float __ovld fract(float x, float *iptr);
 float2 __ovld fract(float2 x, float2 *iptr);
 float3 __ovld fract(float3 x, float3 *iptr);
@@ -7434,7 +7434,7 @@
 half8 __ovld fract(half8 x, __private half8 *iptr);
 half16 __ovld fract(half16 x, __private half16 *iptr);
 #endif //cl_khr_fp16
-#endif //__OPENCL_C_VERSION__ >= CL_VERSION_2_0
+#endif //defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ >= CL_VERSION_2_0)
 
 /**
  * Extract mantissa and exponent from x. For each
@@ -7442,7 +7442,7 @@
  * magnitude in the interval [1/2, 1) or 0. Each
  * component of x equals mantissa returned * 2^exp.
  */
-#if __OPENCL_C_VERSION__ >= CL_VERSION_2_0
+#if defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ >= CL_VERSION_2_0)
 float __ovld frexp(float x, int *exp);
 float2 __ovld frexp(float2 x, int2 *exp);
 float3 __ovld frexp(float3 x, int3 *exp);
@@ -7524,7 +7524,7 @@
 half8 __ovld frexp(half8 x, __private int8 *exp);
 half16 __ovld frexp(half16 x, __private int16 *exp);
 #endif //cl_khr_fp16
-#endif //__OPENCL_C_VERSION__ >= CL_VERSION_2_0
+#endif //defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ >= CL_VERSION_2_0)
 
 /**
  * Compute the value of t

[PATCH] D66080: [OpenCL] Ignore parentheses for sampler initialization

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh created this revision.
svenvh added a reviewer: Anastasia.
Herald added subscribers: cfe-commits, yaxunl.
Herald added a project: clang.

The sampler handling logic in SemaInit.cpp would inadvertently treat
parentheses around sampler arguments as an implicit cast, leading to
an unreachable "can't implicitly cast lvalue to rvalue with
this cast kind".  Fix by ignoring parentheses once we are in the
sampler initializer case.


Repository:
  rC Clang

https://reviews.llvm.org/D66080

Files:
  clang/lib/Sema/SemaInit.cpp
  clang/test/SemaOpenCL/sampler_t.cl


Index: clang/test/SemaOpenCL/sampler_t.cl
===
--- clang/test/SemaOpenCL/sampler_t.cl
+++ clang/test/SemaOpenCL/sampler_t.cl
@@ -10,6 +10,9 @@
 #define CLK_FILTER_NEAREST  0x10
 #define CLK_FILTER_LINEAR   0x20
 
+typedef float float4 __attribute__((ext_vector_type(4)));
+float4 read_imagef(read_only image1d_t, sampler_t, float);
+
 constant sampler_t glb_smp = CLK_ADDRESS_CLAMP_TO_EDGE | 
CLK_NORMALIZED_COORDS_TRUE | CLK_FILTER_LINEAR;
 constant sampler_t glb_smp2; // expected-error{{variable in constant address 
space must be initialized}}
 global sampler_t glb_smp3 = CLK_ADDRESS_CLAMP_TO_EDGE | 
CLK_NORMALIZED_COORDS_TRUE | CLK_FILTER_NEAREST; // expected-error{{sampler 
type cannot be used with the __local and __global address space qualifiers}} 
expected-error {{global sampler requires a const or constant address space 
qualifier}}
@@ -74,3 +77,7 @@
   foo(smp1+1); //expected-error{{invalid operands to binary expression 
('sampler_t' and 'int')}}
 }
 
+void smp_args(read_only image1d_t image) {
+  // Test that parentheses around sampler arguments are ignored.
+  float4 res = read_imagef(image, (glb_smp10), 0.0f);
+}
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -8248,7 +8248,7 @@
   // argument passing.
   assert(Step->Type->isSamplerT() &&
  "Sampler initialization on non-sampler type.");
-  Expr *Init = CurInit.get();
+  Expr *Init = CurInit.get()->IgnoreParens();
   QualType SourceType = Init->getType();
   // Case 1
   if (Entity.isParameterKind()) {


Index: clang/test/SemaOpenCL/sampler_t.cl
===
--- clang/test/SemaOpenCL/sampler_t.cl
+++ clang/test/SemaOpenCL/sampler_t.cl
@@ -10,6 +10,9 @@
 #define CLK_FILTER_NEAREST  0x10
 #define CLK_FILTER_LINEAR   0x20
 
+typedef float float4 __attribute__((ext_vector_type(4)));
+float4 read_imagef(read_only image1d_t, sampler_t, float);
+
 constant sampler_t glb_smp = CLK_ADDRESS_CLAMP_TO_EDGE | CLK_NORMALIZED_COORDS_TRUE | CLK_FILTER_LINEAR;
 constant sampler_t glb_smp2; // expected-error{{variable in constant address space must be initialized}}
 global sampler_t glb_smp3 = CLK_ADDRESS_CLAMP_TO_EDGE | CLK_NORMALIZED_COORDS_TRUE | CLK_FILTER_NEAREST; // expected-error{{sampler type cannot be used with the __local and __global address space qualifiers}} expected-error {{global sampler requires a const or constant address space qualifier}}
@@ -74,3 +77,7 @@
   foo(smp1+1); //expected-error{{invalid operands to binary expression ('sampler_t' and 'int')}}
 }
 
+void smp_args(read_only image1d_t image) {
+  // Test that parentheses around sampler arguments are ignored.
+  float4 res = read_imagef(image, (glb_smp10), 0.0f);
+}
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -8248,7 +8248,7 @@
   // argument passing.
   assert(Step->Type->isSamplerT() &&
  "Sampler initialization on non-sampler type.");
-  Expr *Init = CurInit.get();
+  Expr *Init = CurInit.get()->IgnoreParens();
   QualType SourceType = Init->getType();
   // Case 1
   if (Entity.isParameterKind()) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

In D66042#1624697 , @Charusso wrote:

> In D66042#1624684 , @NoQ wrote:
>
> > My idea here was that this new feature isn't going to be user-facing. We 
> > aren't promising to support all combinations of 
> > enabled-disabled-silenced-dependent-registercheckerhacks, but instead use 
> > the new feature when we know it'll do exactly what we want. It is going to 
> > be up to the user-facing UI to decide how to use this feature, but not up 
> > to the end-users who simply want to silence diagnostics.
> >
> > > Here is a problem with your patch: How would you go about silencing 
> > > diagnostics for `osx.cocoa.RetainCount`? I suppose 
> > > `-analyzer-silence-checker=osx.cocoa.RetainCount`. The problem however, 
> > > that the checker tag associated with it refers to 
> > > `osx.cocoa.RetainCountBase` under the hood, so you'll need to silence 
> > > that instead. From that point on, any other checker that suffers from the 
> > > same issue is also silenced, that was not the intent.
> >
> > Hmm, sounds like we need to hack up a fix for the checker tag on the bug 
> > node, so that it was appropriate to the presumed (as opposed to actual) 
> > checker name(?)
>
>
> `StringRef("osx.cocoa.RetainCountBase").startswith("osx.cocoa.RetainCount")` 
> is true, so there is no real issue until we manage the prefixes well. I 
> assume that the user who knows how to disable/silence a checker, knows where 
> to read how to disable/silence it. At least with scan-build there will not be 
> pitfalls with disabling the core modeling.




> scan-build
>  core modeling

This patch affects far more then either of those items. To me, it seems like 
we're trying to solve (a subset) of the checker dependency problem again with 
more hacking, and it didn't turn out too well last time.

In D66042#1624684 , @NoQ wrote:

> My idea here was that this new feature isn't going to be user-facing. We 
> aren't promising to support all combinations of 
> enabled-disabled-silenced-dependent-registercheckerhacks, but instead use the 
> new feature when we know it'll do exactly what we want. It is going to be up 
> to the user-facing UI to decide how to use this feature, but not up to the 
> end-users who simply want to silence diagnostics.


The frontend is our user interface, and until we develop a nicely thought out 
way to interact with the analyzer through the driver, it'll stay that way, 
unfortunately. The only way to use the analyzer is through the frontend 
(including `-Xclang`), and if we add this flag, people responsible for 
developing interafaces for the analyzer might end up using it. We don't promise 
anything, because we don't really submit release notes, so I don't see this as 
a good enough defense.

Please note how we restrained ourselves from adding checker plugins into the 
`examples` folder, we were so concerned with people discovering it. As you said 
in D57858#1498640 , and I mentioned 
again in D62885#1530206 , it is an 
unrealistic expectation that this will be hidden from view.

>> Here is a problem with your patch: How would you go about silencing 
>> diagnostics for `osx.cocoa.RetainCount`? I suppose 
>> `-analyzer-silence-checker=osx.cocoa.RetainCount`. The problem however, that 
>> the checker tag associated with it refers to `osx.cocoa.RetainCountBase` 
>> under the hood, so you'll need to silence that instead. From that point on, 
>> any other checker that suffers from the same issue is also silenced, that 
>> was not the intent.
> 
> Hmm, sounds like we need to hack up a fix for the checker tag on the bug 
> node, so that it was appropriate to the presumed (as opposed to actual) 
> checker name(?)

I wouldn't call it hacking (we would need just a couple 
`CheckerProgramPointTag`s), but again, what if we forget about it? If we fix 
this (add tags for each checker that suffers from this issue) we're literally 
10 lines of code away from a far better solution after which silencing checkers 
wouldn't be needed.

So, here are the things I'm worried about:

- Whenever we forget about adding a checker tag to a subchecker, this issue 
will arise again. We could test this by generating a testcase for each checker 
in which said checker is silenced, modify `-analyzer-list-enabled-checkers` so 
that it also displays whether the checker is silenced (and since some users 
might depend on this output, we might need an 
`-analyzer-list-silenced-checkers` flag that will add boilerplate code), and we 
check with whether everything works as intended.
- No checker depends on another checkers diagnostics. If this is how its 
represented in the implementation, thats a checker dependency issue that has to 
be solved with the existing interface, which will automatically grant us with 
all the benefits that 

[PATCH] D65752: [Sema] Refactor LookupVisibleDecls. NFC

[Sam McCall via Phabricator via cfe-commits]

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

Agreed, this seems fine as is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65752



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

[PATCH] D65591: [AST] Add a flag indicating if any subexpression had errors

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Gentle ping. @rsmith, please take a look when you have time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65591



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

[PATCH] D65591: [AST] Add a flag indicating if any subexpression had errors

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Rather than adding a bit onto `Expr` to specify whether it's erroneous, have 
you considered making this a property of the type system by introducing an 
`ErrorExpr` AST node that other nodes can inherit from? I think that approach 
would work more naturally for things like AST matchers while solving some 
problems we have with being able to pretty printing or AST dump erroneous ASTs. 
The `ErrorExpr` node could contain a subnode of a partially-valid `Expr` object 
that would retain as much of the information as we've been able to gather, but 
the error node (or its subclasses) could contain other information useful when 
handling errors, such as storing the original source text (or range) for the 
expression, potential fixes, etc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65591



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

[PATCH] D66080: [OpenCL] Ignore parentheses for sampler initialization

[Anastasia Stulova via Phabricator via cfe-commits]

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

LGTM! Thanks!


Repository:
  rC Clang

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

https://reviews.llvm.org/D66080



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

[PATCH] D65998: [clangd] Added the vscode SemanticHighlighting feature code but did not enable it in the client.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: 
clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts:8
+// The information clangd sends when highlightings should be updated.
+interface SemanticHighlightingParams {
+  // The information about the text document where these highlightings should 
be

sorry for not being clear in the previous comment, I think we should mirror 
definitions from the LSP 
[proposal](https://github.com/microsoft/vscode-languageserver-node/pull/367/files#diff-ac2186ae4b8ba5cc9f17deebc42c9e28R13),
 the same to the `SemanticHighlightingInformation`.

You may need to add a `vscode-languageserver-types` dependency.



Comment at: 
clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts:32
+
+export const NotificationType = new vscodelc.NotificationType<{}, void>(
+'textDocument/semanticHighlighting');

I think here we should use `SemanticHighlightingParams` in the generic type 
parameter list



Comment at: 
clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts:38
+export class SemanticHighlightingFeature implements vscodelc.StaticFeature {
+  scopeLookupTable: string[][];
+  fillClientCapabilities(capabilities: vscodelc.ClientCapabilities) {

could you add a comment on this?



Comment at: 
clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts:63
+  handleNotification(params: SemanticHighlightingParams) {
+const tokenLines =
+params.lines.map((line): SemanticHighlightingInformation => {

this seems doesn't do anything, can we defer it to a follow-up patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65998



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

[PATCH] D66083: [clangd] Remove highlightings coming from non topLevelDecls from included files.

[Johan Vikström via Phabricator via cfe-commits]

jvikstrom created this revision.
jvikstrom added reviewers: hokein, ilya-biryukov.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay.
Herald added a project: clang.

It is possible to write include code from other files so that the decls from 
there do not become topLevelDecls (For example by including methods for a 
class). These Decls are not filtered by topLevelDecls and therefore 
SemanticHighlighting must manually check that every SLoc belongs in the main 
file. Otherwise there can be highlightings appearing in places where they 
should not.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66083

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -51,9 +51,16 @@
   return ExpectedTokens;
 }
 
-void checkHighlightings(llvm::StringRef Code) {
+// AdditionalFiles are key-value-pair that are included in the TU where the key
+// is the filename and the value is the code for the file.
+void checkHighlightings(llvm::StringRef Code,
+std::vector>
+AdditionalFiles = {}) {
   Annotations Test(Code);
-  auto AST = TestTU::withCode(Test.code()).build();
+  auto TU = TestTU::withCode(Test.code());
+  for (auto File : AdditionalFiles)
+TU.AdditionalFiles.insert({File.first, File.second});
+  auto AST = TU.build();
   std::vector ActualTokens = getSemanticHighlightings(AST);
   EXPECT_THAT(ActualTokens, getExpectedTokens(Test)) << Code;
 }
@@ -321,6 +328,17 @@
   for (const auto &TestCase : TestCases) {
 checkHighlightings(TestCase);
   }
+
+  checkHighlightings(R"cpp(
+class $Class[[A]] {
+  #include "imp.h"
+};
+#endif
+  )cpp",
+ {{"imp.h", R"cpp(
+int someMethod();
+void otherMethod();
+  )cpp"}});
 }
 
 TEST(SemanticHighlighting, GeneratesHighlightsWhenFileChange) {
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -231,6 +231,12 @@
   // FIXME: skip tokens inside macros for now.
   return;
 
+// Non top level decls that are included from a header are not filtered by
+// topLevelDecls. (example: method declarations being included from another
+// file for a class) from another file)
+if (!SM.isWrittenInMainFile(Loc))
+  return;
+
 auto R = getTokenRange(SM, Ctx.getLangOpts(), Loc);
 if (!R) {
   // R should always have a value, if it doesn't something is very wrong.


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -51,9 +51,16 @@
   return ExpectedTokens;
 }
 
-void checkHighlightings(llvm::StringRef Code) {
+// AdditionalFiles are key-value-pair that are included in the TU where the key
+// is the filename and the value is the code for the file.
+void checkHighlightings(llvm::StringRef Code,
+std::vector>
+AdditionalFiles = {}) {
   Annotations Test(Code);
-  auto AST = TestTU::withCode(Test.code()).build();
+  auto TU = TestTU::withCode(Test.code());
+  for (auto File : AdditionalFiles)
+TU.AdditionalFiles.insert({File.first, File.second});
+  auto AST = TU.build();
   std::vector ActualTokens = getSemanticHighlightings(AST);
   EXPECT_THAT(ActualTokens, getExpectedTokens(Test)) << Code;
 }
@@ -321,6 +328,17 @@
   for (const auto &TestCase : TestCases) {
 checkHighlightings(TestCase);
   }
+
+  checkHighlightings(R"cpp(
+class $Class[[A]] {
+  #include "imp.h"
+};
+#endif
+  )cpp",
+ {{"imp.h", R"cpp(
+int someMethod();
+void otherMethod();
+  )cpp"}});
 }
 
 TEST(SemanticHighlighting, GeneratesHighlightsWhenFileChange) {
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -231,6 +231,12 @@
   // FIXME: skip tokens inside macros for now.
   return;
 
+// Non top level decls that are included from a header are not filtered by
+// topLevelDecls. (example: method declarations being included from another
+// file for a class) from another file)
+if (!SM.isWrittenInMainFile(Loc))
+  return;
+
 auto R = getTokenRange(SM, Ctx.getLangOpts(), Loc);
 if 

[PATCH] D66083: [clangd] Remove highlightings coming from non topLevelDecls from included files.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM. Great example for the test case! It will definitely stay valid even if we 
fix all problems caused by RecursiveASTVisitor!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66083



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

[PATCH] D66083: [clangd] Remove highlightings coming from non topLevelDecls from included files.

[Haojian Wu via Phabricator via cfe-commits]

hokein accepted this revision.
hokein added inline comments.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:236
+// topLevelDecls. (example: method declarations being included from another
+// file for a class) from another file)
+if (!SM.isWrittenInMainFile(Loc))

nit: the `)` is not match.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:237
+// file for a class) from another file)
+if (!SM.isWrittenInMainFile(Loc))
+  return;

nit: clangd has its own `isInsideMainFile`, please use it.



Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:57
+void checkHighlightings(llvm::StringRef Code,
+std::vector>
+AdditionalFiles = {}) {

nit: maybe just `std::vector>`, it is clearer and would save you a long 
comment above.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66083



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

[PATCH] D65373: [clangd] Update features table in the docs with links to LSP extension proposals

[Sam McCall via Phabricator via cfe-commits]

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

Sorry, I had marked this as "accepted", but not hit send. I agree with Ilya's 
comments, but please go ahead and land this when ready.

(I wish Phabricator didn't say "sammccall accepted this revision" when it's not 
true - I even went back to check that I'd accepted it)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65373



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

r368561 - [OpenCL] Ignore parentheses for sampler initialization

[Sven van Haastregt via cfe-commits]

Author: svenvh
Date: Mon Aug 12 05:44:26 2019
New Revision: 368561

URL: http://llvm.org/viewvc/llvm-project?rev=368561&view=rev
Log:
[OpenCL] Ignore parentheses for sampler initialization

The sampler handling logic in SemaInit.cpp would inadvertently treat
parentheses around sampler arguments as an implicit cast, leading to
an unreachable "can't implicitly cast lvalue to rvalue with
this cast kind".  Fix by ignoring parentheses once we are in the
sampler initializer case.

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

Modified:
cfe/trunk/lib/Sema/SemaInit.cpp
cfe/trunk/test/SemaOpenCL/sampler_t.cl

Modified: cfe/trunk/lib/Sema/SemaInit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=368561&r1=368560&r2=368561&view=diff
==
--- cfe/trunk/lib/Sema/SemaInit.cpp (original)
+++ cfe/trunk/lib/Sema/SemaInit.cpp Mon Aug 12 05:44:26 2019
@@ -8248,7 +8248,7 @@ ExprResult InitializationSequence::Perfo
   // argument passing.
   assert(Step->Type->isSamplerT() &&
  "Sampler initialization on non-sampler type.");
-  Expr *Init = CurInit.get();
+  Expr *Init = CurInit.get()->IgnoreParens();
   QualType SourceType = Init->getType();
   // Case 1
   if (Entity.isParameterKind()) {

Modified: cfe/trunk/test/SemaOpenCL/sampler_t.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaOpenCL/sampler_t.cl?rev=368561&r1=368560&r2=368561&view=diff
==
--- cfe/trunk/test/SemaOpenCL/sampler_t.cl (original)
+++ cfe/trunk/test/SemaOpenCL/sampler_t.cl Mon Aug 12 05:44:26 2019
@@ -10,6 +10,9 @@
 #define CLK_FILTER_NEAREST  0x10
 #define CLK_FILTER_LINEAR   0x20
 
+typedef float float4 __attribute__((ext_vector_type(4)));
+float4 read_imagef(read_only image1d_t, sampler_t, float);
+
 constant sampler_t glb_smp = CLK_ADDRESS_CLAMP_TO_EDGE | 
CLK_NORMALIZED_COORDS_TRUE | CLK_FILTER_LINEAR;
 constant sampler_t glb_smp2; // expected-error{{variable in constant address 
space must be initialized}}
 global sampler_t glb_smp3 = CLK_ADDRESS_CLAMP_TO_EDGE | 
CLK_NORMALIZED_COORDS_TRUE | CLK_FILTER_NEAREST; // expected-error{{sampler 
type cannot be used with the __local and __global address space qualifiers}} 
expected-error {{global sampler requires a const or constant address space 
qualifier}}
@@ -74,3 +77,7 @@ void bar() {
   foo(smp1+1); //expected-error{{invalid operands to binary expression 
('sampler_t' and 'int')}}
 }
 
+void smp_args(read_only image1d_t image) {
+  // Test that parentheses around sampler arguments are ignored.
+  float4 res = read_imagef(image, (glb_smp10), 0.0f);
+}


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

[PATCH] D66080: [OpenCL] Ignore parentheses for sampler initialization

[Sven van Haastregt via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL368561: [OpenCL] Ignore parentheses for sampler 
initialization (authored by svenvh, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66080?vs=214602&id=214611#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66080

Files:
  cfe/trunk/lib/Sema/SemaInit.cpp
  cfe/trunk/test/SemaOpenCL/sampler_t.cl


Index: cfe/trunk/test/SemaOpenCL/sampler_t.cl
===
--- cfe/trunk/test/SemaOpenCL/sampler_t.cl
+++ cfe/trunk/test/SemaOpenCL/sampler_t.cl
@@ -10,6 +10,9 @@
 #define CLK_FILTER_NEAREST  0x10
 #define CLK_FILTER_LINEAR   0x20
 
+typedef float float4 __attribute__((ext_vector_type(4)));
+float4 read_imagef(read_only image1d_t, sampler_t, float);
+
 constant sampler_t glb_smp = CLK_ADDRESS_CLAMP_TO_EDGE | 
CLK_NORMALIZED_COORDS_TRUE | CLK_FILTER_LINEAR;
 constant sampler_t glb_smp2; // expected-error{{variable in constant address 
space must be initialized}}
 global sampler_t glb_smp3 = CLK_ADDRESS_CLAMP_TO_EDGE | 
CLK_NORMALIZED_COORDS_TRUE | CLK_FILTER_NEAREST; // expected-error{{sampler 
type cannot be used with the __local and __global address space qualifiers}} 
expected-error {{global sampler requires a const or constant address space 
qualifier}}
@@ -74,3 +77,7 @@
   foo(smp1+1); //expected-error{{invalid operands to binary expression 
('sampler_t' and 'int')}}
 }
 
+void smp_args(read_only image1d_t image) {
+  // Test that parentheses around sampler arguments are ignored.
+  float4 res = read_imagef(image, (glb_smp10), 0.0f);
+}
Index: cfe/trunk/lib/Sema/SemaInit.cpp
===
--- cfe/trunk/lib/Sema/SemaInit.cpp
+++ cfe/trunk/lib/Sema/SemaInit.cpp
@@ -8248,7 +8248,7 @@
   // argument passing.
   assert(Step->Type->isSamplerT() &&
  "Sampler initialization on non-sampler type.");
-  Expr *Init = CurInit.get();
+  Expr *Init = CurInit.get()->IgnoreParens();
   QualType SourceType = Init->getType();
   // Case 1
   if (Entity.isParameterKind()) {


Index: cfe/trunk/test/SemaOpenCL/sampler_t.cl
===
--- cfe/trunk/test/SemaOpenCL/sampler_t.cl
+++ cfe/trunk/test/SemaOpenCL/sampler_t.cl
@@ -10,6 +10,9 @@
 #define CLK_FILTER_NEAREST  0x10
 #define CLK_FILTER_LINEAR   0x20
 
+typedef float float4 __attribute__((ext_vector_type(4)));
+float4 read_imagef(read_only image1d_t, sampler_t, float);
+
 constant sampler_t glb_smp = CLK_ADDRESS_CLAMP_TO_EDGE | CLK_NORMALIZED_COORDS_TRUE | CLK_FILTER_LINEAR;
 constant sampler_t glb_smp2; // expected-error{{variable in constant address space must be initialized}}
 global sampler_t glb_smp3 = CLK_ADDRESS_CLAMP_TO_EDGE | CLK_NORMALIZED_COORDS_TRUE | CLK_FILTER_NEAREST; // expected-error{{sampler type cannot be used with the __local and __global address space qualifiers}} expected-error {{global sampler requires a const or constant address space qualifier}}
@@ -74,3 +77,7 @@
   foo(smp1+1); //expected-error{{invalid operands to binary expression ('sampler_t' and 'int')}}
 }
 
+void smp_args(read_only image1d_t image) {
+  // Test that parentheses around sampler arguments are ignored.
+  float4 res = read_imagef(image, (glb_smp10), 0.0f);
+}
Index: cfe/trunk/lib/Sema/SemaInit.cpp
===
--- cfe/trunk/lib/Sema/SemaInit.cpp
+++ cfe/trunk/lib/Sema/SemaInit.cpp
@@ -8248,7 +8248,7 @@
   // argument passing.
   assert(Step->Type->isSamplerT() &&
  "Sampler initialization on non-sampler type.");
-  Expr *Init = CurInit.get();
+  Expr *Init = CurInit.get()->IgnoreParens();
   QualType SourceType = Init->getType();
   // Case 1
   if (Entity.isParameterKind()) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r368562 - [CrossTU] User docs: remove temporary limiation with macro expansion

[Gabor Marton via cfe-commits]

Author: martong
Date: Mon Aug 12 05:46:28 2019
New Revision: 368562

URL: http://llvm.org/viewvc/llvm-project?rev=368562&view=rev
Log:
[CrossTU] User docs: remove temporary limiation with macro expansion

D65064, D64635, D64638 pathces solve the issue with macor expansion.

Modified:
cfe/trunk/docs/analyzer/user-docs/CrossTranslationUnit.rst

Modified: cfe/trunk/docs/analyzer/user-docs/CrossTranslationUnit.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/analyzer/user-docs/CrossTranslationUnit.rst?rev=368562&r1=368561&r2=368562&view=diff
==
--- cfe/trunk/docs/analyzer/user-docs/CrossTranslationUnit.rst (original)
+++ cfe/trunk/docs/analyzer/user-docs/CrossTranslationUnit.rst Mon Aug 12 
05:46:28 2019
@@ -173,15 +173,6 @@ Or we can use `CodeChecker parse -e html
   $ CodeChecker parse -e html -o html_out reports
   $ firefox html_out/index.html
 
-If you experience that Clang crashes during the visitation of the BugPath then 
please disable macro expansion when CTU is used:
-
-.. code-block:: bash
-
-  $ echo "-Xclang -analyzer-stats -Xclang -analyzer-config -Xclang 
expand-macros=false" >./saargs_file
-  $ CodeChecker analyze --ctu --saargs ./saargs_file compile_commands.json -o 
reports
-
-We have a patch which will solve this issue soon.
-
 Automated CTU Analysis with scan-build-py (don't do it)
 ---
 We actively develop CTU with CodeChecker as a "runner" script, `scan-build-py` 
is not actively developed for CTU.


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

[PATCH] D65998: [clangd] Added the vscode SemanticHighlighting feature code but did not enable it in the client.

[Johan Vikström via Phabricator via cfe-commits]

jvikstrom updated this revision to Diff 214613.
jvikstrom marked 4 inline comments as done.
jvikstrom added a comment.

Mirror the LSP proposal SemanticHighlightingParams/Information types.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65998

Files:
  clang-tools-extra/clangd/clients/clangd-vscode/package.json
  clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
  
clang-tools-extra/clangd/clients/clangd-vscode/test/semantic-highlighting.test.ts

Index: clang-tools-extra/clangd/clients/clangd-vscode/test/semantic-highlighting.test.ts
===
--- clang-tools-extra/clangd/clients/clangd-vscode/test/semantic-highlighting.test.ts
+++ clang-tools-extra/clangd/clients/clangd-vscode/test/semantic-highlighting.test.ts
@@ -3,7 +3,7 @@
 
 import * as TM from '../src/semantic-highlighting';
 
-suite('TextMate Tests', () => {
+suite('SemanticHighlighting Tests', () => {
   test('Parses arrays of textmate themes.', async () => {
 const themePath =
 path.join(__dirname, '../../test/assets/includeTheme.jsonc');
@@ -15,4 +15,24 @@
 assert.deepEqual(getScopeRule('b'), {scope : 'b', textColor : '#000'});
 assert.deepEqual(getScopeRule('c'), {scope : 'c', textColor : '#bcd'});
   });
+  test('Decodes tokens correctly', () => {
+const testCases: string[] = [
+  'AAABAAA=', 'AAADAAkEAAEAAA==',
+  'AAADAAkEAAEAAAoAAQAA'
+];
+const expected = [
+  [ {character : 0, scope : 0, length : 1} ],
+  [
+{character : 0, scope : 9, length : 3},
+{character : 4, scope : 0, length : 1}
+  ],
+  [
+{character : 0, scope : 9, length : 3},
+{character : 4, scope : 0, length : 1},
+{character : 10, scope : 0, length : 1}
+  ]
+];
+testCases.forEach((testCase, i) => assert.deepEqual(
+  TM.decodeTokens(testCase), expected[i]));
+  });
 });
Index: clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
===
--- clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
+++ clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
@@ -2,6 +2,94 @@
 import * as jsonc from "jsonc-parser";
 import * as path from 'path';
 import * as vscode from 'vscode';
+import * as vscodelc from 'vscode-languageclient';
+import * as vscodelct from 'vscode-languageserver-types';
+
+// Parameters for the semantic highlighting (server-side) push notification.
+interface SemanticHighlightingParams {
+  // The text document that has to be decorated with the semantic highlighting
+  // information.
+  textDocument: vscodelct.VersionedTextDocumentIdentifier;
+  // An array of semantic highlighting information.
+  lines: SemanticHighlightingInformation[];
+}
+
+// Contains the highlighting information for a specified line.
+interface SemanticHighlightingInformation {
+  // The zero-based line position in the text document.
+  line: number;
+  // A base64 encoded string representing every single highlighted characters
+  // with its start position, length and the "lookup table" index of of the
+  // semantic highlighting Text Mate scopes.
+  tokens?: string;
+}
+
+// A single SemanticHighlightingToken that clangd sent.
+interface SemanticHighlightingToken {
+  // Start column for this token.
+  character: number;
+  // Length of the token.
+  length: number;
+  // The TextMate scope index to the clangd scopes.
+  scope: number;
+}
+
+// Language server push notification providing the semantic highlighting
+// information for a text document.
+export const NotificationType =
+new vscodelc.NotificationType(
+'textDocument/semanticHighlighting');
+
+// The feature that should be registered in the vscode lsp for enabling
+// experimental semantic highlighting.
+export class SemanticHighlightingFeature implements vscodelc.StaticFeature {
+  // The TextMate scope lookup table. A token with scope index i has the scopes
+  // on index i in the lookup table.
+  scopeLookupTable: string[][];
+  fillClientCapabilities(capabilities: vscodelc.ClientCapabilities) {
+// Extend the ClientCapabilities type and add semantic highlighting
+// capability to the object.
+const textDocumentCapabilities: vscodelc.TextDocumentClientCapabilities&
+{semanticHighlightingCapabilities?: {semanticHighlighting : boolean}} =
+capabilities.textDocument;
+textDocumentCapabilities.semanticHighlightingCapabilities = {
+  semanticHighlighting : true,
+};
+  }
+
+  initialize(capabilities: vscodelc.ServerCapabilities,
+ documentSelector: vscodelc.DocumentSelector|undefined) {
+// The semantic highlighting capability information is in the capabilities
+// object but to access the data we must first exten

[clang-tools-extra] r368563 - [clangd] Remove highlightings coming from non topLevelDecls from included files.

[Johan Vikstrom via cfe-commits]

Author: jvikstrom
Date: Mon Aug 12 06:01:11 2019
New Revision: 368563

URL: http://llvm.org/viewvc/llvm-project?rev=368563&view=rev
Log:
[clangd] Remove highlightings coming from non topLevelDecls from included files.

Summary: It is possible to write include code from other files so that the 
decls from there do not become topLevelDecls (For example by including methods 
for a class). These Decls are not filtered by topLevelDecls and therefore 
SemanticHighlighting must manually check that every SLoc belongs in the main 
file. Otherwise there can be highlightings appearing in places where they 
should not.

Reviewers: hokein, ilya-biryukov

Subscribers: MaskRay, jkorous, arphaman, kadircet, cfe-commits

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp

Modified: clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp?rev=368563&r1=368562&r2=368563&view=diff
==
--- clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp (original)
+++ clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp Mon Aug 12 06:01:11 
2019
@@ -231,6 +231,12 @@ private:
   // FIXME: skip tokens inside macros for now.
   return;
 
+// Non top level decls that are included from a header are not filtered by
+// topLevelDecls. (example: method declarations being included from another
+// file for a class from another file)
+if (!isInsideMainFile(Loc, SM))
+  return;
+
 auto R = getTokenRange(SM, Ctx.getLangOpts(), Loc);
 if (!R) {
   // R should always have a value, if it doesn't something is very wrong.

Modified: clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp?rev=368563&r1=368562&r2=368563&view=diff
==
--- clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp 
(original)
+++ clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp Mon 
Aug 12 06:01:11 2019
@@ -51,9 +51,15 @@ std::vector getExpect
   return ExpectedTokens;
 }
 
-void checkHighlightings(llvm::StringRef Code) {
+void checkHighlightings(llvm::StringRef Code,
+std::vector>
+AdditionalFiles = {}) {
   Annotations Test(Code);
-  auto AST = TestTU::withCode(Test.code()).build();
+  auto TU = TestTU::withCode(Test.code());
+  for (auto File : AdditionalFiles)
+TU.AdditionalFiles.insert({File.first, File.second});
+  auto AST = TU.build();
   std::vector ActualTokens = getSemanticHighlightings(AST);
   EXPECT_THAT(ActualTokens, getExpectedTokens(Test)) << Code;
 }
@@ -321,6 +327,17 @@ TEST(SemanticHighlighting, GetsCorrectTo
   for (const auto &TestCase : TestCases) {
 checkHighlightings(TestCase);
   }
+
+  checkHighlightings(R"cpp(
+class $Class[[A]] {
+  #include "imp.h"
+};
+#endif
+  )cpp",
+ {{"imp.h", R"cpp(
+int someMethod();
+void otherMethod();
+  )cpp"}});
 }
 
 TEST(SemanticHighlighting, GeneratesHighlightsWhenFileChange) {


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

[PATCH] D66083: [clangd] Remove highlightings coming from non topLevelDecls from included files.

[Johan Vikström via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
jvikstrom marked 3 inline comments as done.
Closed by commit rL368563: [clangd] Remove highlightings coming from non 
topLevelDecls from included files. (authored by jvikstrom, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66083?vs=214608&id=214616#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66083

Files:
  clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
  clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp


Index: clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
@@ -231,6 +231,12 @@
   // FIXME: skip tokens inside macros for now.
   return;
 
+// Non top level decls that are included from a header are not filtered by
+// topLevelDecls. (example: method declarations being included from another
+// file for a class from another file)
+if (!isInsideMainFile(Loc, SM))
+  return;
+
 auto R = getTokenRange(SM, Ctx.getLangOpts(), Loc);
 if (!R) {
   // R should always have a value, if it doesn't something is very wrong.
Index: clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp
@@ -51,9 +51,15 @@
   return ExpectedTokens;
 }
 
-void checkHighlightings(llvm::StringRef Code) {
+void checkHighlightings(llvm::StringRef Code,
+std::vector>
+AdditionalFiles = {}) {
   Annotations Test(Code);
-  auto AST = TestTU::withCode(Test.code()).build();
+  auto TU = TestTU::withCode(Test.code());
+  for (auto File : AdditionalFiles)
+TU.AdditionalFiles.insert({File.first, File.second});
+  auto AST = TU.build();
   std::vector ActualTokens = getSemanticHighlightings(AST);
   EXPECT_THAT(ActualTokens, getExpectedTokens(Test)) << Code;
 }
@@ -321,6 +327,17 @@
   for (const auto &TestCase : TestCases) {
 checkHighlightings(TestCase);
   }
+
+  checkHighlightings(R"cpp(
+class $Class[[A]] {
+  #include "imp.h"
+};
+#endif
+  )cpp",
+ {{"imp.h", R"cpp(
+int someMethod();
+void otherMethod();
+  )cpp"}});
 }
 
 TEST(SemanticHighlighting, GeneratesHighlightsWhenFileChange) {


Index: clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
@@ -231,6 +231,12 @@
   // FIXME: skip tokens inside macros for now.
   return;
 
+// Non top level decls that are included from a header are not filtered by
+// topLevelDecls. (example: method declarations being included from another
+// file for a class from another file)
+if (!isInsideMainFile(Loc, SM))
+  return;
+
 auto R = getTokenRange(SM, Ctx.getLangOpts(), Loc);
 if (!R) {
   // R should always have a value, if it doesn't something is very wrong.
Index: clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp
@@ -51,9 +51,15 @@
   return ExpectedTokens;
 }
 
-void checkHighlightings(llvm::StringRef Code) {
+void checkHighlightings(llvm::StringRef Code,
+std::vector>
+AdditionalFiles = {}) {
   Annotations Test(Code);
-  auto AST = TestTU::withCode(Test.code()).build();
+  auto TU = TestTU::withCode(Test.code());
+  for (auto File : AdditionalFiles)
+TU.AdditionalFiles.insert({File.first, File.second});
+  auto AST = TU.build();
   std::vector ActualTokens = getSemanticHighlightings(AST);
   EXPECT_THAT(ActualTokens, getExpectedTokens(Test)) << Code;
 }
@@ -321,6 +327,17 @@
   for (const auto &TestCase : TestCases) {
 checkHighlightings(TestCase);
   }
+
+  checkHighlightings(R"cpp(
+class $Class[[A]] {
+  #include "imp.h"
+};
+#endif
+  )cpp",
+ {{"imp.h", R"cpp(
+int someMethod();
+void otherMethod();
+  )cpp"}});
 }
 
 TEST(SemanticHighlighting, GeneratesHighlightsWhenFileChange) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66086: [clangd] Separate chunks with a space when rendering markdown

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added a reviewer: sammccall.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay.
Herald added a project: clang.

This results in better rendering of resulting markdown.

Especially noticeable in coc.nvim that does not have a visible horizontal
spaces around inline code blocks. More details and a screenshot from
coc.nvim can be found in https://github.com/clangd/clangd/issues/95.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66086

Files:
  clang-tools-extra/clangd/FormattedString.cpp
  clang-tools-extra/clangd/unittests/FormattedStringTests.cpp


Index: clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
===
--- clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
+++ clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
@@ -72,7 +72,7 @@
   S.appendText("baz");
 
   EXPECT_EQ(S.renderAsPlainText(), "foo bar baz");
-  EXPECT_EQ(S.renderAsMarkdown(), "foo`bar`baz");
+  EXPECT_EQ(S.renderAsMarkdown(), "foo `bar` baz");
 }
 
 TEST(FormattedString, Escaping) {
@@ -158,6 +158,42 @@
   "`\n");
 }
 
+TEST(FormattedString, MarkdownWhitespace) {
+  // Whitespace should be added as separators between blocks.
+  FormattedString S;
+  S.appendText("foo");
+  S.appendText("bar");
+  EXPECT_EQ(S.renderAsMarkdown(), "foo bar");
+
+  S = FormattedString();
+  S.appendInlineCode("foo");
+  S.appendInlineCode("bar");
+  EXPECT_EQ(S.renderAsMarkdown(), "`foo` `bar`");
+
+  // However, we don't want to add any extra whitespace.
+  S = FormattedString();
+  S.appendText("foo ");
+  S.appendInlineCode("bar");
+  EXPECT_EQ(S.renderAsMarkdown(), "foo `bar`");
+
+  S = FormattedString();
+  S.appendText("foo\n");
+  S.appendInlineCode("bar");
+  EXPECT_EQ(S.renderAsMarkdown(), "foo\n`bar`");
+
+  S = FormattedString();
+  S.appendInlineCode("foo");
+  S.appendText(" bar");
+  EXPECT_EQ(S.renderAsMarkdown(), "`foo` bar");
+
+  S = FormattedString();
+  S.appendText("foo");
+  S.appendCodeBlock("bar");
+  S.appendText("baz");
+  EXPECT_EQ(S.renderAsMarkdown(), "foo\n```cpp\nbar\n```\nbaz");
+}
+
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/FormattedString.cpp
===
--- clang-tools-extra/clangd/FormattedString.cpp
+++ clang-tools-extra/clangd/FormattedString.cpp
@@ -112,15 +112,20 @@
 
 std::string FormattedString::renderAsMarkdown() const {
   std::string R;
+  auto EnsureWhitespace = [&R]() {
+// Adds a space for nicer rendering.
+if (!R.empty() && !isWhitespace(R.back()))
+  R += " ";
+  };
   for (const auto &C : Chunks) {
 switch (C.Kind) {
 case ChunkKind::PlainText:
+  if (!C.Contents.empty() && !isWhitespace(C.Contents.front()))
+EnsureWhitespace();
   R += renderText(C.Contents);
   continue;
 case ChunkKind::InlineCodeBlock:
-  // Make sure we don't glue two backticks together.
-  if (llvm::StringRef(R).endswith("`"))
-R += " ";
+  EnsureWhitespace();
   R += renderInlineBlock(C.Contents);
   continue;
 case ChunkKind::CodeBlock:


Index: clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
===
--- clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
+++ clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
@@ -72,7 +72,7 @@
   S.appendText("baz");
 
   EXPECT_EQ(S.renderAsPlainText(), "foo bar baz");
-  EXPECT_EQ(S.renderAsMarkdown(), "foo`bar`baz");
+  EXPECT_EQ(S.renderAsMarkdown(), "foo `bar` baz");
 }
 
 TEST(FormattedString, Escaping) {
@@ -158,6 +158,42 @@
   "`\n");
 }
 
+TEST(FormattedString, MarkdownWhitespace) {
+  // Whitespace should be added as separators between blocks.
+  FormattedString S;
+  S.appendText("foo");
+  S.appendText("bar");
+  EXPECT_EQ(S.renderAsMarkdown(), "foo bar");
+
+  S = FormattedString();
+  S.appendInlineCode("foo");
+  S.appendInlineCode("bar");
+  EXPECT_EQ(S.renderAsMarkdown(), "`foo` `bar`");
+
+  // However, we don't want to add any extra whitespace.
+  S = FormattedString();
+  S.appendText("foo ");
+  S.appendInlineCode("bar");
+  EXPECT_EQ(S.renderAsMarkdown(), "foo `bar`");
+
+  S = FormattedString();
+  S.appendText("foo\n");
+  S.appendInlineCode("bar");
+  EXPECT_EQ(S.renderAsMarkdown(), "foo\n`bar`");
+
+  S = FormattedString();
+  S.appendInlineCode("foo");
+  S.appendText(" bar");
+  EXPECT_EQ(S.renderAsMarkdown(), "`foo` bar");
+
+  S = FormattedString();
+  S.appendText("foo");
+  S.appendCodeBlock("bar");
+  S.appendText("baz");
+  EXPECT_EQ(S.renderAsMarkdown(), "foo\n```cpp\nbar\n```\nbaz");
+}
+
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/FormattedString.cpp
=

[PATCH] D65998: [clangd] Added the vscode SemanticHighlighting feature code but did not enable it in the client.

[Haojian Wu via Phabricator via cfe-commits]

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

mostly good with a few nits.




Comment at: 
clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts:9
+// Parameters for the semantic highlighting (server-side) push notification.
+interface SemanticHighlightingParams {
+  // The text document that has to be decorated with the semantic highlighting

could you add a comment describing this is a mirror of LSP definition?




Comment at: 
clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts:16
+}
+
+// Contains the highlighting information for a specified line.

nit: I'd remove this blank line to make `SemanticHighlightingParams` and 
`SemanticHighlightingInformation` group closer as they are mirrors of LSP 
definitions.



Comment at: 
clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts:27
+
+// A single SemanticHighlightingToken that clangd sent.
+interface SemanticHighlightingToken {

the comment seems not precise to me, this is the data decoded from the 
base64-encoded string sent by clangd.



Comment at: 
clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts:33
+  length: number;
+  // The TextMate scope index to the clangd scopes.
+  scope: number;

nit: scope lookup table.



Comment at: 
clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts:34
+  // The TextMate scope index to the clangd scopes.
+  scope: number;
+}

nit: maybe name it `scopeIndex`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65998



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

[PATCH] D64741: [clangd] Added highlighting for tokens that are macro arguments.

[Johan Vikström via Phabricator via cfe-commits]

jvikstrom updated this revision to Diff 214619.
jvikstrom added a comment.

Rebased to master.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64741

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -323,6 +323,57 @@
   $Primitive[[auto]] $Variable[[Form]] = 10.2 + 2 * 4;
   $Primitive[[decltype]]($Variable[[Form]]) $Variable[[F]] = 10;
   auto $Variable[[Fun]] = []()->$Primitive[[void]]{};
+)cpp",
+  // Tokens that share a source range but have conflicting Kinds are not
+  // highlighted.
+R"cpp(
+  #define DEF_MULTIPLE(X) namespace X { class X { int X; }; }
+  #define DEF_CLASS(T) class T {};
+  DEF_MULTIPLE(XYZ);
+  DEF_MULTIPLE(XYZW);
+  DEF_CLASS($Class[[A]])
+  #define MACRO_CONCAT(X, V, T) T foo##X = V
+  #define DEF_VAR(X, V) int X = V
+  #define DEF_VAR_T(T, X, V) T X = V
+  #define DEF_VAR_REV(V, X) DEF_VAR(X, V)
+  #define CPY(X) X
+  #define DEF_VAR_TYPE(X, Y) X Y
+  #define SOME_NAME variable
+  #define SOME_NAME_SET variable2 = 123
+  #define INC_VAR(X) X += 2
+  $Primitive[[void]] $Function[[foo]]() {
+DEF_VAR($Variable[[X]],  123);
+DEF_VAR_REV(908, $Variable[[XY]]);
+$Primitive[[int]] CPY( $Variable[[XX]] );
+DEF_VAR_TYPE($Class[[A]], $Variable[[AA]]);
+$Primitive[[double]] SOME_NAME;
+$Primitive[[int]] SOME_NAME_SET;
+$Variable[[variable]] = 20.1;
+MACRO_CONCAT(var, 2, $Primitive[[float]]);
+DEF_VAR_T($Class[[A]], CPY(CPY($Variable[[Nested]])),
+  CPY($Class[[A]]()));
+INC_VAR($Variable[[variable]]);
+  }
+  $Primitive[[void]] SOME_NAME();
+  DEF_VAR($Variable[[XYZ]], 567);
+  DEF_VAR_REV(756, $Variable[[AB]]);
+
+  #define CALL_FN(F) F();
+  #define DEF_FN(F) void F ()
+  DEF_FN($Function[[g]]) {
+CALL_FN($Function[[foo]]);
+  }
+)cpp", 
+R"cpp(
+  #define fail(expr) expr
+  #define assert(COND) if (!(COND)) { fail("assertion failed" #COND); }
+  $Primitive[[int]] $Variable[[x]];
+  $Primitive[[int]] $Variable[[y]];
+  $Primitive[[int]] $Function[[f]]();
+  $Primitive[[void]] $Function[[foo]]() {
+assert($Variable[[x]] != $Variable[[y]]);
+assert($Variable[[x]] != $Function[[f]]());
+  }
 )cpp"};
   for (const auto &TestCase : TestCases) {
 checkHighlightings(TestCase);
@@ -338,6 +389,19 @@
 int someMethod();
 void otherMethod();
   )cpp"}});
+
+  // A separate test for macros in headers.
+  checkHighlightings(R"cpp(
+#include "imp.h"
+DEFINE_Y
+DXYZ_Y(A);
+  )cpp",
+ {{"imp.h", R"cpp(
+#define DXYZ(X) class X {};
+#define DXYZ_Y(Y) DXYZ(x##Y)
+#define DEFINE(X) int X;
+#define DEFINE_Y DEFINE(Y)
+  )cpp"}});
 }
 
 TEST(SemanticHighlighting, GeneratesHighlightsWhenFileChange) {
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -38,6 +38,25 @@
 llvm::sort(Tokens);
 auto Last = std::unique(Tokens.begin(), Tokens.end());
 Tokens.erase(Last, Tokens.end());
+
+// Macros can give tokens that have the same source range but conflicting
+// kinds. In this case all tokens sharing this source range should be
+// removed.
+for (unsigned I = 0; I < Tokens.size(); ++I) {
+  ArrayRef TokRef(Tokens);
+  ArrayRef Conflicting =
+  llvm::ArrayRef(TokRef.begin() + I, TokRef.end())
+  .take_while([&](const HighlightingToken &T) {
+return T.R == Tokens[I].R;
+  });
+
+  if (Conflicting.size() > 1) {
+Tokens.erase(Tokens.begin() + I,
+ Tokens.begin() + I + Conflicting.size());
+--I;
+  }
+}
+
 return Tokens;
   }
 
@@ -227,13 +246,18 @@
   }
 
   void addToken(SourceLocation Loc, HighlightingKind Kind) {
-if (Loc.isMacroID())
-  // FIXME: skip tokens inside macros for now.
-  return;
+if(Loc.isMacroID()) {
+  // Only intereseted in highlighting arguments in macros (DEF_X(arg)).
+  if (!SM.isMacroArgExpansion(Loc))
+return;
+  Loc = SM.getSpellingLoc(Loc);
+}
 
 // Non top level decls that are included from a header are not filtered by
 // topLevelDecls. (example: method declarations being included from another
 // file for a class from another file)
+// There are also cases 

[PATCH] D64741: [clangd] Added highlighting for tokens that are macro arguments.

[Johan Vikström via Phabricator via cfe-commits]

jvikstrom added a comment.

@ilya-biryukov @hokein pinging about this cl (I had completely forgotten about 
it somehow. Just updated it to the new master though)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64741



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

[PATCH] D64741: [clangd] Added highlighting for tokens that are macro arguments.

[Johan Vikström via Phabricator via cfe-commits]

jvikstrom updated this revision to Diff 214620.
jvikstrom added a comment.

Moved comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64741

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -323,6 +323,57 @@
   $Primitive[[auto]] $Variable[[Form]] = 10.2 + 2 * 4;
   $Primitive[[decltype]]($Variable[[Form]]) $Variable[[F]] = 10;
   auto $Variable[[Fun]] = []()->$Primitive[[void]]{};
+)cpp",
+// Tokens that share a source range but have conflicting Kinds are not
+// highlighted.
+R"cpp(
+  #define DEF_MULTIPLE(X) namespace X { class X { int X; }; }
+  #define DEF_CLASS(T) class T {};
+  DEF_MULTIPLE(XYZ);
+  DEF_MULTIPLE(XYZW);
+  DEF_CLASS($Class[[A]])
+  #define MACRO_CONCAT(X, V, T) T foo##X = V
+  #define DEF_VAR(X, V) int X = V
+  #define DEF_VAR_T(T, X, V) T X = V
+  #define DEF_VAR_REV(V, X) DEF_VAR(X, V)
+  #define CPY(X) X
+  #define DEF_VAR_TYPE(X, Y) X Y
+  #define SOME_NAME variable
+  #define SOME_NAME_SET variable2 = 123
+  #define INC_VAR(X) X += 2
+  $Primitive[[void]] $Function[[foo]]() {
+DEF_VAR($Variable[[X]],  123);
+DEF_VAR_REV(908, $Variable[[XY]]);
+$Primitive[[int]] CPY( $Variable[[XX]] );
+DEF_VAR_TYPE($Class[[A]], $Variable[[AA]]);
+$Primitive[[double]] SOME_NAME;
+$Primitive[[int]] SOME_NAME_SET;
+$Variable[[variable]] = 20.1;
+MACRO_CONCAT(var, 2, $Primitive[[float]]);
+DEF_VAR_T($Class[[A]], CPY(CPY($Variable[[Nested]])),
+  CPY($Class[[A]]()));
+INC_VAR($Variable[[variable]]);
+  }
+  $Primitive[[void]] SOME_NAME();
+  DEF_VAR($Variable[[XYZ]], 567);
+  DEF_VAR_REV(756, $Variable[[AB]]);
+
+  #define CALL_FN(F) F();
+  #define DEF_FN(F) void F ()
+  DEF_FN($Function[[g]]) {
+CALL_FN($Function[[foo]]);
+  }
+)cpp", 
+R"cpp(
+  #define fail(expr) expr
+  #define assert(COND) if (!(COND)) { fail("assertion failed" #COND); }
+  $Primitive[[int]] $Variable[[x]];
+  $Primitive[[int]] $Variable[[y]];
+  $Primitive[[int]] $Function[[f]]();
+  $Primitive[[void]] $Function[[foo]]() {
+assert($Variable[[x]] != $Variable[[y]]);
+assert($Variable[[x]] != $Function[[f]]());
+  }
 )cpp"};
   for (const auto &TestCase : TestCases) {
 checkHighlightings(TestCase);
@@ -338,6 +389,19 @@
 int someMethod();
 void otherMethod();
   )cpp"}});
+
+  // A separate test for macros in headers.
+  checkHighlightings(R"cpp(
+#include "imp.h"
+DEFINE_Y
+DXYZ_Y(A);
+  )cpp",
+ {{"imp.h", R"cpp(
+#define DXYZ(X) class X {};
+#define DXYZ_Y(Y) DXYZ(x##Y)
+#define DEFINE(X) int X;
+#define DEFINE_Y DEFINE(Y)
+  )cpp"}});
 }
 
 TEST(SemanticHighlighting, GeneratesHighlightsWhenFileChange) {
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -38,6 +38,25 @@
 llvm::sort(Tokens);
 auto Last = std::unique(Tokens.begin(), Tokens.end());
 Tokens.erase(Last, Tokens.end());
+
+// Macros can give tokens that have the same source range but conflicting
+// kinds. In this case all tokens sharing this source range should be
+// removed.
+for (unsigned I = 0; I < Tokens.size(); ++I) {
+  ArrayRef TokRef(Tokens);
+  ArrayRef Conflicting =
+  llvm::ArrayRef(TokRef.begin() + I, TokRef.end())
+  .take_while([&](const HighlightingToken &T) {
+return T.R == Tokens[I].R;
+  });
+
+  if (Conflicting.size() > 1) {
+Tokens.erase(Tokens.begin() + I,
+ Tokens.begin() + I + Conflicting.size());
+--I;
+  }
+}
+
 return Tokens;
   }
 
@@ -227,13 +246,18 @@
   }
 
   void addToken(SourceLocation Loc, HighlightingKind Kind) {
-if (Loc.isMacroID())
-  // FIXME: skip tokens inside macros for now.
-  return;
+if(Loc.isMacroID()) {
+  // Only intereseted in highlighting arguments in macros (DEF_X(arg)).
+  if (!SM.isMacroArgExpansion(Loc))
+return;
+  Loc = SM.getSpellingLoc(Loc);
+}
 
 // Non top level decls that are included from a header are not filtered by
 // topLevelDecls. (example: method declarations being included from another
 // file for a class from another file)
+// There are also cases with mac

[PATCH] D65591: [AST] Add a flag indicating if any subexpression had errors

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added a comment.

It seems that these two options are not exactly the same right ? The 
`ContainsError` bit is useful to quickly answer "Does this expression contains 
an invalid sub-expression" without doing the search, while adding an 
`ErrorExpr` node is useful to note that //this// sub-expression is invalid (and 
as Aaron says the hypothetical `ErrorExpr` node can carry more info about the 
error).




Comment at: clang/include/clang/AST/Expr.h:1521
+  : Expr(CharacterLiteralClass, type, VK_RValue, OK_Ordinary, false, false,
+ false, false, false),
+Value(value), Loc(l) {

`/*ContainsError=*/false` here and elsewhere ?



Comment at: clang/include/clang/AST/Expr.h:3602
+ ExprValueKind VK, ExprObjectKind OK, SourceLocation opLoc,
+ FPOptions FPFeatures, bool dead2)
+  : Expr(CompoundAssignOperatorClass, ResTy, VK, OK,

What is this `dead2` ?



Comment at: clang/lib/AST/ExprObjC.cpp:29
 : Expr(ObjCArrayLiteralClass, T, VK_RValue, OK_Ordinary, false, false,
-   false, false),
+   false, false, /*ContainsErros*/ false),
   NumElements(Elements.size()), Range(SR), ArrayWithObjectsMethod(Method) {

s/ContainsErros/ContainsErrors


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65591



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

[PATCH] D66087: [clangd] Preserve line break when rendering text chunks of markdown

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added a reviewer: sammccall.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay.
Herald added a project: clang.

Fixes https://github.com/clangd/clangd/issues/95


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66087

Files:
  clang-tools-extra/clangd/FormattedString.cpp
  clang-tools-extra/clangd/unittests/FormattedStringTests.cpp


Index: clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
===
--- clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
+++ clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
@@ -179,7 +179,7 @@
   S = FormattedString();
   S.appendText("foo\n");
   S.appendInlineCode("bar");
-  EXPECT_EQ(S.renderAsMarkdown(), "foo\n`bar`");
+  EXPECT_EQ(S.renderAsMarkdown(), "foo  \n`bar`");
 
   S = FormattedString();
   S.appendInlineCode("foo");
@@ -193,6 +193,12 @@
   EXPECT_EQ(S.renderAsMarkdown(), "foo\n```cpp\nbar\n```\nbaz");
 }
 
+TEST(FormattedString, MarkdownLineBreaks) {
+  FormattedString S;
+  S.appendText("foo\nbar\nbaz");
+  // To preserve line breaks, two spaces are added at the end of each line.
+  EXPECT_EQ(S.renderAsMarkdown(), "foo  \nbar  \nbaz");
+}
 
 } // namespace
 } // namespace clangd
Index: clang-tools-extra/clangd/FormattedString.cpp
===
--- clang-tools-extra/clangd/FormattedString.cpp
+++ clang-tools-extra/clangd/FormattedString.cpp
@@ -23,17 +23,16 @@
   // Escaping ASCII punctiation ensures we can't start a markdown construct.
   constexpr llvm::StringLiteral Punctuation =
   R"txt(!"#$%&'()*+,-./:;<=>?@[\]^_`{|}~)txt";
-
   std::string R;
-  for (size_t From = 0; From < Input.size();) {
-size_t Next = Input.find_first_of(Punctuation, From);
-R += Input.substr(From, Next - From);
-if (Next == llvm::StringRef::npos)
-  break;
-R += "\\";
-R += Input[Next];
-
-From = Next + 1;
+  for (char C : Input) {
+if (Punctuation.contains(C)) {
+  // Has to be escaped.
+  R += "\\";
+} else if (C == '\n') {
+  // Putting two spaces at the end of the line preserves line breaks.
+  R += "  ";
+}
+R += C;
   }
   return R;
 }


Index: clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
===
--- clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
+++ clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
@@ -179,7 +179,7 @@
   S = FormattedString();
   S.appendText("foo\n");
   S.appendInlineCode("bar");
-  EXPECT_EQ(S.renderAsMarkdown(), "foo\n`bar`");
+  EXPECT_EQ(S.renderAsMarkdown(), "foo  \n`bar`");
 
   S = FormattedString();
   S.appendInlineCode("foo");
@@ -193,6 +193,12 @@
   EXPECT_EQ(S.renderAsMarkdown(), "foo\n```cpp\nbar\n```\nbaz");
 }
 
+TEST(FormattedString, MarkdownLineBreaks) {
+  FormattedString S;
+  S.appendText("foo\nbar\nbaz");
+  // To preserve line breaks, two spaces are added at the end of each line.
+  EXPECT_EQ(S.renderAsMarkdown(), "foo  \nbar  \nbaz");
+}
 
 } // namespace
 } // namespace clangd
Index: clang-tools-extra/clangd/FormattedString.cpp
===
--- clang-tools-extra/clangd/FormattedString.cpp
+++ clang-tools-extra/clangd/FormattedString.cpp
@@ -23,17 +23,16 @@
   // Escaping ASCII punctiation ensures we can't start a markdown construct.
   constexpr llvm::StringLiteral Punctuation =
   R"txt(!"#$%&'()*+,-./:;<=>?@[\]^_`{|}~)txt";
-
   std::string R;
-  for (size_t From = 0; From < Input.size();) {
-size_t Next = Input.find_first_of(Punctuation, From);
-R += Input.substr(From, Next - From);
-if (Next == llvm::StringRef::npos)
-  break;
-R += "\\";
-R += Input[Next];
-
-From = Next + 1;
+  for (char C : Input) {
+if (Punctuation.contains(C)) {
+  // Has to be escaped.
+  R += "\\";
+} else if (C == '\n') {
+  // Putting two spaces at the end of the line preserves line breaks.
+  R += "  ";
+}
+R += C;
   }
   return R;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64274: [analyzer] VirtualCallChecker overhaul.

[Kristóf Umann via Phabricator via cfe-commits]

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

In D64274#1623644 , @NoQ wrote:

> In D64274#1600834 , @Szelethus wrote:
>
> > In D64274#1598226 , @NoQ wrote:
> >
> > > I'm confused though; the way i was previously understanding your work, 
> > > when disabling a dependency and then enabling a dependent checker *later* 
> > > in the run-line, it should re-enable the dependency automatically. Did 
> > > you consciously decide not to implement it that way?
> >
> >
> > Yes, somewhat. I chose another direction, which is simply not exposing base 
> > checkers (which I detailed in D60925 ) to 
> > the users (`-analyzer-checker-help-developer` is a thing now), so they 
> > aren't ever enabled if none of their dependent checkers are. This worked 
> > wonders, because, interestingly, none of the base checkers were emitting 
> > diagnostics.
> >
> > I find this clearer, if I disable something because it crashes on my code, 
> > I don't want to see it again. Though, 9.0.0-final isn't out, and I'm sorry 
> > that I didn't make this decision clear enough -- shall I fix it?
>
>
> Yes, generally i think it's good to have later options override earlier 
> options, because it allows people who run analysis through multiple layers of 
> scripts (eg., buildbots) to override their previous decisions without digging 
> through the whole pile of scripts. This isn't *that* important because the 
> use case is pretty rare, but if you have any immediate ideas on how to fix 
> this i'd be pretty happy :)


The immediate idea is to never allow checkers that emit diagnostics to be 
dependencies. That way, we'll let the analyzer handle enabling/disabling them 
under the hood, and the checker dependency structure would be far more 
accurate, since no checker depends on diagnostics. What do you think? This 
seems to a hot topic anyways now, and would solve the issue behind core 
checkers as mentioned in D66042 . I think it 
wouldn't be too much work, especially that I'm very familiar with this part of 
the codebase. That said, the solution might be lengthier than what folks would 
be comfortable merging so late into the release cycle.

My entire checker dependency project was about getting rid of bugs to 
eventually list analyzer/checker options, but I think the idea of separating 
modeling/diagnostic parts should be a project-wide rule. I feel bad for 
dragging you through this, as I previously said

In D64274#1572407 , @Szelethus wrote:

> [side note: since this checker doesn't really do any modeling, in fact it 
> wouldn even store a state if it inspected the call stack, it wouldn't be 
> logical to create a modeling checker that would be a dependency of both 
> `PureVirtualCall` and `VirtualCall`. Since `PureVirtualCall` is a strict 
> subset of `VirtualCall`, if it causes any crashes, you have to disable both 
> anyways. ]


but now I believe having a common base would have been the ideal solution. 
Please feel free to commit as is, I'll pick this up as I enforce the rule.

Maybe its also time to escalate this issue to cfe-dev.


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

https://reviews.llvm.org/D64274



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

[clang-tools-extra] r368568 - [clangd] Added the vscode SemanticHighlighting feature code but did not enable it in the client.

[Johan Vikstrom via cfe-commits]

Author: jvikstrom
Date: Mon Aug 12 06:33:43 2019
New Revision: 368568

URL: http://llvm.org/viewvc/llvm-project?rev=368568&view=rev
Log:
[clangd] Added the vscode SemanticHighlighting feature code but did not enable 
it in the client.

Summary: Added the code for the StaticFeature that must be registered to the 
client. Also decoding the notification data into objects. Did not register it 
to the client yet.

Reviewers: hokein, ilya-biryukov

Subscribers: MaskRay, jkorous, arphaman, kadircet, cfe-commits

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json

clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/semantic-highlighting.ts

clang-tools-extra/trunk/clangd/clients/clangd-vscode/test/semantic-highlighting.test.ts

Modified: clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json?rev=368568&r1=368567&r2=368568&view=diff
==
--- clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json (original)
+++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json Mon Aug 
12 06:33:43 2019
@@ -38,7 +38,8 @@
 "dependencies": {
 "jsonc-parser": "^2.1.0",
 "vscode-languageclient": "^5.3.0-next.6",
-"vscode-languageserver": "^5.3.0-next.6"
+"vscode-languageserver": "^5.3.0-next.6",
+"vscode-languageserver-types": "^3.14.0"
 },
 "devDependencies": {
 "@types/mocha": "^2.2.32",

Modified: 
clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/semantic-highlighting.ts?rev=368568&r1=368567&r2=368568&view=diff
==
--- 
clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
 (original)
+++ 
clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
 Mon Aug 12 06:33:43 2019
@@ -2,6 +2,95 @@ import * as fs from 'fs';
 import * as jsonc from "jsonc-parser";
 import * as path from 'path';
 import * as vscode from 'vscode';
+import * as vscodelc from 'vscode-languageclient';
+import * as vscodelct from 'vscode-languageserver-types';
+
+// Parameters for the semantic highlighting (server-side) push notification.
+// Mirrors the structure in the semantic highlighting proposal for LSP.
+interface SemanticHighlightingParams {
+  // The text document that has to be decorated with the semantic highlighting
+  // information.
+  textDocument: vscodelct.VersionedTextDocumentIdentifier;
+  // An array of semantic highlighting information.
+  lines: SemanticHighlightingInformation[];
+}
+// Contains the highlighting information for a specified line. Mirrors the
+// structure in the semantic highlighting proposal for LSP.
+interface SemanticHighlightingInformation {
+  // The zero-based line position in the text document.
+  line: number;
+  // A base64 encoded string representing every single highlighted characters
+  // with its start position, length and the "lookup table" index of of the
+  // semantic highlighting Text Mate scopes.
+  tokens?: string;
+}
+
+// A SemanticHighlightingToken decoded from the base64 data sent by clangd.
+interface SemanticHighlightingToken {
+  // Start column for this token.
+  character: number;
+  // Length of the token.
+  length: number;
+  // The TextMate scope index to the clangd scope lookup table.
+  scopeIndex: number;
+}
+
+// Language server push notification providing the semantic highlighting
+// information for a text document.
+export const NotificationType =
+new vscodelc.NotificationType(
+'textDocument/semanticHighlighting');
+
+// The feature that should be registered in the vscode lsp for enabling
+// experimental semantic highlighting.
+export class SemanticHighlightingFeature implements vscodelc.StaticFeature {
+  // The TextMate scope lookup table. A token with scope index i has the scopes
+  // on index i in the lookup table.
+  scopeLookupTable: string[][];
+  fillClientCapabilities(capabilities: vscodelc.ClientCapabilities) {
+// Extend the ClientCapabilities type and add semantic highlighting
+// capability to the object.
+const textDocumentCapabilities: vscodelc.TextDocumentClientCapabilities&
+{semanticHighlightingCapabilities?: {semanticHighlighting : boolean}} =
+capabilities.textDocument;
+textDocumentCapabilities.semanticHighlightingCapabilities = {
+  semanticHighlighting : true,
+};
+  }
+
+  initialize(capabilities: vscodelc.ServerCapabilities,
+ documentSelector: vscodelc.DocumentSelector|undefined) {
+// The semantic highlighting capability information is in the capabilities

[PATCH] D65998: [clangd] Added the vscode SemanticHighlighting feature code but did not enable it in the client.

[Johan Vikström via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
jvikstrom marked 5 inline comments as done.
Closed by commit rL368568: [clangd] Added the vscode SemanticHighlighting 
feature code but did not enable… (authored by jvikstrom, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65998?vs=214613&id=214624#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65998

Files:
  clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json
  
clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
  
clang-tools-extra/trunk/clangd/clients/clangd-vscode/test/semantic-highlighting.test.ts

Index: clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
===
--- clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
+++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
@@ -2,6 +2,95 @@
 import * as jsonc from "jsonc-parser";
 import * as path from 'path';
 import * as vscode from 'vscode';
+import * as vscodelc from 'vscode-languageclient';
+import * as vscodelct from 'vscode-languageserver-types';
+
+// Parameters for the semantic highlighting (server-side) push notification.
+// Mirrors the structure in the semantic highlighting proposal for LSP.
+interface SemanticHighlightingParams {
+  // The text document that has to be decorated with the semantic highlighting
+  // information.
+  textDocument: vscodelct.VersionedTextDocumentIdentifier;
+  // An array of semantic highlighting information.
+  lines: SemanticHighlightingInformation[];
+}
+// Contains the highlighting information for a specified line. Mirrors the
+// structure in the semantic highlighting proposal for LSP.
+interface SemanticHighlightingInformation {
+  // The zero-based line position in the text document.
+  line: number;
+  // A base64 encoded string representing every single highlighted characters
+  // with its start position, length and the "lookup table" index of of the
+  // semantic highlighting Text Mate scopes.
+  tokens?: string;
+}
+
+// A SemanticHighlightingToken decoded from the base64 data sent by clangd.
+interface SemanticHighlightingToken {
+  // Start column for this token.
+  character: number;
+  // Length of the token.
+  length: number;
+  // The TextMate scope index to the clangd scope lookup table.
+  scopeIndex: number;
+}
+
+// Language server push notification providing the semantic highlighting
+// information for a text document.
+export const NotificationType =
+new vscodelc.NotificationType(
+'textDocument/semanticHighlighting');
+
+// The feature that should be registered in the vscode lsp for enabling
+// experimental semantic highlighting.
+export class SemanticHighlightingFeature implements vscodelc.StaticFeature {
+  // The TextMate scope lookup table. A token with scope index i has the scopes
+  // on index i in the lookup table.
+  scopeLookupTable: string[][];
+  fillClientCapabilities(capabilities: vscodelc.ClientCapabilities) {
+// Extend the ClientCapabilities type and add semantic highlighting
+// capability to the object.
+const textDocumentCapabilities: vscodelc.TextDocumentClientCapabilities&
+{semanticHighlightingCapabilities?: {semanticHighlighting : boolean}} =
+capabilities.textDocument;
+textDocumentCapabilities.semanticHighlightingCapabilities = {
+  semanticHighlighting : true,
+};
+  }
+
+  initialize(capabilities: vscodelc.ServerCapabilities,
+ documentSelector: vscodelc.DocumentSelector|undefined) {
+// The semantic highlighting capability information is in the capabilities
+// object but to access the data we must first extend the ServerCapabilities
+// type.
+const serverCapabilities: vscodelc.ServerCapabilities&
+{semanticHighlighting?: {scopes : string[][]}} = capabilities;
+if (!serverCapabilities.semanticHighlighting)
+  return;
+this.scopeLookupTable = serverCapabilities.semanticHighlighting.scopes;
+  }
+
+  handleNotification(params: SemanticHighlightingParams) {}
+}
+
+// Converts a string of base64 encoded tokens into the corresponding array of
+// HighlightingTokens.
+export function decodeTokens(tokens: string): SemanticHighlightingToken[] {
+  const scopeMask = 0x;
+  const lenShift = 0x10;
+  const uint32Size = 4;
+  const buf = Buffer.from(tokens, 'base64');
+  const retTokens = [];
+  for (let i = 0, end = buf.length / uint32Size; i < end; i += 2) {
+const start = buf.readUInt32BE(i * uint32Size);
+const lenKind = buf.readUInt32BE((i + 1) * uint32Size);
+const scopeIndex = lenKind & scopeMask;
+const len = lenKind >>> lenShift;
+retTokens.push({character : start, scopeIndex : scopeIndex, length : len})

Re: [clang-tools-extra] r368498 - clangd: use -j for background index pool

[Hans Wennborg via cfe-commits]

Merged to release_90 in r368569.

On Sat, Aug 10, 2019 at 1:02 AM Sam McCall via cfe-commits
 wrote:
>
> Author: sammccall
> Date: Fri Aug  9 16:03:32 2019
> New Revision: 368498
>
> URL: http://llvm.org/viewvc/llvm-project?rev=368498&view=rev
> Log:
> clangd: use -j for background index pool
>
> Summary:
> clangd supports a -j option to limit the amount of threads to use for parsing
> TUs. However, when using -background-index (the default in later versions of
> clangd), the parallelism used by clangd defaults to the hardware_parallelisn,
> i.e. number of physical cores.
>
> On shared hardware environments, with large projects, this can significantly
> affect performance with no way to tune it down.
>
> This change makes the -j parameter apply equally to parsing and background
> index. It's not perfect, because the total number of threads is 2x the -j 
> value,
> which may still be unexpected. But at least this change allows users to 
> prevent
> clangd using all CPU cores.
>
> Reviewers: kadircet, sammccall
>
> Reviewed By: sammccall
>
> Subscribers: javed.absar, jfb, sammccall, ilya-biryukov, MaskRay, jkorous, 
> arphaman, cfe-commits
>
> Tags: #clang
>
> Differential Revision: https://reviews.llvm.org/D66031
>
> Modified:
> clang-tools-extra/trunk/clangd/ClangdServer.cpp
> clang-tools-extra/trunk/clangd/TUScheduler.cpp
> clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
>
> Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=368498&r1=368497&r2=368498&view=diff
> ==
> --- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
> +++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Fri Aug  9 16:03:32 2019
> @@ -40,9 +40,11 @@
>  #include "llvm/Support/FileSystem.h"
>  #include "llvm/Support/Path.h"
>  #include "llvm/Support/raw_ostream.h"
> +#include 
>  #include 
>  #include 
>  #include 
> +#include 
>
>  namespace clang {
>  namespace clangd {
> @@ -117,8 +119,7 @@ ClangdServer::ClangdServer(const GlobalC
>   : nullptr),
>GetClangTidyOptions(Opts.GetClangTidyOptions),
>SuggestMissingIncludes(Opts.SuggestMissingIncludes),
> -  TweakFilter(Opts.TweakFilter),
> -  WorkspaceRoot(Opts.WorkspaceRoot),
> +  TweakFilter(Opts.TweakFilter), WorkspaceRoot(Opts.WorkspaceRoot),
>// Pass a callback into `WorkScheduler` to extract symbols from a newly
>// parsed file and rebuild the file index synchronously each time an 
> AST
>// is parsed.
> @@ -144,7 +145,8 @@ ClangdServer::ClangdServer(const GlobalC
>  BackgroundIdx = llvm::make_unique(
>  Context::current().clone(), FSProvider, CDB,
>  BackgroundIndexStorage::createDiskBackedStorageFactory(
> -[&CDB](llvm::StringRef File) { return CDB.getProjectInfo(File); 
> }));
> +[&CDB](llvm::StringRef File) { return CDB.getProjectInfo(File); 
> }),
> +std::max(Opts.AsyncThreadsCount, 1u));
>  AddIndex(BackgroundIdx.get());
>}
>if (DynamicIdx)
>
> Modified: clang-tools-extra/trunk/clangd/TUScheduler.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/TUScheduler.cpp?rev=368498&r1=368497&r2=368498&view=diff
> ==
> --- clang-tools-extra/trunk/clangd/TUScheduler.cpp (original)
> +++ clang-tools-extra/trunk/clangd/TUScheduler.cpp Fri Aug  9 16:03:32 2019
> @@ -54,6 +54,7 @@
>  #include "llvm/ADT/ScopeExit.h"
>  #include "llvm/Support/Errc.h"
>  #include "llvm/Support/Path.h"
> +#include "llvm/Support/Threading.h"
>  #include 
>  #include 
>  #include 
> @@ -801,10 +802,10 @@ std::string renderTUAction(const TUActio
>  } // namespace
>
>  unsigned getDefaultAsyncThreadsCount() {
> -  unsigned HardwareConcurrency = std::thread::hardware_concurrency();
> -  // C++ standard says that hardware_concurrency()
> -  // may return 0, fallback to 1 worker thread in
> -  // that case.
> +  unsigned HardwareConcurrency = llvm::heavyweight_hardware_concurrency();
> +  // heavyweight_hardware_concurrency may fall back to hardware_concurrency.
> +  // C++ standard says that hardware_concurrency() may return 0; fallback to 
> 1
> +  // worker thread in that case.
>if (HardwareConcurrency == 0)
>  return 1;
>return HardwareConcurrency;
>
> Modified: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp?rev=368498&r1=368497&r2=368498&view=diff
> ==
> --- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp (original)
> +++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp Fri Aug  9 16:03:32 
> 2019
> @@ -267,7 +267,8 @@ list TweakList{
>  opt WorkerThreadsCount{
>  "j",
>  cat(Mis

[PATCH] D65591: [AST] Add a flag indicating if any subexpression had errors

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In D65591#1625154 , @riccibruno wrote:

> It seems that these two options are not exactly the same right ? The 
> `ContainsError` bit is useful to quickly answer "Does this expression 
> contains an invalid sub-expression" without doing the search, while adding an 
> `ErrorExpr` node is useful to note that //this// sub-expression is invalid 
> (and as Aaron says the hypothetical `ErrorExpr` node can carry more info 
> about the error).


Exactly right, thanks for summing this up!

In D65591#1625019 , @aaron.ballman 
wrote:

> Rather than adding a bit onto `Expr` to specify whether it's erroneous, have 
> you considered making this a property of the type system by introducing an 
> `ErrorExpr` AST node that other nodes can inherit from? I think that approach 
> would work more naturally for things like AST matchers while solving some 
> problems we have with being able to pretty printing or AST dump erroneous 
> ASTs. The `ErrorExpr` node could contain a subnode of a partially-valid 
> `Expr` object that would retain as much of the information as we've been able 
> to gather, but the error node (or its subclasses) could contain other 
> information useful when handling errors, such as storing the original source 
> text (or range) for the expression, potential fixes, etc.


Having something similar to `ErrorExpr` is actually the next logical step I'm 
currently exploring.  The idea of this bit is to carry information on whether 
any (recursive) child of an expression had an error that was already diagnosed.
Detecting those expression is useful to avoid producing additional irrelevant 
diagnostics (see the dependent revision that uses the bit to get rid of one 
such diagnostic) and avoid running various functions that are not prepared to 
handle subexpressions that have errors (e.g. constant evaluation).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65591



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

[PATCH] D65591: [AST] Add a flag indicating if any subexpression had errors

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D65591#1625154 , @riccibruno wrote:

> It seems that these two options are not exactly the same right ? The 
> `ContainsError` bit is useful to quickly answer "Does this expression 
> contains an invalid sub-expression" without doing the search, while adding an 
> `ErrorExpr` node is useful to note that //this// sub-expression is invalid 
> (and as Aaron says the hypothetical `ErrorExpr` node can carry more info 
> about the error).


That's true. I had figured that answering "does this expression contain an 
invalid sub-expression" could be implemented with a walk of the expression tree 
rather than consuming a bit. To consumers of `containsErrors()`, there 
shouldn't be much difference aside from performance (and that may be sufficient 
reason to go with a bit, but I think I'd like to see performance measurements 
that show the bit is necessary).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65591



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

[PATCH] D66031: clangd: use -j for background index pool

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

In D66031#1623935 , @sammccall wrote:

> @hans If you don't mind merging this, it's a nice usability improvement.


Sure! Merged in r368569.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66031



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

[PATCH] D65591: [AST] Add a flag indicating if any subexpression had errors

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added a comment.

In D65591#1625183 , @aaron.ballman 
wrote:

> In D65591#1625154 , @riccibruno 
> wrote:
>
> > It seems that these two options are not exactly the same right ? The 
> > `ContainsError` bit is useful to quickly answer "Does this expression 
> > contains an invalid sub-expression" without doing the search, while adding 
> > an `ErrorExpr` node is useful to note that //this// sub-expression is 
> > invalid (and as Aaron says the hypothetical `ErrorExpr` node can carry more 
> > info about the error).
>
>
> That's true. I had figured that answering "does this expression contain an 
> invalid sub-expression" could be implemented with a walk of the expression 
> tree rather than consuming a bit. To consumers of `containsErrors()`, there 
> shouldn't be much difference aside from performance (and that may be 
> sufficient reason to go with a bit, but I think I'd like to see performance 
> measurements that show the bit is necessary).


Fair enough, especially given that IIRC there are not many bits left in these 
bit-fields classes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65591



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

[PATCH] D65591: [AST] Add a flag indicating if any subexpression had errors

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In D65591#1625183 , @aaron.ballman 
wrote:

> In D65591#1625154 , @riccibruno 
> wrote:
>
> > It seems that these two options are not exactly the same right ? The 
> > `ContainsError` bit is useful to quickly answer "Does this expression 
> > contains an invalid sub-expression" without doing the search, while adding 
> > an `ErrorExpr` node is useful to note that //this// sub-expression is 
> > invalid (and as Aaron says the hypothetical `ErrorExpr` node can carry more 
> > info about the error).
>
>
> That's true. I had figured that answering "does this expression contain an 
> invalid sub-expression" could be implemented with a walk of the expression 
> tree rather than consuming a bit. To consumers of `containsErrors()`, there 
> shouldn't be much difference aside from performance (and that may be 
> sufficient reason to go with a bit, but I think I'd like to see performance 
> measurements that show the bit is necessary).


Are expression bits scarce?
We don't  any checks if expressions contain errors now, we simply drop too many 
invalid expressions and never put them into the AST.
It's impossible to do the measurements at this point, but it would be nice if 
adding those checks was cheap.

We can also assume they're cheap, use the visitor-based implementation and 
later switch if this turn out to be a problem.
I think we should prefer the approach that guarantees the compiler is not 
getting slow ever rather than chase the slowness later.

In D65591#1625192 , @riccibruno wrote:

> Fair enough, especially given that IIRC there are not many bits left in these 
> bit-fields classes.


May be reasonable in that case, but we should be aware of a potential `O(n^2)` 
and even exponential complexities in the frontend arising from calling 
`containsErrors` in too many places in that case.
If we ever run out of bits, we could remove this bit and instead introduce a 
cache (something like `set InvalidExprs`) in `ASTContext`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65591



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

[PATCH] D55125: [clang-tidy] Fix a false positive in misc-redundant-expression check

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

One more nit.




Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:674
+  return !(
+  (isTokAtEndOfExpr(Lsr, LTok, SM) && isTokAtEndOfExpr(Rsr, RTok, SM)) &&
+  isSameToken(LTok, RTok, SM));

aaron.ballman wrote:
> You can drop a set of parens here as they don't change the order of 
> evaluation.
I'd propagate the negation into the parentheses. I find it easier to understand 
`!a || !b || !c` than `!(a && b && c)`, since the former can be digested piece 
by piece, but the latter requires the reader to keep the `!` in their mind and 
"recurse" into the nested parentheses. It's similar to the "early return" 
pattern vs. nested conditional statements.


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

https://reviews.llvm.org/D55125



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

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

In D66042#1624081 , @NoQ wrote:

> +@alexfh because clang-tidy now finally has a way of safely disabling core 
> checkers without causing crashes all over the place! Would you like to take 
> the same approach as we picked in scan-build, i.e. when the user asks to 
> disable a core checker, silence it instead?


clang-tidy's native way to enable/disable diagnostics is applied to the static 
analyzer twice: first time when the list of enabled checkers is created (and 
then core checkers are always added to that list), and the second time - to 
each diagnostic generated by the static analyzer (this time the original check 
name filter is applied, without core checkers). This already works consistently 
from a user's perspective: https://gcc.godbolt.org/z/MEvSsP

Are there any benefits in using the new CheckerSilenceVector mechanism in 
clang-tidy?


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

https://reviews.llvm.org/D66042



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

[clang-tools-extra] r368581 - [clangd] Separate chunks with a space when rendering markdown

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Mon Aug 12 07:35:30 2019
New Revision: 368581

URL: http://llvm.org/viewvc/llvm-project?rev=368581&view=rev
Log:
[clangd] Separate chunks with a space when rendering markdown

Summary:
This results in better rendering of resulting markdown.

Especially noticeable in coc.nvim that does not have a visible horizontal
spaces around inline code blocks. More details and a screenshot from
coc.nvim can be found in https://github.com/clangd/clangd/issues/95.

Reviewers: sammccall

Reviewed By: sammccall

Subscribers: MaskRay, jkorous, arphaman, kadircet, cfe-commits

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/FormattedString.cpp
clang-tools-extra/trunk/clangd/unittests/FormattedStringTests.cpp

Modified: clang-tools-extra/trunk/clangd/FormattedString.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/FormattedString.cpp?rev=368581&r1=368580&r2=368581&view=diff
==
--- clang-tools-extra/trunk/clangd/FormattedString.cpp (original)
+++ clang-tools-extra/trunk/clangd/FormattedString.cpp Mon Aug 12 07:35:30 2019
@@ -112,15 +112,20 @@ void FormattedString::appendInlineCode(s
 
 std::string FormattedString::renderAsMarkdown() const {
   std::string R;
+  auto EnsureWhitespace = [&R]() {
+// Adds a space for nicer rendering.
+if (!R.empty() && !isWhitespace(R.back()))
+  R += " ";
+  };
   for (const auto &C : Chunks) {
 switch (C.Kind) {
 case ChunkKind::PlainText:
+  if (!C.Contents.empty() && !isWhitespace(C.Contents.front()))
+EnsureWhitespace();
   R += renderText(C.Contents);
   continue;
 case ChunkKind::InlineCodeBlock:
-  // Make sure we don't glue two backticks together.
-  if (llvm::StringRef(R).endswith("`"))
-R += " ";
+  EnsureWhitespace();
   R += renderInlineBlock(C.Contents);
   continue;
 case ChunkKind::CodeBlock:

Modified: clang-tools-extra/trunk/clangd/unittests/FormattedStringTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/FormattedStringTests.cpp?rev=368581&r1=368580&r2=368581&view=diff
==
--- clang-tools-extra/trunk/clangd/unittests/FormattedStringTests.cpp (original)
+++ clang-tools-extra/trunk/clangd/unittests/FormattedStringTests.cpp Mon Aug 
12 07:35:30 2019
@@ -72,7 +72,7 @@ after)md";
   S.appendText("baz");
 
   EXPECT_EQ(S.renderAsPlainText(), "foo bar baz");
-  EXPECT_EQ(S.renderAsMarkdown(), "foo`bar`baz");
+  EXPECT_EQ(S.renderAsMarkdown(), "foo `bar` baz");
 }
 
 TEST(FormattedString, Escaping) {
@@ -158,6 +158,42 @@ TEST(FormattedString, Escaping) {
   "`\n");
 }
 
+TEST(FormattedString, MarkdownWhitespace) {
+  // Whitespace should be added as separators between blocks.
+  FormattedString S;
+  S.appendText("foo");
+  S.appendText("bar");
+  EXPECT_EQ(S.renderAsMarkdown(), "foo bar");
+
+  S = FormattedString();
+  S.appendInlineCode("foo");
+  S.appendInlineCode("bar");
+  EXPECT_EQ(S.renderAsMarkdown(), "`foo` `bar`");
+
+  // However, we don't want to add any extra whitespace.
+  S = FormattedString();
+  S.appendText("foo ");
+  S.appendInlineCode("bar");
+  EXPECT_EQ(S.renderAsMarkdown(), "foo `bar`");
+
+  S = FormattedString();
+  S.appendText("foo\n");
+  S.appendInlineCode("bar");
+  EXPECT_EQ(S.renderAsMarkdown(), "foo\n`bar`");
+
+  S = FormattedString();
+  S.appendInlineCode("foo");
+  S.appendText(" bar");
+  EXPECT_EQ(S.renderAsMarkdown(), "`foo` bar");
+
+  S = FormattedString();
+  S.appendText("foo");
+  S.appendCodeBlock("bar");
+  S.appendText("baz");
+  EXPECT_EQ(S.renderAsMarkdown(), "foo\n```cpp\nbar\n```\nbaz");
+}
+
+
 } // namespace
 } // namespace clangd
 } // namespace clang


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

[PATCH] D66086: [clangd] Separate chunks with a space when rendering markdown

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL368581: [clangd] Separate chunks with a space when rendering 
markdown (authored by ibiryukov, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66086?vs=214618&id=214634#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66086

Files:
  clang-tools-extra/trunk/clangd/FormattedString.cpp
  clang-tools-extra/trunk/clangd/unittests/FormattedStringTests.cpp


Index: clang-tools-extra/trunk/clangd/unittests/FormattedStringTests.cpp
===
--- clang-tools-extra/trunk/clangd/unittests/FormattedStringTests.cpp
+++ clang-tools-extra/trunk/clangd/unittests/FormattedStringTests.cpp
@@ -72,7 +72,7 @@
   S.appendText("baz");
 
   EXPECT_EQ(S.renderAsPlainText(), "foo bar baz");
-  EXPECT_EQ(S.renderAsMarkdown(), "foo`bar`baz");
+  EXPECT_EQ(S.renderAsMarkdown(), "foo `bar` baz");
 }
 
 TEST(FormattedString, Escaping) {
@@ -158,6 +158,42 @@
   "`\n");
 }
 
+TEST(FormattedString, MarkdownWhitespace) {
+  // Whitespace should be added as separators between blocks.
+  FormattedString S;
+  S.appendText("foo");
+  S.appendText("bar");
+  EXPECT_EQ(S.renderAsMarkdown(), "foo bar");
+
+  S = FormattedString();
+  S.appendInlineCode("foo");
+  S.appendInlineCode("bar");
+  EXPECT_EQ(S.renderAsMarkdown(), "`foo` `bar`");
+
+  // However, we don't want to add any extra whitespace.
+  S = FormattedString();
+  S.appendText("foo ");
+  S.appendInlineCode("bar");
+  EXPECT_EQ(S.renderAsMarkdown(), "foo `bar`");
+
+  S = FormattedString();
+  S.appendText("foo\n");
+  S.appendInlineCode("bar");
+  EXPECT_EQ(S.renderAsMarkdown(), "foo\n`bar`");
+
+  S = FormattedString();
+  S.appendInlineCode("foo");
+  S.appendText(" bar");
+  EXPECT_EQ(S.renderAsMarkdown(), "`foo` bar");
+
+  S = FormattedString();
+  S.appendText("foo");
+  S.appendCodeBlock("bar");
+  S.appendText("baz");
+  EXPECT_EQ(S.renderAsMarkdown(), "foo\n```cpp\nbar\n```\nbaz");
+}
+
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/trunk/clangd/FormattedString.cpp
===
--- clang-tools-extra/trunk/clangd/FormattedString.cpp
+++ clang-tools-extra/trunk/clangd/FormattedString.cpp
@@ -112,15 +112,20 @@
 
 std::string FormattedString::renderAsMarkdown() const {
   std::string R;
+  auto EnsureWhitespace = [&R]() {
+// Adds a space for nicer rendering.
+if (!R.empty() && !isWhitespace(R.back()))
+  R += " ";
+  };
   for (const auto &C : Chunks) {
 switch (C.Kind) {
 case ChunkKind::PlainText:
+  if (!C.Contents.empty() && !isWhitespace(C.Contents.front()))
+EnsureWhitespace();
   R += renderText(C.Contents);
   continue;
 case ChunkKind::InlineCodeBlock:
-  // Make sure we don't glue two backticks together.
-  if (llvm::StringRef(R).endswith("`"))
-R += " ";
+  EnsureWhitespace();
   R += renderInlineBlock(C.Contents);
   continue;
 case ChunkKind::CodeBlock:


Index: clang-tools-extra/trunk/clangd/unittests/FormattedStringTests.cpp
===
--- clang-tools-extra/trunk/clangd/unittests/FormattedStringTests.cpp
+++ clang-tools-extra/trunk/clangd/unittests/FormattedStringTests.cpp
@@ -72,7 +72,7 @@
   S.appendText("baz");
 
   EXPECT_EQ(S.renderAsPlainText(), "foo bar baz");
-  EXPECT_EQ(S.renderAsMarkdown(), "foo`bar`baz");
+  EXPECT_EQ(S.renderAsMarkdown(), "foo `bar` baz");
 }
 
 TEST(FormattedString, Escaping) {
@@ -158,6 +158,42 @@
   "`\n");
 }
 
+TEST(FormattedString, MarkdownWhitespace) {
+  // Whitespace should be added as separators between blocks.
+  FormattedString S;
+  S.appendText("foo");
+  S.appendText("bar");
+  EXPECT_EQ(S.renderAsMarkdown(), "foo bar");
+
+  S = FormattedString();
+  S.appendInlineCode("foo");
+  S.appendInlineCode("bar");
+  EXPECT_EQ(S.renderAsMarkdown(), "`foo` `bar`");
+
+  // However, we don't want to add any extra whitespace.
+  S = FormattedString();
+  S.appendText("foo ");
+  S.appendInlineCode("bar");
+  EXPECT_EQ(S.renderAsMarkdown(), "foo `bar`");
+
+  S = FormattedString();
+  S.appendText("foo\n");
+  S.appendInlineCode("bar");
+  EXPECT_EQ(S.renderAsMarkdown(), "foo\n`bar`");
+
+  S = FormattedString();
+  S.appendInlineCode("foo");
+  S.appendText(" bar");
+  EXPECT_EQ(S.renderAsMarkdown(), "`foo` bar");
+
+  S = FormattedString();
+  S.appendText("foo");
+  S.appendCodeBlock("bar");
+  S.appendText("baz");
+  EXPECT_EQ(S.renderAsMarkdown(), "foo\n```cpp\nbar\n```\nbaz");
+}
+
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/trunk/clangd/FormattedString.cpp
==

[PATCH] D66087: [clangd] Preserve line break when rendering text chunks of markdown

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Added a bit of a braindump to https://github.com/clangd/clangd/issues/95

I think always forcing a break is maybe at least as bad as forcing no break, 
and we should aim to be cleverer.
Some (relatively) simple punctuation/capitalization/line-length based 
heuristics may get us a long way here - is it ok to punt on this for a while?

This change would be consistent with how we handle plaintext, but including the 
breaks seems less problematic there: it's not clear editors are supposed to 
rewrap plaintext if we unwrap it, whereas markdown is very clear about that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66087



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

[PATCH] D66087: [clangd] Preserve line break when rendering text chunks of markdown

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

The heuristics for properly rendering the comments are probably a good way to 
go.
I'd say this change is still a step in the right direction - text blocks in 
formatted strings should be properly escaped to avoid being interpreted like 
markdown constructs.
Any structure we want to have should be explicit.

However, landing this without the aforementioned heuristics means changing our 
behavior and since some folks like the current one better, I'll just keep this 
change in a limbo until we get to those heuristics.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66087



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

[PATCH] D65591: [AST] Add a flag indicating if any subexpression had errors

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added a comment.

In D65591#1625228 , @ilya-biryukov 
wrote:

> In D65591#1625183 , @aaron.ballman 
> wrote:
>
> > In D65591#1625154 , @riccibruno 
> > wrote:
> >
> > > It seems that these two options are not exactly the same right ? The 
> > > `ContainsError` bit is useful to quickly answer "Does this expression 
> > > contains an invalid sub-expression" without doing the search, while 
> > > adding an `ErrorExpr` node is useful to note that //this// sub-expression 
> > > is invalid (and as Aaron says the hypothetical `ErrorExpr` node can carry 
> > > more info about the error).
> >
> >
> > That's true. I had figured that answering "does this expression contain an 
> > invalid sub-expression" could be implemented with a walk of the expression 
> > tree rather than consuming a bit. To consumers of `containsErrors()`, there 
> > shouldn't be much difference aside from performance (and that may be 
> > sufficient reason to go with a bit, but I think I'd like to see performance 
> > measurements that show the bit is necessary).
>
>
> Are expression bits scarce?
>  We don't have any checks if expressions contain errors now, we simply drop 
> too many invalid expressions and never put them into the AST.
>  It's impossible to do the measurements at this point, but it would be nice 
> if adding those checks was cheap.


Sort of... I went through the list of bit-fields classes and if I count 
correctly there is 4 (possibly 5) bits left in `Stmt` or `Expr` which can be 
used without doing anything else. I guess that using one of them is okay.

> We can also assume they're cheap, use the visitor-based implementation and 
> later switch if this turn out to be a problem.
>  I think we should prefer the approach that guarantees the compiler is not 
> getting slow ever rather than chase the slowness later.
> 
> In D65591#1625192 , @riccibruno 
> wrote:
> 
>> Fair enough, especially given that IIRC there are not many bits left in 
>> these bit-fields classes.
> 
> 
> May be reasonable in that case, but we should be aware of a potential 
> `O(n^2)` and even exponential complexities in the frontend arising from 
> calling `containsErrors` in too many places in that case.
>  If we ever run out of bits, we could remove this bit and instead introduce a 
> cache (something like `set InvalidExprs`) in `ASTContext`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65591



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

Re: [clang-tools-extra] r368581 - [clangd] Separate chunks with a space when rendering markdown

[Ilya Biryukov via cfe-commits]

+Hans Wennborg , could we merge this into the release?

On Mon, Aug 12, 2019 at 4:34 PM Ilya Biryukov via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: ibiryukov
> Date: Mon Aug 12 07:35:30 2019
> New Revision: 368581
>
> URL: http://llvm.org/viewvc/llvm-project?rev=368581&view=rev
> Log:
> [clangd] Separate chunks with a space when rendering markdown
>
> Summary:
> This results in better rendering of resulting markdown.
>
> Especially noticeable in coc.nvim that does not have a visible horizontal
> spaces around inline code blocks. More details and a screenshot from
> coc.nvim can be found in https://github.com/clangd/clangd/issues/95.
>
> Reviewers: sammccall
>
> Reviewed By: sammccall
>
> Subscribers: MaskRay, jkorous, arphaman, kadircet, cfe-commits
>
> Tags: #clang
>
> Differential Revision: https://reviews.llvm.org/D66086
>
> Modified:
> clang-tools-extra/trunk/clangd/FormattedString.cpp
> clang-tools-extra/trunk/clangd/unittests/FormattedStringTests.cpp
>
> Modified: clang-tools-extra/trunk/clangd/FormattedString.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/FormattedString.cpp?rev=368581&r1=368580&r2=368581&view=diff
>
> ==
> --- clang-tools-extra/trunk/clangd/FormattedString.cpp (original)
> +++ clang-tools-extra/trunk/clangd/FormattedString.cpp Mon Aug 12 07:35:30
> 2019
> @@ -112,15 +112,20 @@ void FormattedString::appendInlineCode(s
>
>  std::string FormattedString::renderAsMarkdown() const {
>std::string R;
> +  auto EnsureWhitespace = [&R]() {
> +// Adds a space for nicer rendering.
> +if (!R.empty() && !isWhitespace(R.back()))
> +  R += " ";
> +  };
>for (const auto &C : Chunks) {
>  switch (C.Kind) {
>  case ChunkKind::PlainText:
> +  if (!C.Contents.empty() && !isWhitespace(C.Contents.front()))
> +EnsureWhitespace();
>R += renderText(C.Contents);
>continue;
>  case ChunkKind::InlineCodeBlock:
> -  // Make sure we don't glue two backticks together.
> -  if (llvm::StringRef(R).endswith("`"))
> -R += " ";
> +  EnsureWhitespace();
>R += renderInlineBlock(C.Contents);
>continue;
>  case ChunkKind::CodeBlock:
>
> Modified: clang-tools-extra/trunk/clangd/unittests/FormattedStringTests.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/FormattedStringTests.cpp?rev=368581&r1=368580&r2=368581&view=diff
>
> ==
> --- clang-tools-extra/trunk/clangd/unittests/FormattedStringTests.cpp
> (original)
> +++ clang-tools-extra/trunk/clangd/unittests/FormattedStringTests.cpp Mon
> Aug 12 07:35:30 2019
> @@ -72,7 +72,7 @@ after)md";
>S.appendText("baz");
>
>EXPECT_EQ(S.renderAsPlainText(), "foo bar baz");
> -  EXPECT_EQ(S.renderAsMarkdown(), "foo`bar`baz");
> +  EXPECT_EQ(S.renderAsMarkdown(), "foo `bar` baz");
>  }
>
>  TEST(FormattedString, Escaping) {
> @@ -158,6 +158,42 @@ TEST(FormattedString, Escaping) {
>"`\n");
>  }
>
> +TEST(FormattedString, MarkdownWhitespace) {
> +  // Whitespace should be added as separators between blocks.
> +  FormattedString S;
> +  S.appendText("foo");
> +  S.appendText("bar");
> +  EXPECT_EQ(S.renderAsMarkdown(), "foo bar");
> +
> +  S = FormattedString();
> +  S.appendInlineCode("foo");
> +  S.appendInlineCode("bar");
> +  EXPECT_EQ(S.renderAsMarkdown(), "`foo` `bar`");
> +
> +  // However, we don't want to add any extra whitespace.
> +  S = FormattedString();
> +  S.appendText("foo ");
> +  S.appendInlineCode("bar");
> +  EXPECT_EQ(S.renderAsMarkdown(), "foo `bar`");
> +
> +  S = FormattedString();
> +  S.appendText("foo\n");
> +  S.appendInlineCode("bar");
> +  EXPECT_EQ(S.renderAsMarkdown(), "foo\n`bar`");
> +
> +  S = FormattedString();
> +  S.appendInlineCode("foo");
> +  S.appendText(" bar");
> +  EXPECT_EQ(S.renderAsMarkdown(), "`foo` bar");
> +
> +  S = FormattedString();
> +  S.appendText("foo");
> +  S.appendCodeBlock("bar");
> +  S.appendText("baz");
> +  EXPECT_EQ(S.renderAsMarkdown(), "foo\n```cpp\nbar\n```\nbaz");
> +}
> +
> +
>  } // namespace
>  } // namespace clangd
>  } // namespace clang
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>


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

[PATCH] D62046: [OpenMP][bugfix] Add missing math functions variants for log and abs.

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

I'm not sure about this diff. I think it's breaking  and . 
Raised bug https://bugs.llvm.org/show_bug.cgi?id=42972


Repository:
  rC Clang

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

https://reviews.llvm.org/D62046



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

r368588 - Fix multiple lifetime warning messages for range based for loop

[Gabor Horvath via cfe-commits]

Author: xazax
Date: Mon Aug 12 09:19:39 2019
New Revision: 368588

URL: http://llvm.org/viewvc/llvm-project?rev=368588&view=rev
Log:
Fix multiple lifetime warning messages for range based for loop

Modified:
cfe/trunk/lib/Sema/SemaInit.cpp
cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp

Modified: cfe/trunk/lib/Sema/SemaInit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=368588&r1=368587&r2=368588&view=diff
==
--- cfe/trunk/lib/Sema/SemaInit.cpp (original)
+++ cfe/trunk/lib/Sema/SemaInit.cpp Mon Aug 12 09:19:39 2019
@@ -7070,8 +7070,11 @@ static SourceRange nextPathEntryRange(co
   // supporting lifetime extension.
   break;
 
-case IndirectLocalPathEntry::DefaultInit:
 case IndirectLocalPathEntry::VarInit:
+  if (cast(Path[I].D)->isImplicit())
+return SourceRange();
+  LLVM_FALLTHROUGH;
+case IndirectLocalPathEntry::DefaultInit:
   return Path[I].E->getSourceRange();
 }
   }
@@ -7138,7 +7141,7 @@ void Sema::checkInitializerLifetime(cons
 return false;
   }
 
-  if (IsGslPtrInitWithGslTempOwner) {
+  if (IsGslPtrInitWithGslTempOwner && DiagLoc.isValid()) {
 Diag(DiagLoc, diag::warn_dangling_lifetime_pointer) << DiagRange;
 return false;
   }

Modified: cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp?rev=368588&r1=368587&r2=368588&view=diff
==
--- cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp (original)
+++ cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp Mon Aug 12 09:19:39 
2019
@@ -209,6 +209,13 @@ void danglingReferenceFromTempOwner() {
 std::vector getTempVec();
 std::optional> getTempOptVec();
 
+void testLoops() {
+  for (auto i : getTempVec()) // ok
+;
+  for (auto i : *getTempOptVec()) // expected-warning {{object backing the 
pointer will be destroyed at the end of the full-expression}}
+;
+}
+
 int &usedToBeFalsePositive(std::vector &v) {
   std::vector::iterator it = v.begin();
   int& value = *it;


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

[PATCH] D66092: [CodeGen] Generate constrained fp intrinsics depending on FPOptions

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff created this revision.
sepavloff added reviewers: rjmccall, anemet, kpn, aaron.ballman, hfinkel.
Herald added a project: clang.

If the value of FPOption is modified, for example by using pragma
'clang fp', create calls to constrained fp intrinsics with metadata
arguments corresponding to the selected rounding mode and exception
behavior.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66092

Files:
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/test/CodeGen/pragma-fp-2.cpp

Index: clang/test/CodeGen/pragma-fp-2.cpp
===
--- /dev/null
+++ clang/test/CodeGen/pragma-fp-2.cpp
@@ -0,0 +1,79 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -emit-llvm -o - %s | FileCheck %s
+
+void func_01(float x, float y) {
+#pragma clang fp rounding(upward) exceptions(maytrap)
+  float z = x + y;
+// CHECK-LABEL: _Z7func_01ff
+// CHECK: call float @llvm.experimental.constrained.fadd.f32({{.*}}, metadata !"round.upward", metadata !"fpexcept.maytrap")
+}
+
+void func_02(float x, float y) {
+  float z = x + y;
+// CHECK-LABEL: _Z7func_02ff
+// CHECK: fadd float %0, %1
+}
+
+void func_03(float x, float y) {
+  float z1 = x + y;
+  {
+#pragma clang fp rounding(downward) exceptions(strict)
+float z2 = x - y;
+  }
+  float z3 = x * y;
+// CHECK-LABEL: _Z7func_03ff
+// CHECK: fadd float
+// CHECK: call float @llvm.experimental.constrained.fsub.f32({{.*}}, metadata !"round.downward", metadata !"fpexcept.strict")
+// CHECK: fmul float
+}
+
+void func_03a(float x, float y) {
+  float z1 = x + y;
+  {
+#pragma clang fp rounding(downward) exceptions(strict)
+float z2 = x - y;
+{
+  #pragma clang fp rounding(upward) exceptions(maytrap)
+  float z2a = x * y;
+}
+float z2b = x + y;
+  }
+  float z3 = x * y;
+// CHECK-LABEL: _Z8func_03aff
+// CHECK: fadd
+// CHECK: call float @llvm.experimental.constrained.fsub.f32({{.*}}, metadata !"round.downward", metadata !"fpexcept.strict") 
+// CHECK: call float @llvm.experimental.constrained.fmul.f32({{.*}}, metadata !"round.upward", metadata !"fpexcept.maytrap")
+// CHECK: call float @llvm.experimental.constrained.fadd.f32({{.*}}, metadata !"round.downward", metadata !"fpexcept.strict")
+// CHECK: fmul
+}
+
+
+#pragma clang fp rounding(towardzero) exceptions(ignore)
+
+void func_04(float x, float y) {
+  float z = x + y;
+// CHECK-LABEL: _Z7func_04ff
+// CHECK: call float @llvm.experimental.constrained.fadd.f32({{.*}}, metadata !"round.towardzero", metadata !"fpexcept.ignore")
+}
+
+void func_05(float x, float y) {
+  float z = x / y;
+// CHECK-LABEL: _Z7func_05ff
+// CHECK: call float @llvm.experimental.constrained.fdiv.f32({{.*}}, metadata !"round.towardzero", metadata !"fpexcept.ignore")
+}
+
+#pragma clang fp rounding(dynamic) exceptions(maytrap)
+
+void func_06(float x, float y) {
+  float z = x + y;
+// CHECK-LABEL: _Z7func_06ff
+// CHECK: call float @llvm.experimental.constrained.fadd.f32({{.*}}, metadata !"round.dynamic", metadata !"fpexcept.maytrap")
+}
+
+#pragma clang fp rounding(tonearest) exceptions(ignore)
+
+void func_07(float x, float y) {
+  float z = x + y;
+// CHECK-LABEL: _Z7func_07ff
+// CHECK: fadd float
+}
+
Index: clang/lib/CodeGen/CGExprScalar.cpp
===
--- clang/lib/CodeGen/CGExprScalar.cpp
+++ clang/lib/CodeGen/CGExprScalar.cpp
@@ -140,6 +140,34 @@
 }
 return false;
   }
+
+  llvm::ConstrainedFPIntrinsic::RoundingMode getConstrainedRounding() const {
+switch (FPFeatures.getRoundingMode()) {
+case LangOptions::FPRoundingModeKind::ToNearest:
+  return llvm::ConstrainedFPIntrinsic::RoundingMode::rmToNearest;
+case LangOptions::FPRoundingModeKind::Downward:
+  return llvm:: ConstrainedFPIntrinsic::RoundingMode::rmDownward;
+case LangOptions::FPRoundingModeKind::Upward:
+  return llvm::ConstrainedFPIntrinsic::RoundingMode::rmUpward;
+case LangOptions::FPRoundingModeKind::TowardZero:
+  return llvm::ConstrainedFPIntrinsic::RoundingMode::rmTowardZero;
+case LangOptions::FPRoundingModeKind::Dynamic:
+  return llvm::ConstrainedFPIntrinsic::RoundingMode::rmDynamic;
+}
+return llvm::ConstrainedFPIntrinsic::RoundingMode::rmDynamic;
+  }
+
+  llvm::ConstrainedFPIntrinsic::ExceptionBehavior getConstrainedExcept() const {
+switch (FPFeatures.getExceptionMode()) {
+case LangOptions::FPExceptionModeKind::Ignore:
+  return llvm::ConstrainedFPIntrinsic::ExceptionBehavior::ebIgnore;
+case LangOptions::FPExceptionModeKind::MayTrap:
+  return llvm::ConstrainedFPIntrinsic::ExceptionBehavior::ebMayTrap;
+case LangOptions::FPExceptionModeKind::Strict:
+  return llvm::ConstrainedFPIntrinsic::ExceptionBehavior::ebStrict;
+}
+return llvm::ConstrainedFPIntrinsic::ExceptionBehavior::ebStrict;
+  }
 };
 
 static bool MustVisitNullValue(const Expr *E) {
@@ -728,6 +756,9 @@
   return EmitOverflowCheckedBinOp(Ops);
 
 i

[PATCH] D66092: [CodeGen] Generate constrained fp intrinsics depending on FPOptions

[Kevin P. Neal via Phabricator via cfe-commits]

kpn added a comment.

Does this work for anything that uses TreeTransform, like C++ templates?

Also, if any constrained intrinsics are used in a function then the entire 
function needs to be constrained. Is this handled anywhere?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66092



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

[PATCH] D66014: [analyzer] Avoid unnecessary enum range check on LValueToRValue casts

[Chris Hamilton via Phabricator via cfe-commits]

chrish_ericsson_atx marked 3 inline comments as done.
chrish_ericsson_atx added inline comments.



Comment at: 
clang/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp:94-98
+  // If cast is implicit LValueToRValue, no conversion is taking place,
+  // and therefore no range check is needed.  Don't analyze further.
+  if (CE->getCastKind() == CK_LValueToRValue)
+return;
+

NoQ wrote:
> If you look at the list of cast kinds, you'll be shocked to see ridiculously 
> weird cornercases. Even though lvalue-to-rvalue cast definitely stands out 
> (because it's the only one that has very little to do with actual casting), 
> i'd still be much more comfortable if we *whitelist* the casts that we check, 
> rather than blacklist them.
> 
> Can you take a look at the list and find which casts are we actually talking 
> about and hardcode them here?
I'm happy to cross-check a list; however, I'm not aware of the list you are 
referring to.  Would you mind providing a bit more detail?



Comment at: 
clang/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp:121-122
   // Check if any of the enum values possibly match.
   bool PossibleValueMatch = llvm::any_of(
   DeclValues, ConstraintBasedEQEvaluator(C, *ValueToCast));
 

Szelethus wrote:
> So this is where the assertion comes from, and will eventually lead to 
> `SValBuilder::evalEQ`, which calls `SValBuilder::evalBinOp`, where this will 
> fire on line 427:
> ```
> assert(op == BO_Add);
> ```
> Seems like this happens because `unused`'s value in your testcase will be 
> retrieved as a `Loc`, while the values in the enum are (correctly) `NonLoc`, 
> and `SValBuilder::evalBinOp` thinks this is some sort of pointer arithmetic 
> (`5 + ptr` etc).
> 
> How about instead of checking for LValueToRValue cast, we check whether 
> `ValueToCast` is `Loc`, and bail out if so? That sounds like a more general 
> solution, but I didn't sit atop of this for hours.
Is this the only case where ValueToCast will be Loc?   I was unsure about the 
implications of Loc vs. NonLoc in this code, and I didn't want to risk breaking 
a check that should have been valid.  That's why I opted for bailing on an 
LValueToRValue cast at a higher level-- that seemed much safer to me.  I 
certainly might be missing something, but I couldn't find any evidence that an 
LValueToRValue cast should ever need to be checked for enum range errors.   I 
will certainly defer to your judgement, but I'd like to have a better 
understanding of why looking for ValueToCast == Loc would actually be more 
correct.



Comment at: clang/test/Analysis/enum-cast-out-of-range.c:27-34
+enum unscoped_unspecified_t unused;
+void unusedExpr() {
+// following line is not something that EnumCastOutOfRangeChecker should 
evaluate.  checker should either ignore this line
+// or process it without producing any warnings.  However, compilation 
will (and should) still generate a warning having 
+// nothing to do with this checker.
+unused; // expected-warning {{expression result unused}}
+}

NoQ wrote:
> I guess this covers D33672#1537287!
> 
> That said, there's one thing about this test that i don't understand. Why 
> does this AST contain an implicit lvalue-to-rvalue cast at all? This looks 
> like a (most likely otherwise harmless) bug in the AST; if i understand 
> correctly, this should be a freestanding `DeclRefExpr`.
It sure looks like D33672 is the same bug, so yes, I bet it will fix that one.  
 This fix also addresses https://bugs.llvm.org/show_bug.cgi?id=41388.  (Forgive 
me if there's a way to directly link to bugs.llvm.org with this markup.)

Not sure about whether the AST should have represented this node as a 
DeclRefExpr-- that certainly makes some sense, but the LValueToRValue cast 
isn't really wrong either.   At the end of the day, this statement is useless, 
so I'm not sure it matters (much) how the AST represents it.  Representing it 
as LValueToRValue cast wouldn't cause side effects, would it? (E.g.,  cause IR 
to contain explicit dereferencing code?)


Repository:
  rC Clang

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

https://reviews.llvm.org/D66014



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

[PATCH] D64464: [CodeGen] Emit destructor calls for non-trivial C structs

[Akira Hatanaka via Phabricator via cfe-commits]

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



Comment at: lib/CodeGen/CGExpr.cpp:4647
+pushDestroy(QualType::DK_nontrivial_c_struct, RV.getAggregateAddress(),
+E->getType());
+

rjmccall wrote:
> Does `EmitCallExpr` not enter a cleanup when it returns an aggregate that's 
> not into an externally-destructed slot?  That seems wrong and dangerous.
I'm going to split this patch into two parts, one for compound literals and the 
other for everything else. The patch is getting too large and I also found 
another bug: a cleanup isn't pushed for ObjC message send.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64464



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

[clang-tools-extra] r368590 - [clangd] Refactor computation of extracted expr in ExtractVariable tweak. NFC

[Sam McCall via cfe-commits]

Author: sammccall
Date: Mon Aug 12 10:05:35 2019
New Revision: 368590

URL: http://llvm.org/viewvc/llvm-project?rev=368590&view=rev
Log:
[clangd] Refactor computation of extracted expr in ExtractVariable tweak. NFC

Summary:
This takes this logic out of the Tweak class, and simplifies the signature of
the function where the main logic is.

The goal is to make it easier to turn into a loop like:

  for (current = N; current and current->parent are both expr; current = 
current->parent)
if (suitable(current))
  return current;
  return null;

Reviewers: SureYeaah

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/refactor/tweaks/ExtractVariable.cpp

Modified: clang-tools-extra/trunk/clangd/refactor/tweaks/ExtractVariable.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/refactor/tweaks/ExtractVariable.cpp?rev=368590&r1=368589&r2=368590&view=diff
==
--- clang-tools-extra/trunk/clangd/refactor/tweaks/ExtractVariable.cpp 
(original)
+++ clang-tools-extra/trunk/clangd/refactor/tweaks/ExtractVariable.cpp Mon Aug 
12 10:05:35 2019
@@ -319,55 +319,6 @@ SourceRange ExtractionContext::getExtrac
   return *toHalfOpenFileRange(SM, Ctx.getLangOpts(), Expr->getSourceRange());
 }
 
-/// Extracts an expression to the variable dummy
-/// Before:
-/// int x = 5 + 4 * 3;
-/// ^
-/// After:
-/// auto dummy = 5 + 4;
-/// int x = dummy * 3;
-class ExtractVariable : public Tweak {
-public:
-  const char *id() const override final;
-  bool prepare(const Selection &Inputs) override;
-  Expected apply(const Selection &Inputs) override;
-  std::string title() const override {
-return "Extract subexpression to variable";
-  }
-  Intent intent() const override { return Refactor; }
-  // Compute the extraction context for the Selection
-  bool computeExtractionContext(const SelectionTree::Node *N,
-const SourceManager &SM, const ASTContext 
&Ctx);
-
-private:
-  // the expression to extract
-  std::unique_ptr Target;
-};
-REGISTER_TWEAK(ExtractVariable)
-bool ExtractVariable::prepare(const Selection &Inputs) {
-  // we don't trigger on empty selections for now
-  if (Inputs.SelectionBegin == Inputs.SelectionEnd)
-return false;
-  const ASTContext &Ctx = Inputs.AST.getASTContext();
-  const SourceManager &SM = Inputs.AST.getSourceManager();
-  const SelectionTree::Node *N = Inputs.ASTSelection.commonAncestor();
-  return computeExtractionContext(N, SM, Ctx);
-}
-
-Expected ExtractVariable::apply(const Selection &Inputs) {
-  tooling::Replacements Result;
-  // FIXME: get variable name from user or suggest based on type
-  std::string VarName = "dummy";
-  SourceRange Range = Target->getExtractionChars();
-  // insert new variable declaration
-  if (auto Err = Result.add(Target->insertDeclaration(VarName, Range)))
-return std::move(Err);
-  // replace expression with variable name
-  if (auto Err = Result.add(Target->replaceWithVar(Range, VarName)))
-return std::move(Err);
-  return Effect::applyEdit(Result);
-}
-
 // Find the CallExpr whose callee is the (possibly wrapped) DeclRef
 const SelectionTree::Node *getCallExpr(const SelectionTree::Node *DeclRef) {
   const SelectionTree::Node &MaybeCallee = DeclRef->outerImplicit();
@@ -445,25 +396,79 @@ bool eligibleForExtraction(const Selecti
   return true;
 }
 
-// Find the node that will form our ExtractionContext.
+// Find the Expr node that we're going to extract.
 // We don't want to trigger for assignment expressions and variable/field
 // DeclRefs. For function/member function, we want to extract the entire
 // function call.
-bool ExtractVariable::computeExtractionContext(const SelectionTree::Node *N,
-   const SourceManager &SM,
-   const ASTContext &Ctx) {
+const SelectionTree::Node *computeExtractedExpr(const SelectionTree::Node *N) {
   if (!N)
-return false;
+return nullptr;
   const SelectionTree::Node *TargetNode = N;
+  const clang::Expr *SelectedExpr = N->ASTNode.get();
+  if (!SelectedExpr)
+return nullptr;
   // For function and member function DeclRefs, extract the whole call.
-  if (const Expr *E = N->ASTNode.get())
-if (llvm::isa(E) || llvm::isa(E))
-  if (const SelectionTree::Node *Call = getCallExpr(N))
-TargetNode = Call;
-  if (!eligibleForExtraction(TargetNode))
+  if (llvm::isa(SelectedExpr) ||
+  llvm::isa(SelectedExpr))
+if (const SelectionTree::Node *Call = getCallExpr(N))
+  TargetNode = Call;
+  // Extracting Exprs like a = 1 gives dummy = a = 1 which isn't useful.
+  if (const BinaryOperator *BinOpExpr =
+  dyn_cast_or_null(SelectedExpr)) {
+if (BinOpExpr->getOpcode() == BinaryOperatorKind::BO_Assign)
+  ret

r368591 - [ASTDump] Add is_anonymous to VisitCXXRecordDecl

[Shafik Yaghmour via cfe-commits]

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

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

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

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

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

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

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


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

[PATCH] D65333: [clangd] Refactor computation of extracted expr in ExtractVariable tweak. NFC

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 214652.
sammccall added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65333

Files:
  clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp

Index: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
@@ -319,55 +319,6 @@
   return *toHalfOpenFileRange(SM, Ctx.getLangOpts(), Expr->getSourceRange());
 }
 
-/// Extracts an expression to the variable dummy
-/// Before:
-/// int x = 5 + 4 * 3;
-/// ^
-/// After:
-/// auto dummy = 5 + 4;
-/// int x = dummy * 3;
-class ExtractVariable : public Tweak {
-public:
-  const char *id() const override final;
-  bool prepare(const Selection &Inputs) override;
-  Expected apply(const Selection &Inputs) override;
-  std::string title() const override {
-return "Extract subexpression to variable";
-  }
-  Intent intent() const override { return Refactor; }
-  // Compute the extraction context for the Selection
-  bool computeExtractionContext(const SelectionTree::Node *N,
-const SourceManager &SM, const ASTContext &Ctx);
-
-private:
-  // the expression to extract
-  std::unique_ptr Target;
-};
-REGISTER_TWEAK(ExtractVariable)
-bool ExtractVariable::prepare(const Selection &Inputs) {
-  // we don't trigger on empty selections for now
-  if (Inputs.SelectionBegin == Inputs.SelectionEnd)
-return false;
-  const ASTContext &Ctx = Inputs.AST.getASTContext();
-  const SourceManager &SM = Inputs.AST.getSourceManager();
-  const SelectionTree::Node *N = Inputs.ASTSelection.commonAncestor();
-  return computeExtractionContext(N, SM, Ctx);
-}
-
-Expected ExtractVariable::apply(const Selection &Inputs) {
-  tooling::Replacements Result;
-  // FIXME: get variable name from user or suggest based on type
-  std::string VarName = "dummy";
-  SourceRange Range = Target->getExtractionChars();
-  // insert new variable declaration
-  if (auto Err = Result.add(Target->insertDeclaration(VarName, Range)))
-return std::move(Err);
-  // replace expression with variable name
-  if (auto Err = Result.add(Target->replaceWithVar(Range, VarName)))
-return std::move(Err);
-  return Effect::applyEdit(Result);
-}
-
 // Find the CallExpr whose callee is the (possibly wrapped) DeclRef
 const SelectionTree::Node *getCallExpr(const SelectionTree::Node *DeclRef) {
   const SelectionTree::Node &MaybeCallee = DeclRef->outerImplicit();
@@ -445,25 +396,79 @@
   return true;
 }
 
-// Find the node that will form our ExtractionContext.
+// Find the Expr node that we're going to extract.
 // We don't want to trigger for assignment expressions and variable/field
 // DeclRefs. For function/member function, we want to extract the entire
 // function call.
-bool ExtractVariable::computeExtractionContext(const SelectionTree::Node *N,
-   const SourceManager &SM,
-   const ASTContext &Ctx) {
+const SelectionTree::Node *computeExtractedExpr(const SelectionTree::Node *N) {
   if (!N)
-return false;
+return nullptr;
   const SelectionTree::Node *TargetNode = N;
+  const clang::Expr *SelectedExpr = N->ASTNode.get();
+  if (!SelectedExpr)
+return nullptr;
   // For function and member function DeclRefs, extract the whole call.
-  if (const Expr *E = N->ASTNode.get())
-if (llvm::isa(E) || llvm::isa(E))
-  if (const SelectionTree::Node *Call = getCallExpr(N))
-TargetNode = Call;
-  if (!eligibleForExtraction(TargetNode))
+  if (llvm::isa(SelectedExpr) ||
+  llvm::isa(SelectedExpr))
+if (const SelectionTree::Node *Call = getCallExpr(N))
+  TargetNode = Call;
+  // Extracting Exprs like a = 1 gives dummy = a = 1 which isn't useful.
+  if (const BinaryOperator *BinOpExpr =
+  dyn_cast_or_null(SelectedExpr)) {
+if (BinOpExpr->getOpcode() == BinaryOperatorKind::BO_Assign)
+  return nullptr;
+  }
+  if (!TargetNode || !eligibleForExtraction(TargetNode))
+return nullptr;
+  return TargetNode;
+}
+
+/// Extracts an expression to the variable dummy
+/// Before:
+/// int x = 5 + 4 * 3;
+/// ^
+/// After:
+/// auto dummy = 5 + 4;
+/// int x = dummy * 3;
+class ExtractVariable : public Tweak {
+public:
+  const char *id() const override final;
+  bool prepare(const Selection &Inputs) override;
+  Expected apply(const Selection &Inputs) override;
+  std::string title() const override {
+return "Extract subexpression to variable";
+  }
+  Intent intent() const override { return Refactor; }
+
+private:
+  // the expression to extract
+  std::unique_ptr Target;
+};
+REGISTER_TWEAK(ExtractVariable)
+bool ExtractVariable::prepare(const Selection &Inputs) {
+  // we don't t

[PATCH] D66028: [ASTDump] Add is_anonymous to VisitCXXRecordDecl

[Shafik Yaghmour via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL368591: [ASTDump] Add is_anonymous to VisitCXXRecordDecl 
(authored by shafik, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66028?vs=214436&id=214655#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66028

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


Index: cfe/trunk/lib/AST/TextNodeDumper.cpp
===
--- cfe/trunk/lib/AST/TextNodeDumper.cpp
+++ cfe/trunk/lib/AST/TextNodeDumper.cpp
@@ -1537,6 +1537,7 @@
 FLAG(isGenericLambda, generic);
 FLAG(isLambda, lambda);
 
+FLAG(isAnonymousStructOrUnion, is_anonymous);
 FLAG(canPassInRegisters, pass_in_registers);
 FLAG(isEmpty, empty);
 FLAG(isAggregate, aggregate);
Index: cfe/trunk/test/AST/ast-dump-records.cpp
===
--- cfe/trunk/test/AST/ast-dump-records.cpp
+++ cfe/trunk/test/AST/ast-dump-records.cpp
@@ -65,7 +65,7 @@
 
   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 {
 // 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 {
 // 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 @@
 
   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


Index: cfe/trunk/lib/AST/TextNodeDumper.cpp
===
--- cfe/trunk/lib/AST/TextNodeDumper.cpp
+++ cfe/trunk/lib/AST/TextNodeDumper.cpp
@@ -1537,6 +1537,7 @@
 FLAG(isGenericLambda, generic);
 FLAG(isLambda, lambda);
 
+FLAG(isAnonymousStructOrUnion, is_anonymous);
 FLAG(canPassInRegisters, pass_in_registers);
 FLAG(isEmpty, empty);
 FLAG(isAggregate, aggregate);
Index: cfe/trunk/test/AST/ast-dump-records.cpp
===
--- cfe/trunk/test/AST/ast-dump-records.cpp
+++ cfe/trunk/test/AST/ast-dump-records.cpp
@@ -65,7 +65,7 @@
 
   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 {
 // CHECK-NEXT: CXXRecord

[PATCH] D65333: [clangd] Refactor computation of extracted expr in ExtractVariable tweak. NFC

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6a3c2c84be2b: [clangd] Refactor computation of extracted 
expr in ExtractVariable tweak. NFC (authored by sammccall).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65333

Files:
  clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp

Index: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
@@ -319,55 +319,6 @@
   return *toHalfOpenFileRange(SM, Ctx.getLangOpts(), Expr->getSourceRange());
 }
 
-/// Extracts an expression to the variable dummy
-/// Before:
-/// int x = 5 + 4 * 3;
-/// ^
-/// After:
-/// auto dummy = 5 + 4;
-/// int x = dummy * 3;
-class ExtractVariable : public Tweak {
-public:
-  const char *id() const override final;
-  bool prepare(const Selection &Inputs) override;
-  Expected apply(const Selection &Inputs) override;
-  std::string title() const override {
-return "Extract subexpression to variable";
-  }
-  Intent intent() const override { return Refactor; }
-  // Compute the extraction context for the Selection
-  bool computeExtractionContext(const SelectionTree::Node *N,
-const SourceManager &SM, const ASTContext &Ctx);
-
-private:
-  // the expression to extract
-  std::unique_ptr Target;
-};
-REGISTER_TWEAK(ExtractVariable)
-bool ExtractVariable::prepare(const Selection &Inputs) {
-  // we don't trigger on empty selections for now
-  if (Inputs.SelectionBegin == Inputs.SelectionEnd)
-return false;
-  const ASTContext &Ctx = Inputs.AST.getASTContext();
-  const SourceManager &SM = Inputs.AST.getSourceManager();
-  const SelectionTree::Node *N = Inputs.ASTSelection.commonAncestor();
-  return computeExtractionContext(N, SM, Ctx);
-}
-
-Expected ExtractVariable::apply(const Selection &Inputs) {
-  tooling::Replacements Result;
-  // FIXME: get variable name from user or suggest based on type
-  std::string VarName = "dummy";
-  SourceRange Range = Target->getExtractionChars();
-  // insert new variable declaration
-  if (auto Err = Result.add(Target->insertDeclaration(VarName, Range)))
-return std::move(Err);
-  // replace expression with variable name
-  if (auto Err = Result.add(Target->replaceWithVar(Range, VarName)))
-return std::move(Err);
-  return Effect::applyEdit(Result);
-}
-
 // Find the CallExpr whose callee is the (possibly wrapped) DeclRef
 const SelectionTree::Node *getCallExpr(const SelectionTree::Node *DeclRef) {
   const SelectionTree::Node &MaybeCallee = DeclRef->outerImplicit();
@@ -445,25 +396,79 @@
   return true;
 }
 
-// Find the node that will form our ExtractionContext.
+// Find the Expr node that we're going to extract.
 // We don't want to trigger for assignment expressions and variable/field
 // DeclRefs. For function/member function, we want to extract the entire
 // function call.
-bool ExtractVariable::computeExtractionContext(const SelectionTree::Node *N,
-   const SourceManager &SM,
-   const ASTContext &Ctx) {
+const SelectionTree::Node *computeExtractedExpr(const SelectionTree::Node *N) {
   if (!N)
-return false;
+return nullptr;
   const SelectionTree::Node *TargetNode = N;
+  const clang::Expr *SelectedExpr = N->ASTNode.get();
+  if (!SelectedExpr)
+return nullptr;
   // For function and member function DeclRefs, extract the whole call.
-  if (const Expr *E = N->ASTNode.get())
-if (llvm::isa(E) || llvm::isa(E))
-  if (const SelectionTree::Node *Call = getCallExpr(N))
-TargetNode = Call;
-  if (!eligibleForExtraction(TargetNode))
+  if (llvm::isa(SelectedExpr) ||
+  llvm::isa(SelectedExpr))
+if (const SelectionTree::Node *Call = getCallExpr(N))
+  TargetNode = Call;
+  // Extracting Exprs like a = 1 gives dummy = a = 1 which isn't useful.
+  if (const BinaryOperator *BinOpExpr =
+  dyn_cast_or_null(SelectedExpr)) {
+if (BinOpExpr->getOpcode() == BinaryOperatorKind::BO_Assign)
+  return nullptr;
+  }
+  if (!TargetNode || !eligibleForExtraction(TargetNode))
+return nullptr;
+  return TargetNode;
+}
+
+/// Extracts an expression to the variable dummy
+/// Before:
+/// int x = 5 + 4 * 3;
+/// ^
+/// After:
+/// auto dummy = 5 + 4;
+/// int x = dummy * 3;
+class ExtractVariable : public Tweak {
+public:
+  const char *id() const override final;
+  bool prepare(const Selection &Inputs) override;
+  Expected apply(const Selection &Inputs) override;
+  std::string title() const override {
+return "Extract subexpression to variable";
+  }
+  Intent intent() const override { return Refactor; }
+
+private:
+  // the expression to extract
+  std::uni

[PATCH] D66094: [CodeGen] Emit destructor calls for non-trivial C structs returned by function calls and loaded from volatile objects

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak created this revision.
ahatanak added a reviewer: rjmccall.
ahatanak added a project: clang.
Herald added subscribers: llvm-commits, dexonsmith, jkorous.
Herald added a project: LLVM.

This is the patch I split out of https://reviews.llvm.org/D64464.

The cleanup is pushed in `EmitCallExpr` and `EmitObjCMessageExpr` so that the 
destructor is called to destruct function call and ObjC message returns. I also 
added test cases for block function calls since the patch in 
https://reviews.llvm.org/D64464 wasn't handling that case correctly.

rdar://problem/51867864


Repository:
  rC Clang

https://reviews.llvm.org/D66094

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGCall.h
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprAgg.cpp
  clang/lib/CodeGen/CGObjC.cpp
  clang/lib/Parse/ParseObjc.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/test/CodeGenObjC/arc.m
  clang/test/CodeGenObjC/strong-in-c-struct.m
  llvm/test/Bitcode/upgrade-arc-runtime-calls.bc
  llvm/test/Bitcode/upgrade-mrr-runtime-calls.bc

Index: clang/test/CodeGenObjC/strong-in-c-struct.m
===
--- clang/test/CodeGenObjC/strong-in-c-struct.m
+++ clang/test/CodeGenObjC/strong-in-c-struct.m
@@ -90,6 +90,13 @@
 void calleeStrongSmall(StrongSmall);
 void func(Strong *);
 
+@interface C
+-(StrongSmall)getStrongSmall;
++(StrongSmall)getStrongSmallClass;
+@end
+
+id g0;
+
 // CHECK: %[[STRUCT_STRONGOUTER:.*]] = type { %[[STRUCT_STRONG:.*]], i8*, double }
 // CHECK: %[[STRUCT_STRONG]] = type { %[[STRUCT_TRIVIAL:.*]], i8* }
 // CHECK: %[[STRUCT_TRIVIAL]] = type { [4 x i32] }
@@ -477,6 +484,18 @@
   getStrongSmall();
 }
 
+// CHECK: define void @test_destructor_ignored_result2(%{{.*}}* %[[C:.*]])
+// CHECK: %[[TMP:.*]] = alloca %[[STRUCT_STRONGSMALL]], align 8
+// CHECK: %[[CALL:.*]] = call [2 x i64]{{.*}}@objc_msgSend
+// CHECK: %[[V5:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[TMP]] to [2 x i64]*
+// CHECK: store [2 x i64] %[[CALL]], [2 x i64]* %[[V5]], align 8
+// CHECK: %[[V6:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[TMP]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V6]])
+
+void test_destructor_ignored_result2(C *c) {
+  [c getStrongSmall];
+}
+
 // CHECK: define void @test_copy_constructor_StrongBlock(
 // CHECK: call void @__copy_constructor_8_8_sb0(
 // CHECK: call void @__destructor_8_sb0(
@@ -521,7 +540,9 @@
 
 // CHECK: define void @test_copy_constructor_StrongVolatile0(
 // CHECK: call void @__copy_constructor_8_8_t0w4_sv8(
+// CHECK-NOT: call
 // CHECK: call void @__destructor_8_sv8(
+// CHECK-NOT: call
 
 // CHECK: define linkonce_odr hidden void @__copy_constructor_8_8_t0w4_sv8(
 // CHECK: %[[V8:.*]] = load volatile i8*, i8** %{{.*}}, align 8
@@ -718,4 +739,62 @@
   VolatileArray t = *a;
 }
 
+// CHECK: define void @test_member_access(
+// CHECK: %[[TMP:.*]] = alloca %[[STRUCT_STRONGSMALL]],
+// CHECK: %[[V3:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[TMP]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V3]])
+// CHECK: call void @func(
+
+void test_member_access(void) {
+  g0 = getStrongSmall().f1;
+  func(0);
+}
+
+// CHECK: define void @test_member_access2(%{{.*}}* %[[C:.*]])
+// CHECK: %[[COERCE:.*]] = alloca %[[STRUCT_STRONGSMALL]], align 8
+// CHECK: %[[V8:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[COERCE]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V8]])
+// CHECK: call void @func(
+
+void test_member_access2(C *c) {
+  g0 = [c getStrongSmall].f1;
+  func(0);
+}
+
+// CHECK: define void @test_member_access3(
+// CHECK: %[[COERCE:.*]] = alloca %[[STRUCT_STRONGSMALL]], align 8
+// CHECK: %[[V8:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[COERCE]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V8]])
+// CHECK: call void @func(
+
+void test_member_access3(void) {
+  g0 = [C getStrongSmallClass].f1;
+  func(0);
+}
+
+// CHECK: define void @test_member_access4()
+// CHECK: %[[COERCE:.*]] = alloca %[[STRUCT_STRONGSMALL]], align 8
+// CHECK: %[[V5:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[COERCE]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V5]])
+// CHECK: call void @func(
+
+void test_member_access4(void) {
+  g0 = ^{ StrongSmall s; return s; }().f1;
+  func(0);
+}
+
+// CHECK: define void @test_volatile_variable_reference(
+// CHECK: %[[AGG_TMP_ENSURED:.*]] = alloca %[[STRUCT_STRONGSMALL]],
+// CHECK: %[[V1:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[AGG_TMP_ENSURED]] to i8**
+// CHECK: %[[V2:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %{{.*}} to i8**
+// CHECK: call void @__copy_constructor_8_8_tv0w32_sv8(i8** %[[V1]], i8** %[[V2]])
+// CHECK: %[[V3:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[AGG_TMP_ENSURED]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V3]])
+// CHECK: call void @func(
+
+void test_volatile_variable_reference(volatile StrongSmall *a) {
+  (void)*a;
+  func(0);
+}
+
 #endif /* USESTRUCT */
Index: clang/test/CodeGenObjC/arc.m
===

[PATCH] D65994: Extended FPOptions with new attributes

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Since this setting changes IR output, you should write a test that emits IR for 
source code and tests that you get the right IR output.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65994



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

[PATCH] D66094: [CodeGen] Emit destructor calls for non-trivial C structs returned by function calls and loaded from volatile objects

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak updated this revision to Diff 214660.
ahatanak added a comment.

Revert changes I made to llvm that are unrelated to this patch.


Repository:
  rC Clang

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

https://reviews.llvm.org/D66094

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGCall.h
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprAgg.cpp
  clang/lib/CodeGen/CGObjC.cpp
  clang/lib/Parse/ParseObjc.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/test/CodeGenObjC/arc.m
  clang/test/CodeGenObjC/strong-in-c-struct.m

Index: clang/test/CodeGenObjC/strong-in-c-struct.m
===
--- clang/test/CodeGenObjC/strong-in-c-struct.m
+++ clang/test/CodeGenObjC/strong-in-c-struct.m
@@ -90,6 +90,13 @@
 void calleeStrongSmall(StrongSmall);
 void func(Strong *);
 
+@interface C
+-(StrongSmall)getStrongSmall;
++(StrongSmall)getStrongSmallClass;
+@end
+
+id g0;
+
 // CHECK: %[[STRUCT_STRONGOUTER:.*]] = type { %[[STRUCT_STRONG:.*]], i8*, double }
 // CHECK: %[[STRUCT_STRONG]] = type { %[[STRUCT_TRIVIAL:.*]], i8* }
 // CHECK: %[[STRUCT_TRIVIAL]] = type { [4 x i32] }
@@ -477,6 +484,18 @@
   getStrongSmall();
 }
 
+// CHECK: define void @test_destructor_ignored_result2(%{{.*}}* %[[C:.*]])
+// CHECK: %[[TMP:.*]] = alloca %[[STRUCT_STRONGSMALL]], align 8
+// CHECK: %[[CALL:.*]] = call [2 x i64]{{.*}}@objc_msgSend
+// CHECK: %[[V5:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[TMP]] to [2 x i64]*
+// CHECK: store [2 x i64] %[[CALL]], [2 x i64]* %[[V5]], align 8
+// CHECK: %[[V6:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[TMP]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V6]])
+
+void test_destructor_ignored_result2(C *c) {
+  [c getStrongSmall];
+}
+
 // CHECK: define void @test_copy_constructor_StrongBlock(
 // CHECK: call void @__copy_constructor_8_8_sb0(
 // CHECK: call void @__destructor_8_sb0(
@@ -521,7 +540,9 @@
 
 // CHECK: define void @test_copy_constructor_StrongVolatile0(
 // CHECK: call void @__copy_constructor_8_8_t0w4_sv8(
+// CHECK-NOT: call
 // CHECK: call void @__destructor_8_sv8(
+// CHECK-NOT: call
 
 // CHECK: define linkonce_odr hidden void @__copy_constructor_8_8_t0w4_sv8(
 // CHECK: %[[V8:.*]] = load volatile i8*, i8** %{{.*}}, align 8
@@ -718,4 +739,62 @@
   VolatileArray t = *a;
 }
 
+// CHECK: define void @test_member_access(
+// CHECK: %[[TMP:.*]] = alloca %[[STRUCT_STRONGSMALL]],
+// CHECK: %[[V3:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[TMP]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V3]])
+// CHECK: call void @func(
+
+void test_member_access(void) {
+  g0 = getStrongSmall().f1;
+  func(0);
+}
+
+// CHECK: define void @test_member_access2(%{{.*}}* %[[C:.*]])
+// CHECK: %[[COERCE:.*]] = alloca %[[STRUCT_STRONGSMALL]], align 8
+// CHECK: %[[V8:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[COERCE]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V8]])
+// CHECK: call void @func(
+
+void test_member_access2(C *c) {
+  g0 = [c getStrongSmall].f1;
+  func(0);
+}
+
+// CHECK: define void @test_member_access3(
+// CHECK: %[[COERCE:.*]] = alloca %[[STRUCT_STRONGSMALL]], align 8
+// CHECK: %[[V8:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[COERCE]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V8]])
+// CHECK: call void @func(
+
+void test_member_access3(void) {
+  g0 = [C getStrongSmallClass].f1;
+  func(0);
+}
+
+// CHECK: define void @test_member_access4()
+// CHECK: %[[COERCE:.*]] = alloca %[[STRUCT_STRONGSMALL]], align 8
+// CHECK: %[[V5:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[COERCE]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V5]])
+// CHECK: call void @func(
+
+void test_member_access4(void) {
+  g0 = ^{ StrongSmall s; return s; }().f1;
+  func(0);
+}
+
+// CHECK: define void @test_volatile_variable_reference(
+// CHECK: %[[AGG_TMP_ENSURED:.*]] = alloca %[[STRUCT_STRONGSMALL]],
+// CHECK: %[[V1:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[AGG_TMP_ENSURED]] to i8**
+// CHECK: %[[V2:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %{{.*}} to i8**
+// CHECK: call void @__copy_constructor_8_8_tv0w32_sv8(i8** %[[V1]], i8** %[[V2]])
+// CHECK: %[[V3:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[AGG_TMP_ENSURED]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V3]])
+// CHECK: call void @func(
+
+void test_volatile_variable_reference(volatile StrongSmall *a) {
+  (void)*a;
+  func(0);
+}
+
 #endif /* USESTRUCT */
Index: clang/test/CodeGenObjC/arc.m
===
--- clang/test/CodeGenObjC/arc.m
+++ clang/test/CodeGenObjC/arc.m
@@ -1536,12 +1536,13 @@
 
 // CHECK-LABEL: define void @test71
 void test71(void) {
-  // FIXME: It would be nice if the __destructor_8_s40 for the first call (and
-  // the following lifetime.end) came before the second call.
-  //
   // CHECK: %[[T:[^ ]+]] = bitcast %struct.AggDtor* %[[TMP1:[^ ]+]] to i8*
   // CHECK: call void @llvm.lifetime.start.p0i

[PATCH] D66068: cmake: Make building clang-shlib optional

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.

I generally am not a fan of adding more and more options. As long as you're not 
linking the library it won't be generated by any of the check targets. With the 
llvm dylib we've had many issues over the years where changes to LLVM break 
building the dylib and many developers don't build it, so we have to wait for a 
bot to catch it. Making the clang dylib build as part of `all` in every 
possible build configuration is a recognition that this is something we ship 
and we should ensure works.


Repository:
  rC Clang

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

https://reviews.llvm.org/D66068



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

[PATCH] D47419: [SemaDeclCXX] Allow inheriting constructor declaration that specify a cv-qualified type

[John McCall via Phabricator via cfe-commits]

rjmccall accepted this revision.
rjmccall added inline comments.
This revision is now accepted and ready to land.



Comment at: lib/Sema/SemaDeclCXX.cpp:9690
+  CanQualType CanonicalDesiredBase = DesiredBase->getCanonicalTypeUnqualified()
+.getUnqualifiedType();
   for (auto &Base : Derived->bases()) {

cpplearner wrote:
> rsmith wrote:
> > How are we getting a qualified type here? Is this actually a bug in 
> > `getCanonicalTypeUnqualified`?
> It seems that `getCanonicalTypeUnqualified` does not strip qualifiers from 
> the canonical type. I guess "Unqualified" here just means the method does not 
> include local qualifiers, unlike QualType::getCanonicalType.
> 
> Thus, in the case of `using cbase = const base;`, 
> `getCanonicalTypeUnqualified` will return the canonical type of `cbase` as 
> is, which is `const base`, a const-qualified type.
Maybe we should have a method that promises to return an unqualified type.

This change seems reasonable overall.


Repository:
  rC Clang

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

https://reviews.llvm.org/D47419



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

[PATCH] D65665: Support for attributes on @class declarations

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Have you looked through the attributes that can be written on `@interface`s and 
verified that they're all sensible to write on a `@class`?  It's not hard to 
imagine that *some* of them should be diagnosed when added to a non-definition.


Repository:
  rC Clang

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

https://reviews.llvm.org/D65665



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

[PATCH] D65877: [libTooling] In Transformer, generalize `applyFirst` to admit rules with incompatible matchers.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

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

I was going to add a test for `Type`/`QualType` and realized that they don't 
carry any source location info.  Therefore, I don't think they belong as 
top-level matchers for rewrite rules.  Instead, users should use 
`typeLoc(loc()`.  Therefore, the logic of `getKind` can be 
eliminated in favor of using `K.getSupportedKind()` directly.  However, that 
means we'll need to keep them out of the collection of matchers that we're 
processing.

For that, I propose either we ban them in `buildMatchers()` or we detect them 
and wrap them in `typeLoc(loc()`. I'm fine with either one, 
but prefer the latter. WDYT?




Comment at: clang/lib/Tooling/Refactoring/Transformer.cpp:127
+  // in `taggedMatchers`.
+  std::map, 1>>
+  Buckets;

gribozavr wrote:
> `unordered_map`?
fails because `ASTNodeKind` doesn't have a hash function.  Which is odd since 
there's an obvious one we could define, but doesn't seem worth doing just for 
here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65877



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

[PATCH] D65987: [clang-doc] Generate HTML links for children namespaces/records

[Julie Hockett via Phabricator via cfe-commits]

juliehockett accepted this revision.
juliehockett marked an inline comment as done.
juliehockett added a comment.
This revision is now accepted and ready to land.

LGTM




Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:250
+  // The resulting path is "../../../A/B/D" instead of a "../D". It is correct
+  // but it would be better to have the shorter version.
   StringRef Dir = Directory;

DiegoAstiazaran wrote:
> DiegoAstiazaran wrote:
> > juliehockett wrote:
> > > Would `llvm::sys::path::remove_dots()` do this? It might not, but is 
> > > worth investigating.
> > `llvm::sys::path::remove_dots()` removes all `../` (except for leading 
> > `../`) so the resulting path in the example would be `../A/B/D`, which is 
> > not correct.
> Sorry about that, I was incorrect. `llvm::sys::path::remove_dots()` do what 
> we want but from the right side of the path, not the left side.
> For `A/B/C/../..` it changes it to `A`. So it still does not work for us.
That's irritating :(


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

https://reviews.llvm.org/D65987



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

[PATCH] D25845: [CUDA] Ignore implicit target attributes during function template instantiation.

[Artem Belevich via Phabricator via cfe-commits]

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



Comment at: cfe/trunk/lib/Sema/SemaDecl.cpp:8416
+// in the CheckFunctionTemplateSpecialization() call below.
+if (getLangOpts().CUDA & !isFunctionTemplateSpecialization)
+  maybeAddCUDAHostDeviceAttrs(NewFD, Previous);

RKSimon wrote:
> @tra Sorry for touching such an old ticket - but cppcheck is warning that we 
> are using a boolean result in a bitwise expression - I'm assuming this should 
> be:
> ```
> if (getLangOpts().CUDA && !isFunctionTemplateSpecialization)
> ```
Good catch. I'll fix it.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D25845



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

[PATCH] D66035: [WebAssembly] WIP: Add support for reference types

[Jacob Gravelle via Phabricator via cfe-commits]

jgravelle-google added a comment.

Overall direction looks good to me as well.

Ditto that tests would be useful just to make it clearer how this is 
represented in IR.




Comment at: llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.h:47
+  MVT getPointerTy(const DataLayout &DL, uint32_t AS = 0) const override {
+return AS == 1 ? MVT::anyref :
+  MVT::getIntegerVT(DL.getPointerSizeInBits(AS));

We'll almost-surely want to pull this `1` out into a constant, 
`WasmAnyrefAddressSpace` or something


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66035



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

[PATCH] D66100: Add __has_builtin support for builtin function-like type traits.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith created this revision.
rsmith added a reviewer: aaron.ballman.
Herald added subscribers: cfe-commits, kristina, jfb.
Herald added a project: clang.

Previously __has_builtin(__builtin_*) would return false for
__builtin_*s that we modeled as keywords rather than as functions
(because they take type arguments). With this patch, all builtins
that are called with function-call-like syntax return true from
__has_builtin (covering __builtin_* and also the __is_* and __has_* type
traits and the handful of similar builtins without such a prefix).

Update the documentation on __has_builtin and on type traits to match.
While doing this I noticed the type trait documentation was out of date
and incomplete; that's fixed here too.


Repository:
  rC Clang

https://reviews.llvm.org/D66100

Files:
  docs/LanguageExtensions.rst
  include/clang/Basic/Features.def
  include/clang/Basic/TokenKinds.def
  lib/Lex/PPMacroExpansion.cpp
  test/Preprocessor/feature_tests.c
  test/Preprocessor/feature_tests.cpp

Index: test/Preprocessor/feature_tests.cpp
===
--- /dev/null
+++ test/Preprocessor/feature_tests.cpp
@@ -0,0 +1,43 @@
+// RUN: %clang_cc1 %s -triple=i686-apple-darwin9 -verify -DVERIFY
+// expected-no-diagnostics
+
+#ifndef __has_feature
+#error Should have __has_feature
+#endif
+
+#if __has_feature(something_we_dont_have)
+#error Bad
+#endif
+
+#if  !__has_builtin(__builtin_huge_val) || \
+ !__has_builtin(__builtin_shufflevector) || \
+ !__has_builtin(__builtin_convertvector) || \
+ !__has_builtin(__builtin_trap) || \
+ !__has_builtin(__c11_atomic_init) || \
+ !__has_builtin(__builtin_launder) || \
+ !__has_feature(attribute_analyzer_noreturn) || \
+ !__has_feature(attribute_overloadable)
+#error Clang should have these
+#endif
+
+// These are technically implemented as keywords, but __has_builtin should
+// still return true.
+#if !__has_builtin(__builtin_LINE) || \
+!__has_builtin(__builtin_FILE) || \
+!__has_builtin(__builtin_FUNCTION) || \
+!__has_builtin(__builtin_COLUMN) || \
+!__has_builtin(__array_rank) || \
+!__has_builtin(__underlying_type) || \
+!__has_builtin(__is_trivial) || \
+!__has_builtin(__has_unique_object_representations)
+#error Clang should have these
+#endif
+
+// This is a C-only builtin.
+#if __has_builtin(__builtin_types_compatible_p)
+#error Clang should not have this in C++ mode
+#endif
+
+#if __has_builtin(__builtin_insanity)
+#error Clang should not have this
+#endif
Index: test/Preprocessor/feature_tests.c
===
--- test/Preprocessor/feature_tests.c
+++ test/Preprocessor/feature_tests.c
@@ -25,10 +25,18 @@
 #if !__has_builtin(__builtin_LINE) || \
 !__has_builtin(__builtin_FILE) || \
 !__has_builtin(__builtin_FUNCTION) || \
-!__has_builtin(__builtin_COLUMN)
+!__has_builtin(__builtin_COLUMN) || \
+!__has_builtin(__builtin_types_compatible_p)
 #error Clang should have these
 #endif
 
+// These are C++-only builtins.
+#if __has_builtin(__array_rank) || \
+__has_builtin(__underlying_type) || \
+__has_builtin(__is_trivial)
+#error Clang should not have these in C mode
+#endif
+
 #if __has_builtin(__builtin_insanity)
 #error Clang should not have this
 #endif
Index: lib/Lex/PPMacroExpansion.cpp
===
--- lib/Lex/PPMacroExpansion.cpp
+++ lib/Lex/PPMacroExpansion.cpp
@@ -1617,21 +1617,38 @@
 return true;
   }
   return true;
+} else if (II->getTokenID() != tok::identifier ||
+   II->hasRevertedTokenIDToIdentifier()) {
+  // Treat all keywords that introduce a custom syntax of the form
+  //
+  //   '__some_keyword' '(' [...] ')'
+  //
+  // as being "builtin functions", even if the syntax isn't a valid
+  // function call (for example, because the builtin takes a type
+  // argument).
+  if (II->getName().startswith("__builtin_") ||
+  II->getName().startswith("__is_") ||
+  II->getName().startswith("__has_"))
+return true;
+  return llvm::StringSwitch(II->getName())
+  .Case("__array_rank", true)
+  .Case("__array_extent", true)
+  .Case("__reference_binds_to_temporary", true)
+  .Case("__underlying_type", true)
+  .Default(false);
 } else {
   return llvm::StringSwitch(II->getName())
-  .Case("__make_integer_seq", LangOpts.CPlusPlus)
-  .Case("__type_pack_element", LangOpts.CPlusPlus)
-  .Case("__builtin_available", true)
-  .Case("__is_target_arch", true)
-  .Case("__is_target_vendor", true)
-  .Case("__is_target_os", true)
-  .Case("__is_target_environme

[PATCH] D65877: [libTooling] In Transformer, generalize `applyFirst` to admit rules with incompatible matchers.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added a comment.

In D65877#1625493 , @ymandel wrote:

> I was going to add a test for `Type`/`QualType` and realized that they don't 
> carry any source location info.  Therefore, I don't think they belong as 
> top-level matchers for rewrite rules.


Agreed.

> Instead, users should use `typeLoc(loc()`.  Therefore, the 
> logic of `getKind` can be eliminated in favor of using `K.getSupportedKind()` 
> directly.  However, that means we'll need to keep them out of the collection 
> of matchers that we're processing.
> 
> For that, I propose either we ban them in `buildMatchers()` or we detect them 
> and wrap them in `typeLoc(loc()`. I'm fine with either 
> one, but prefer the latter. WDYT?

I'd prefer we ban them completely, and let the user wrap them manually if they 
need to. While some users would appreciate the ergonomics, I think AST matchers 
are complicated even without us adding more implicit behavior on top.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65877



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

r368600 - [Sema] Require a complete type for __builtin_bit_cast operands

[Erik Pilkington via cfe-commits]

Author: epilk
Date: Mon Aug 12 11:31:27 2019
New Revision: 368600

URL: http://llvm.org/viewvc/llvm-project?rev=368600&view=rev
Log:
[Sema] Require a complete type for __builtin_bit_cast operands

Fixes llvm.org/PR42936

Modified:
cfe/trunk/lib/Sema/SemaCast.cpp
cfe/trunk/test/SemaCXX/builtin-bit-cast.cpp

Modified: cfe/trunk/lib/Sema/SemaCast.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCast.cpp?rev=368600&r1=368599&r2=368600&view=diff
==
--- cfe/trunk/lib/Sema/SemaCast.cpp (original)
+++ cfe/trunk/lib/Sema/SemaCast.cpp Mon Aug 12 11:31:27 2019
@@ -2803,6 +2803,14 @@ void CastOperation::CheckBuiltinBitCast(
 SrcExpr = Self.CreateMaterializeTemporaryExpr(SrcType, SrcExpr.get(),
   /*IsLValueReference=*/false);
 
+  if (Self.RequireCompleteType(OpRange.getBegin(), DestType,
+   diag::err_typecheck_cast_to_incomplete) ||
+  Self.RequireCompleteType(OpRange.getBegin(), SrcType,
+   diag::err_incomplete_type)) {
+SrcExpr = ExprError();
+return;
+  }
+
   CharUnits DestSize = Self.Context.getTypeSizeInChars(DestType);
   CharUnits SourceSize = Self.Context.getTypeSizeInChars(SrcType);
   if (DestSize != SourceSize) {

Modified: cfe/trunk/test/SemaCXX/builtin-bit-cast.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/builtin-bit-cast.cpp?rev=368600&r1=368599&r2=368600&view=diff
==
--- cfe/trunk/test/SemaCXX/builtin-bit-cast.cpp (original)
+++ cfe/trunk/test/SemaCXX/builtin-bit-cast.cpp Mon Aug 12 11:31:27 2019
@@ -37,3 +37,12 @@ constexpr unsigned long ul = __builtin_b
 
 // expected-error@+1 {{__builtin_bit_cast destination type must be trivially 
copyable}}
 constexpr long us = __builtin_bit_cast(unsigned long &, 0L);
+
+namespace PR42936 {
+template  struct S { int m; };
+
+extern S extern_decl;
+
+int x = __builtin_bit_cast(int, extern_decl);
+S y = __builtin_bit_cast(S, 0);
+}


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

Re: r368600 - [Sema] Require a complete type for __builtin_bit_cast operands

[Richard Smith via cfe-commits]

On Mon, 12 Aug 2019 at 11:30, Erik Pilkington via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: epilk
> Date: Mon Aug 12 11:31:27 2019
> New Revision: 368600
>
> URL: http://llvm.org/viewvc/llvm-project?rev=368600&view=rev
> Log:
> [Sema] Require a complete type for __builtin_bit_cast operands
>
> Fixes llvm.org/PR42936
>
> Modified:
> cfe/trunk/lib/Sema/SemaCast.cpp
> cfe/trunk/test/SemaCXX/builtin-bit-cast.cpp
>
> Modified: cfe/trunk/lib/Sema/SemaCast.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCast.cpp?rev=368600&r1=368599&r2=368600&view=diff
>
> ==
> --- cfe/trunk/lib/Sema/SemaCast.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaCast.cpp Mon Aug 12 11:31:27 2019
> @@ -2803,6 +2803,14 @@ void CastOperation::CheckBuiltinBitCast(
>  SrcExpr = Self.CreateMaterializeTemporaryExpr(SrcType, SrcExpr.get(),
>
>  /*IsLValueReference=*/false);
>
> +  if (Self.RequireCompleteType(OpRange.getBegin(), DestType,
> +   diag::err_typecheck_cast_to_incomplete) ||
> +  Self.RequireCompleteType(OpRange.getBegin(), SrcType,
> +   diag::err_incomplete_type)) {
>

Nit: we should check the source type for completeness before performing
temporary materialization conversion, as materializing a temporary only
makes sense for a complete type.


> +SrcExpr = ExprError();
> +return;
> +  }
> +
>CharUnits DestSize = Self.Context.getTypeSizeInChars(DestType);
>CharUnits SourceSize = Self.Context.getTypeSizeInChars(SrcType);
>if (DestSize != SourceSize) {
>
> Modified: cfe/trunk/test/SemaCXX/builtin-bit-cast.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/builtin-bit-cast.cpp?rev=368600&r1=368599&r2=368600&view=diff
>
> ==
> --- cfe/trunk/test/SemaCXX/builtin-bit-cast.cpp (original)
> +++ cfe/trunk/test/SemaCXX/builtin-bit-cast.cpp Mon Aug 12 11:31:27 2019
> @@ -37,3 +37,12 @@ constexpr unsigned long ul = __builtin_b
>
>  // expected-error@+1 {{__builtin_bit_cast destination type must be
> trivially copyable}}
>  constexpr long us = __builtin_bit_cast(unsigned long &, 0L);
> +
> +namespace PR42936 {
> +template  struct S { int m; };
> +
> +extern S extern_decl;
> +
> +int x = __builtin_bit_cast(int, extern_decl);
> +S y = __builtin_bit_cast(S, 0);
> +}
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r368602 - [clang-doc] Generate HTML links for children namespaces/records

[Diego Astiazaran via cfe-commits]

Author: diegoastiazaran
Date: Mon Aug 12 11:42:46 2019
New Revision: 368602

URL: http://llvm.org/viewvc/llvm-project?rev=368602&view=rev
Log:
[clang-doc] Generate HTML links for children namespaces/records

Path is now stored in the references to the child while serializing,
then this path is used to generate the relative path in the HTML
generator.
Now some references have paths and some don't so in the reducing phase,
references are now properly merged checking for empty attributes.
Tests added for HTML and YAML generators, merging and serializing.
computeRelativePath function had a bug when the filepath is part of the
given directory; it returned a path that starts with a separator. This
has been fixed.

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

Modified:
clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp
clang-tools-extra/trunk/clang-doc/Representation.cpp
clang-tools-extra/trunk/clang-doc/Representation.h
clang-tools-extra/trunk/clang-doc/Serialize.cpp
clang-tools-extra/trunk/unittests/clang-doc/HTMLGeneratorTest.cpp
clang-tools-extra/trunk/unittests/clang-doc/MergeTest.cpp
clang-tools-extra/trunk/unittests/clang-doc/SerializeTest.cpp
clang-tools-extra/trunk/unittests/clang-doc/YAMLGeneratorTest.cpp

Modified: clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp?rev=368602&r1=368601&r2=368602&view=diff
==
--- clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp (original)
+++ clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp Mon Aug 12 11:42:46 2019
@@ -207,26 +207,44 @@ static void AppendVector(std::vector computeRelativePath(StringRef FilePath,
-StringRef Directory) {
-  StringRef Path = FilePath;
-  while (!Path.empty()) {
-if (Directory == Path)
-  return FilePath.substr(Path.size());
-Path = llvm::sys::path::parent_path(Path);
-  }
-
-  StringRef Dir = Directory;
-  SmallString<128> Result;
-  while (!Dir.empty()) {
-if (Dir == FilePath)
-  break;
-Dir = llvm::sys::path::parent_path(Dir);
+// Compute the relative path from an Origin directory to a Destination 
directory
+static SmallString<128> computeRelativePath(StringRef Destination,
+StringRef Origin) {
+  // If Origin is empty, the relative path to the Destination is its complete
+  // path.
+  if (Origin.empty())
+return Destination;
+
+  // The relative path is an empty path if both directories are the same.
+  if (Destination == Origin)
+return {};
+
+  // These iterators iterate through each of their parent directories
+  llvm::sys::path::const_iterator FileI = llvm::sys::path::begin(Destination);
+  llvm::sys::path::const_iterator FileE = llvm::sys::path::end(Destination);
+  llvm::sys::path::const_iterator DirI = llvm::sys::path::begin(Origin);
+  llvm::sys::path::const_iterator DirE = llvm::sys::path::end(Origin);
+  // Advance both iterators until the paths differ. Example:
+  //Destination = A/B/C/D
+  //Origin  = A/B/E/F
+  // FileI will point to C and DirI to E. The directories behind them is the
+  // directory they share (A/B).
+  while (FileI != FileE && DirI != DirE && *FileI == *DirI) {
+++FileI;
+++DirI;
+  }
+  SmallString<128> Result; // This will hold the resulting path.
+  // Result has to go up one directory for each of the remaining directories in
+  // Origin
+  while (DirI != DirE) {
 llvm::sys::path::append(Result, "..");
+++DirI;
+  }
+  // Result has to append each of the remaining directories in Destination
+  while (FileI != FileE) {
+llvm::sys::path::append(Result, *FileI);
+++FileI;
   }
-  llvm::sys::path::append(Result, FilePath.substr(Dir.size()));
   return Result;
 }
 
@@ -271,8 +289,8 @@ static std::unique_ptr genLink(
 }
 
 static std::unique_ptr
-genTypeReference(const Reference &Type, StringRef CurrentDirectory,
- llvm::Optional JumpToSection = None) {
+genReference(const Reference &Type, StringRef CurrentDirectory,
+ llvm::Optional JumpToSection = None) {
   if (Type.Path.empty() && !Type.IsInGlobalNamespace) {
 if (!JumpToSection)
   return llvm::make_unique(Type.Name);
@@ -296,7 +314,7 @@ genReferenceList(const llvm::SmallVector
   for (const auto &R : Refs) {
 if (&R != Refs.begin())
   Out.emplace_back(llvm::make_unique(", "));
-Out.emplace_back(genTypeReference(R, CurrentDirectory));
+Out.emplace_back(genReference(R, CurrentDirectory));
   }
   return Out;
 }
@@ -372,7 +390,7 @@ genRecordMembersBlock(const llvm::SmallV
   Access = Access + " ";
 auto LIBody = llvm::make_unique(HTMLTag::TAG_LI);
 LIBody->Children.emplace_back(llvm::make_unique(Access));
-LIBody->Children.emplace_back(genTypeReference(M.Type, ParentInfoDir));
+LIBody->Children.emplace_back(gen

[PATCH] D65987: [clang-doc] Generate HTML links for children namespaces/records

[Diego Astiazarán via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL368602: [clang-doc] Generate HTML links for children 
namespaces/records (authored by DiegoAstiazaran, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65987?vs=21&id=214677#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65987

Files:
  clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp
  clang-tools-extra/trunk/clang-doc/Representation.cpp
  clang-tools-extra/trunk/clang-doc/Representation.h
  clang-tools-extra/trunk/clang-doc/Serialize.cpp
  clang-tools-extra/trunk/unittests/clang-doc/HTMLGeneratorTest.cpp
  clang-tools-extra/trunk/unittests/clang-doc/MergeTest.cpp
  clang-tools-extra/trunk/unittests/clang-doc/SerializeTest.cpp
  clang-tools-extra/trunk/unittests/clang-doc/YAMLGeneratorTest.cpp

Index: clang-tools-extra/trunk/unittests/clang-doc/HTMLGeneratorTest.cpp
===
--- clang-tools-extra/trunk/unittests/clang-doc/HTMLGeneratorTest.cpp
+++ clang-tools-extra/trunk/unittests/clang-doc/HTMLGeneratorTest.cpp
@@ -39,8 +39,9 @@
   I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
 
   I.ChildNamespaces.emplace_back(EmptySID, "ChildNamespace",
- InfoType::IT_namespace);
-  I.ChildRecords.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record);
+ InfoType::IT_namespace, "Namespace");
+  I.ChildRecords.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record,
+  "Namespace");
   I.ChildFunctions.emplace_back();
   I.ChildFunctions.back().Name = "OneFunction";
   I.ChildEnums.emplace_back();
@@ -100,11 +101,15 @@
   namespace Namespace
   Namespaces
   
-ChildNamespace
+
+  ChildNamespace
+
   
   Records
   
-ChildStruct
+
+  ChildStruct
+
   
   Functions
   
@@ -137,7 +142,8 @@
   I.Parents.emplace_back(EmptySID, "F", InfoType::IT_record, PathTo);
   I.VirtualParents.emplace_back(EmptySID, "G", InfoType::IT_record);
 
-  I.ChildRecords.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record);
+  I.ChildRecords.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record,
+  "X/Y/Z/r");
   I.ChildFunctions.emplace_back();
   I.ChildFunctions.back().Name = "OneFunction";
   I.ChildEnums.emplace_back();
@@ -215,7 +221,9 @@
   
   Records
   
-ChildStruct
+
+  ChildStruct
+
   
   Functions
   
Index: clang-tools-extra/trunk/unittests/clang-doc/YAMLGeneratorTest.cpp
===
--- clang-tools-extra/trunk/unittests/clang-doc/YAMLGeneratorTest.cpp
+++ clang-tools-extra/trunk/unittests/clang-doc/YAMLGeneratorTest.cpp
@@ -29,8 +29,9 @@
   I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
 
   I.ChildNamespaces.emplace_back(EmptySID, "ChildNamespace",
- InfoType::IT_namespace);
-  I.ChildRecords.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record);
+ InfoType::IT_namespace, "path/to/A/Namespace");
+  I.ChildRecords.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record,
+  "path/to/A/Namespace");
   I.ChildFunctions.emplace_back();
   I.ChildFunctions.back().Name = "OneFunction";
   I.ChildEnums.emplace_back();
@@ -53,9 +54,11 @@
 ChildNamespaces:
   - Type:Namespace
 Name:'ChildNamespace'
+Path:'path/to/A/Namespace'
 ChildRecords:
   - Type:Record
 Name:'ChildStruct'
+Path:'path/to/A/Namespace'
 ChildFunctions:
   - USR: ''
 Name:'OneFunction'
@@ -71,7 +74,7 @@
 TEST(YAMLGeneratorTest, emitRecordYAML) {
   RecordInfo I;
   I.Name = "r";
-  I.Path = "path/to/r";
+  I.Path = "path/to/A";
   I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
 
   I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
@@ -85,7 +88,8 @@
   I.VirtualParents.emplace_back(EmptySID, "G", InfoType::IT_record,
 "path/to/G");
 
-  I.ChildRecords.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record);
+  I.ChildRecords.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record,
+  "path/to/A/r");
   I.ChildFunctions.emplace_back();
   I.ChildFunctions.back().Name = "OneFunction";
   I.ChildEnums.emplace_back();
@@ -101,7 +105,7 @@
   R"raw(---
 USR: ''
 Name:'r'
-Path:'path/to/r'
+Path:'path/to/A'
 Namespace:
   - Type:Namespace
 Name:'A'
@@ -129,6 +133,7 @@
 ChildRecords:
   - Type:Record
 Name: 

[PATCH] D65877: [libTooling] In Transformer, generalize `applyFirst` to admit rules with incompatible matchers.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

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

In D65877#1625616 , @gribozavr wrote:

> I'd prefer we ban them completely, and let the user wrap them manually if 
> they need to. While some users would appreciate the ergonomics, I think AST 
> matchers are complicated even without us adding more implicit behavior on top.


Done. I came to the same conclusion once I started implementing. Less (magic) 
is more, in this case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65877



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

[PATCH] D65877: [libTooling] In Transformer, generalize `applyFirst` to admit rules with incompatible matchers.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 214679.
ymandel added a comment.

Bans qualtype and type and adds corresponding comments and test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65877

Files:
  clang/include/clang/Tooling/Refactoring/Transformer.h
  clang/lib/Tooling/Refactoring/Transformer.cpp
  clang/unittests/Tooling/TransformerTest.cpp

Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- clang/unittests/Tooling/TransformerTest.cpp
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -482,65 +482,85 @@
   testRule(applyFirst({ruleStrlenSize(), FlagRule}), Input, Expected);
 }
 
-// Version of ruleStrlenSizeAny that inserts a method with a different name than
-// ruleStrlenSize, so we can tell their effect apart.
-RewriteRule ruleStrlenSizeDistinct() {
-  StringRef S;
-  return makeRule(
-  callExpr(callee(functionDecl(hasName("strlen"))),
-   hasArgument(0, cxxMemberCallExpr(
-  on(expr().bind(S)),
-  callee(cxxMethodDecl(hasName("c_str")),
-  change(text("DISTINCT")));
-}
-
 TEST_F(TransformerTest, OrderedRuleRelated) {
   std::string Input = R"cc(
-namespace foo {
-struct mystring {
-  char* c_str();
-};
-int f(mystring s) { return strlen(s.c_str()); }
-}  // namespace foo
-int g(string s) { return strlen(s.c_str()); }
+void f1();
+void f2();
+void call_f1() { f1(); }
+void call_f2() { f2(); }
   )cc";
   std::string Expected = R"cc(
-namespace foo {
-struct mystring {
-  char* c_str();
-};
-int f(mystring s) { return DISTINCT; }
-}  // namespace foo
-int g(string s) { return REPLACED; }
+void f1();
+void f2();
+void call_f1() { REPLACE_F1; }
+void call_f2() { REPLACE_F1_OR_F2; }
   )cc";
 
-  testRule(applyFirst({ruleStrlenSize(), ruleStrlenSizeDistinct()}), Input,
-   Expected);
+  RewriteRule ReplaceF1 =
+  makeRule(callExpr(callee(functionDecl(hasName("f1",
+   change(text("REPLACE_F1")));
+  RewriteRule ReplaceF1OrF2 =
+  makeRule(callExpr(callee(functionDecl(hasAnyName("f1", "f2",
+   change(text("REPLACE_F1_OR_F2")));
+  testRule(applyFirst({ReplaceF1, ReplaceF1OrF2}), Input, Expected);
 }
 
-// Change the order of the rules to get a different result.
+// Change the order of the rules to get a different result. When `ReplaceF1OrF2`
+// comes first, it applies for both uses, so `ReplaceF1` never applies.
 TEST_F(TransformerTest, OrderedRuleRelatedSwapped) {
   std::string Input = R"cc(
-namespace foo {
-struct mystring {
-  char* c_str();
-};
-int f(mystring s) { return strlen(s.c_str()); }
-}  // namespace foo
-int g(string s) { return strlen(s.c_str()); }
+void f1();
+void f2();
+void call_f1() { f1(); }
+void call_f2() { f2(); }
   )cc";
   std::string Expected = R"cc(
-namespace foo {
-struct mystring {
-  char* c_str();
-};
-int f(mystring s) { return DISTINCT; }
-}  // namespace foo
-int g(string s) { return DISTINCT; }
+void f1();
+void f2();
+void call_f1() { REPLACE_F1_OR_F2; }
+void call_f2() { REPLACE_F1_OR_F2; }
+  )cc";
+
+  RewriteRule ReplaceF1 =
+  makeRule(callExpr(callee(functionDecl(hasName("f1",
+   change(text("REPLACE_F1")));
+  RewriteRule ReplaceF1OrF2 =
+  makeRule(callExpr(callee(functionDecl(hasAnyName("f1", "f2",
+   change(text("REPLACE_F1_OR_F2")));
+  testRule(applyFirst({ReplaceF1OrF2, ReplaceF1}), Input, Expected);
+}
+
+// Verify that a set of rules whose matchers have different base kinds works
+// properly, including that `applyFirst` produces multiple matchers.  We test
+// two different kinds of rules: Expr and Decl. We place the Decl rule in the
+// middle to test that `buildMatchers` works even when the kinds aren't grouped
+// together.
+TEST_F(TransformerTest, OrderedRuleMultipleKinds) {
+  std::string Input = R"cc(
+void f1();
+void f2();
+void call_f1() { f1(); }
+void call_f2() { f2(); }
   )cc";
-
-  testRule(applyFirst({ruleStrlenSizeDistinct(), ruleStrlenSize()}), Input,
-   Expected);
+  std::string Expected = R"cc(
+void f1();
+void DECL_RULE();
+void call_f1() { REPLACE_F1; }
+void call_f2() { REPLACE_F1_OR_F2; }
+  )cc";
+
+  RewriteRule ReplaceF1 =
+  makeRule(callExpr(callee(functionDecl(hasName("f1",
+   change(text("REPLACE_F1")));
+  RewriteRule ReplaceF1OrF2 =
+  makeRule(callExpr(callee(functionDecl(hasAnyName("f1", "f2",
+   change(text("REPLACE_F1_OR_F2")));
+  RewriteRule DeclRule = makeRule(functionDecl(hasName("f2")).bind("fun"),
+  change(name("fun"), text("DECL_RULE")));
+
+  RewriteRule Rule = applyFirst({ReplaceF1, 

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc

[Jian Cai via Phabricator via cfe-commits]

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



Comment at: llvm/test/CodeGen/ARM/gnu_mcount_nc.ll:1-6
+; RUN: llc -mtriple=armv7a-linux-gnueabihf %s -o - | FileCheck %s 
--check-prefix=CHECK-ARM
+; RUN: llc -mtriple=armv7a-linux-gnueabihf %s -o - | FileCheck %s 
--check-prefix=CHECK-ARM-FAST-ISEL
+; RUN: llc -mtriple=armv7a-linux-gnueabihf %s -o - | FileCheck %s 
--check-prefix=CHECK-ARM-GLOBAL-ISEL
+; RUN: llc -mtriple=thumbv7a-linux-gnueabihf %s -o - | FileCheck %s 
--check-prefix=CHECK-THUMB
+; RUN: llc -mtriple=thumbv7a-linux-gnueabihf %s -o - | FileCheck %s 
--check-prefix=CHECK-THUMB-FAST-ISEL
+; RUN: llc -mtriple=thumbv7a-linux-gnueabihf %s -o - | FileCheck %s 
--check-prefix=CHECK-THUMB-GLOBAL-ISEL

kristof.beyls wrote:
> It seems the -fast-isel/-global-isel command line options are missing in the 
> RUN lines aiming to test fast and global isel do the right thing?
Sorry must have forgotten to add the instruction selection options while 
copying the RUN commands. Just tested locally and -fast-isel option worked, 
although -global-isel failed due to "LLVM ERROR: unable to map instruction: 
G_INTRINSIC_W_SIDE_EFFECTS intrinsic(@llvm.arm.gnu.eabi.mcount)". It seems 
global-isel does not fall back to  DAGISel? Will have to investigate further.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65019



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

[PATCH] D65999: [ASTImporter] Import additional flags for functions.

[Aleksei Sidorin via Phabricator via cfe-commits]

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

Hi Balazs,
The change looks good. I think it can be committed after an unrelated part is 
removed.




Comment at: clang/unittests/AST/ASTImporterTest.cpp:5373
 
+INSTANTIATE_TEST_CASE_P(ParameterizedTests, SVEBuiltins,
+::testing::Values(ArgVector{"-target",

This is a separate functional change and I prefer to move it into a separate 
patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65999



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

[PATCH] D66014: [analyzer] Avoid unnecessary enum range check on LValueToRValue casts

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: 
clang/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp:94-98
+  // If cast is implicit LValueToRValue, no conversion is taking place,
+  // and therefore no range check is needed.  Don't analyze further.
+  if (CE->getCastKind() == CK_LValueToRValue)
+return;
+

chrish_ericsson_atx wrote:
> NoQ wrote:
> > If you look at the list of cast kinds, you'll be shocked to see 
> > ridiculously weird cornercases. Even though lvalue-to-rvalue cast 
> > definitely stands out (because it's the only one that has very little to do 
> > with actual casting), i'd still be much more comfortable if we *whitelist* 
> > the casts that we check, rather than blacklist them.
> > 
> > Can you take a look at the list and find which casts are we actually 
> > talking about and hardcode them here?
> I'm happy to cross-check a list; however, I'm not aware of the list you are 
> referring to.  Would you mind providing a bit more detail?
See **[[ 
https://github.com/llvm-mirror/clang/blob/release_90/include/clang/AST/OperationKinds.def
 | include/clang/AST/OperationKinds.def ]]**.



Comment at: 
clang/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp:121-122
   // Check if any of the enum values possibly match.
   bool PossibleValueMatch = llvm::any_of(
   DeclValues, ConstraintBasedEQEvaluator(C, *ValueToCast));
 

chrish_ericsson_atx wrote:
> Szelethus wrote:
> > So this is where the assertion comes from, and will eventually lead to 
> > `SValBuilder::evalEQ`, which calls `SValBuilder::evalBinOp`, where this 
> > will fire on line 427:
> > ```
> > assert(op == BO_Add);
> > ```
> > Seems like this happens because `unused`'s value in your testcase will be 
> > retrieved as a `Loc`, while the values in the enum are (correctly) 
> > `NonLoc`, and `SValBuilder::evalBinOp` thinks this is some sort of pointer 
> > arithmetic (`5 + ptr` etc).
> > 
> > How about instead of checking for LValueToRValue cast, we check whether 
> > `ValueToCast` is `Loc`, and bail out if so? That sounds like a more general 
> > solution, but I didn't sit atop of this for hours.
> Is this the only case where ValueToCast will be Loc?   I was unsure about the 
> implications of Loc vs. NonLoc in this code, and I didn't want to risk 
> breaking a check that should have been valid.  That's why I opted for bailing 
> on an LValueToRValue cast at a higher level-- that seemed much safer to me.  
> I certainly might be missing something, but I couldn't find any evidence that 
> an LValueToRValue cast should ever need to be checked for enum range errors.  
>  I will certainly defer to your judgement, but I'd like to have a better 
> understanding of why looking for ValueToCast == Loc would actually be more 
> correct.
> How about instead of checking for `LValueToRValue` cast, we check whether 
> `ValueToCast` is `Loc`, and bail out if so?

In this case it probably doesn't matter too much, but generally i always 
recommend against this sort of stuff: if possible, consult the AST. A `Loc` may 
represent either a glvalue or a pointer-typed prvalue, and there's no way to 
tell the two apart by only looking at that `Loc`. In particular, both unary 
operators `*` and `&` are modeled as //no-op//. I've been a lot of incorrect 
code that tried to dereference a `Loc` too many or too few times because the 
code had misjudged whether it has already obtained the prvalue it was looking 
for. But it's easy to discriminate between the two when you look at the AST.

The Static Analyzer is a converter from ASTs to ExplodedGraphs. When in doubt, 
consult the AST.


Repository:
  rC Clang

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

https://reviews.llvm.org/D66014



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

r368610 - [Sema] Check __builtin_bit_cast operand for completeness before materializing it.

[Erik Pilkington via cfe-commits]

Author: epilk
Date: Mon Aug 12 12:29:43 2019
New Revision: 368610

URL: http://llvm.org/viewvc/llvm-project?rev=368610&view=rev
Log:
[Sema] Check __builtin_bit_cast operand for completeness before materializing 
it.

This shouldn't be observable, but it doesn't make sense to materialize an
incomplete type.

Modified:
cfe/trunk/lib/Sema/SemaCast.cpp

Modified: cfe/trunk/lib/Sema/SemaCast.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCast.cpp?rev=368610&r1=368609&r2=368610&view=diff
==
--- cfe/trunk/lib/Sema/SemaCast.cpp (original)
+++ cfe/trunk/lib/Sema/SemaCast.cpp Mon Aug 12 12:29:43 2019
@@ -2799,9 +2799,6 @@ void CastOperation::CheckCStyleCast() {
 
 void CastOperation::CheckBuiltinBitCast() {
   QualType SrcType = SrcExpr.get()->getType();
-  if (SrcExpr.get()->isRValue())
-SrcExpr = Self.CreateMaterializeTemporaryExpr(SrcType, SrcExpr.get(),
-  /*IsLValueReference=*/false);
 
   if (Self.RequireCompleteType(OpRange.getBegin(), DestType,
diag::err_typecheck_cast_to_incomplete) ||
@@ -2811,6 +2808,10 @@ void CastOperation::CheckBuiltinBitCast(
 return;
   }
 
+  if (SrcExpr.get()->isRValue())
+SrcExpr = Self.CreateMaterializeTemporaryExpr(SrcType, SrcExpr.get(),
+  /*IsLValueReference=*/false);
+
   CharUnits DestSize = Self.Context.getTypeSizeInChars(DestType);
   CharUnits SourceSize = Self.Context.getTypeSizeInChars(SrcType);
   if (DestSize != SourceSize) {


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

Re: r368600 - [Sema] Require a complete type for __builtin_bit_cast operands

[Erik Pilkington via cfe-commits]

Sure, I fixed this in r368610.

Erik

> On Aug 12, 2019, at 11:39 AM, Richard Smith  wrote:
> 
> On Mon, 12 Aug 2019 at 11:30, Erik Pilkington via cfe-commits 
> mailto:cfe-commits@lists.llvm.org>> wrote:
> Author: epilk
> Date: Mon Aug 12 11:31:27 2019
> New Revision: 368600
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=368600&view=rev 
> 
> Log:
> [Sema] Require a complete type for __builtin_bit_cast operands
> 
> Fixes llvm.org/PR42936 
> 
> Modified:
> cfe/trunk/lib/Sema/SemaCast.cpp
> cfe/trunk/test/SemaCXX/builtin-bit-cast.cpp
> 
> Modified: cfe/trunk/lib/Sema/SemaCast.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCast.cpp?rev=368600&r1=368599&r2=368600&view=diff
>  
> 
> ==
> --- cfe/trunk/lib/Sema/SemaCast.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaCast.cpp Mon Aug 12 11:31:27 2019
> @@ -2803,6 +2803,14 @@ void CastOperation::CheckBuiltinBitCast(
>  SrcExpr = Self.CreateMaterializeTemporaryExpr(SrcType, SrcExpr.get(),
>
> /*IsLValueReference=*/false);
> 
> +  if (Self.RequireCompleteType(OpRange.getBegin(), DestType,
> +   diag::err_typecheck_cast_to_incomplete) ||
> +  Self.RequireCompleteType(OpRange.getBegin(), SrcType,
> +   diag::err_incomplete_type)) {
> 
> Nit: we should check the source type for completeness before performing 
> temporary materialization conversion, as materializing a temporary only makes 
> sense for a complete type.
>  
> +SrcExpr = ExprError();
> +return;
> +  }
> +
>CharUnits DestSize = Self.Context.getTypeSizeInChars(DestType);
>CharUnits SourceSize = Self.Context.getTypeSizeInChars(SrcType);
>if (DestSize != SourceSize) {
> 
> Modified: cfe/trunk/test/SemaCXX/builtin-bit-cast.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/builtin-bit-cast.cpp?rev=368600&r1=368599&r2=368600&view=diff
>  
> 
> ==
> --- cfe/trunk/test/SemaCXX/builtin-bit-cast.cpp (original)
> +++ cfe/trunk/test/SemaCXX/builtin-bit-cast.cpp Mon Aug 12 11:31:27 2019
> @@ -37,3 +37,12 @@ constexpr unsigned long ul = __builtin_b
> 
>  // expected-error@+1 {{__builtin_bit_cast destination type must be trivially 
> copyable}}
>  constexpr long us = __builtin_bit_cast(unsigned long &, 0L);
> +
> +namespace PR42936 {
> +template  struct S { int m; };
> +
> +extern S extern_decl;
> +
> +int x = __builtin_bit_cast(int, extern_decl);
> +S y = __builtin_bit_cast(S, 0);
> +}
> 
> 
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org 
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits 
> 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66014: [analyzer] Avoid unnecessary enum range check on LValueToRValue casts

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

Oh, btw, thank you for working on this!




Comment at: 
clang/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp:121-122
   // Check if any of the enum values possibly match.
   bool PossibleValueMatch = llvm::any_of(
   DeclValues, ConstraintBasedEQEvaluator(C, *ValueToCast));
 

NoQ wrote:
> chrish_ericsson_atx wrote:
> > Szelethus wrote:
> > > So this is where the assertion comes from, and will eventually lead to 
> > > `SValBuilder::evalEQ`, which calls `SValBuilder::evalBinOp`, where this 
> > > will fire on line 427:
> > > ```
> > > assert(op == BO_Add);
> > > ```
> > > Seems like this happens because `unused`'s value in your testcase will be 
> > > retrieved as a `Loc`, while the values in the enum are (correctly) 
> > > `NonLoc`, and `SValBuilder::evalBinOp` thinks this is some sort of 
> > > pointer arithmetic (`5 + ptr` etc).
> > > 
> > > How about instead of checking for LValueToRValue cast, we check whether 
> > > `ValueToCast` is `Loc`, and bail out if so? That sounds like a more 
> > > general solution, but I didn't sit atop of this for hours.
> > Is this the only case where ValueToCast will be Loc?   I was unsure about 
> > the implications of Loc vs. NonLoc in this code, and I didn't want to risk 
> > breaking a check that should have been valid.  That's why I opted for 
> > bailing on an LValueToRValue cast at a higher level-- that seemed much 
> > safer to me.  I certainly might be missing something, but I couldn't find 
> > any evidence that an LValueToRValue cast should ever need to be checked for 
> > enum range errors.   I will certainly defer to your judgement, but I'd like 
> > to have a better understanding of why looking for ValueToCast == Loc would 
> > actually be more correct.
> > How about instead of checking for `LValueToRValue` cast, we check whether 
> > `ValueToCast` is `Loc`, and bail out if so?
> 
> In this case it probably doesn't matter too much, but generally i always 
> recommend against this sort of stuff: if possible, consult the AST. A `Loc` 
> may represent either a glvalue or a pointer-typed prvalue, and there's no way 
> to tell the two apart by only looking at that `Loc`. In particular, both 
> unary operators `*` and `&` are modeled as //no-op//. I've been a lot of 
> incorrect code that tried to dereference a `Loc` too many or too few times 
> because the code had misjudged whether it has already obtained the prvalue it 
> was looking for. But it's easy to discriminate between the two when you look 
> at the AST.
> 
> The Static Analyzer is a converter from ASTs to ExplodedGraphs. When in 
> doubt, consult the AST.
Notes taken, thanks! :)


Repository:
  rC Clang

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

https://reviews.llvm.org/D66014



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

[PATCH] D65877: [libTooling] In Transformer, generalize `applyFirst` to admit rules with incompatible matchers.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr accepted this revision.
gribozavr added inline comments.



Comment at: clang/lib/Tooling/Refactoring/Transformer.cpp:127
+if (!hasValidKind(Cases[I].Matcher))
+  return std::vector();
+Buckets[Cases[I].Matcher.getSupportedKind()].emplace_back(I, Cases[I]);

Returning an empty vector is a valid API choice, but why not assert instead? If 
an empty vector is returned, unsupported matchers are silently ignored instead 
of being diagnosed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65877



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