[PATCH] D75951: Keep a list of already-included pragma-once files for mods.

2020-03-26 Thread Vy Nguyen via Phabricator via cfe-commits
oontvoo updated this revision to Diff 252761.
oontvoo added a comment.

.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75951

Files:
  clang/include/clang/Lex/HeaderSearch.h
  clang/include/clang/Lex/Preprocessor.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/include/clang/Serialization/ASTReader.h
  clang/include/clang/Serialization/ASTWriter.h
  clang/lib/Lex/HeaderSearch.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp

Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1619,7 +1619,7 @@
   endian::Writer LE(Out, little);
   unsigned KeyLen = key.Filename.size() + 1 + 8 + 8;
   LE.write(KeyLen);
-  unsigned DataLen = 1 + 2 + 4 + 4;
+  unsigned DataLen = 1 + 2 + 4 + 4 + 4;
   for (auto ModInfo : Data.KnownHeaders)
 if (Writer.getLocalOrImportedSubmoduleID(ModInfo.getModule()))
   DataLen += 4;
@@ -1676,6 +1676,9 @@
   }
   LE.write(Offset);
 
+  // Write this file UID.
+  LE.write(Data.HFI.UID);
+
   auto EmitModule = [&](Module *M, ModuleMap::ModuleHeaderRole Role) {
 if (uint32_t ModID = Writer.getLocalOrImportedSubmoduleID(M)) {
   uint32_t Value = (ModID << 2) | (unsigned)Role;
@@ -1703,7 +1706,7 @@
 /// Write the header search block for the list of files that
 ///
 /// \param HS The header search structure to save.
-void ASTWriter::WriteHeaderSearch(const HeaderSearch &HS) {
+void ASTWriter::WriteHeaderSearch(HeaderSearch &HS) {
   HeaderFileInfoTrait GeneratorTrait(*this);
   llvm::OnDiskChainedHashTableGenerator Generator;
   SmallVector SavedStrings;
@@ -1781,8 +1784,7 @@
 // changed since it was loaded. Also skip it if it's for a modular header
 // from a different module; in that case, we rely on the module(s)
 // containing the header to provide this information.
-const HeaderFileInfo *HFI =
-HS.getExistingFileInfo(File, /*WantExternal*/!Chain);
+HeaderFileInfo *HFI = HS.getExistingFileInfo(File, /*WantExternal*/ !Chain);
 if (!HFI || (HFI->isModuleHeader && !HFI->isCompilingModuleHeader))
   continue;
 
@@ -1799,8 +1801,13 @@
 HeaderFileInfoTrait::key_type Key = {
   Filename, File->getSize(), getTimestampForOutput(File)
 };
+// Set the UID for this HFI so that its importers could use it
+// when serializing.
+HFI->UID = UID;
 HeaderFileInfoTrait::data_type Data = {
-  *HFI, HS.getModuleMap().findAllModulesForHeader(File), {}
+*HFI,
+HS.getModuleMap().findAllModulesForHeader(File),
+{},
 };
 Generator.insert(Key, Data, GeneratorTrait);
 ++NumHeaderSearchEntries;
@@ -2634,6 +2641,25 @@
   Stream.EmitRecord(SUBMODULE_IMPORTS, Record);
 }
 
+// Emit the imported header's UIDs.
+{
+  auto it = PP->Submodules.find(Mod);
+  if (it != PP->Submodules.end() && !it->second.IncludedFiles.empty()) {
+RecordData Record;
+for (unsigned UID : it->second.IncludedFiles) {
+  // Only save it if the header is actually import/pragma once.
+  // FIXME When we first see a header, it always goes into the mod's
+  // list of included, regardless of whether it was pragma-once or not.
+  // Maybe better to fix that earlier?
+  auto HFI = PP->getHeaderSearchInfo().FileInfo[UID];
+  if (HFI.isImport || HFI.isPragmaOnce) {
+Record.push_back(HFI.UID);
+  }
+}
+Stream.EmitRecord(SUBMODULE_IMPORTED_HEADERS, Record);
+  }
+}
+
 // Emit the exports.
 if (!Mod->Exports.empty()) {
   RecordData Record;
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -1,3 +1,4 @@
+
 //===- ASTReader.cpp - AST File Reader ===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
@@ -1898,6 +1899,9 @@
 HFI.Framework = HS->getUniqueFrameworkName(FrameworkName);
   }
 
+  // Read the file old UID
+  HFI.UID = endian::readNext(d);
+
   assert((End - d) % 4 == 0 &&
  "Wrong data length in HeaderFileInfo deserialization");
   while (d != End) {
@@ -5615,6 +5619,11 @@
   }
   break;
 
+case SUBMODULE_IMPORTED_HEADERS:
+  for (unsigned Idx = 0; Idx < Record.size(); ++Idx) {
+PendingImportedHeaders[&F].push_back(Record[Idx]);
+  }
+  break;
 case SUBMODULE_EXPORTS:
   for (unsigned Idx = 0; Idx + 1 < Record.size(); Idx += 2) {
 UnresolvedModuleRef Unresolved;
@@ -9271,6 +9280,37 @@
   for (auto *ND : PendingMergedDefinitionsToDeduplicate)
 getContext().deduplicateMergedDefinit

[PATCH] D76365: [cuda][hip] Add CUDA builtin surface/texture reference support.

2020-03-26 Thread Michael Liao via Phabricator via cfe-commits
hliao updated this revision to Diff 252758.
hliao added a comment.

Fix windows build and revise header change.

- When including `drivers_types.h` or `host_defines.h` with `__CUDACC__`, the 
only difference is the additional attributes added. No additional change.
- After including `host_defines.h` firstly with `__CUDACC__`, there is no 
significant change from the one including `drivers_types.h`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76365

Files:
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/Type.cpp
  clang/lib/CodeGen/CGCUDANV.cpp
  clang/lib/CodeGen/CGCUDARuntime.h
  clang/lib/CodeGen/CGExprAgg.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/CodeGen/TargetInfo.h
  clang/lib/Headers/__clang_cuda_runtime_wrapper.h
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CodeGenCUDA/surface.cu
  clang/test/CodeGenCUDA/texture.cu
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaCUDA/attr-declspec.cu
  clang/test/SemaCUDA/attributes-on-non-cuda.cu
  clang/test/SemaCUDA/bad-attributes.cu
  llvm/include/llvm/IR/Operator.h

Index: llvm/include/llvm/IR/Operator.h
===
--- llvm/include/llvm/IR/Operator.h
+++ llvm/include/llvm/IR/Operator.h
@@ -599,6 +599,25 @@
   }
 };
 
+class AddrSpaceCastOperator
+: public ConcreteOperator {
+  friend class AddrSpaceCastInst;
+  friend class ConstantExpr;
+
+public:
+  Value *getPointerOperand() { return getOperand(0); }
+
+  const Value *getPointerOperand() const { return getOperand(0); }
+
+  unsigned getSrcAddressSpace() const {
+return getPointerOperand()->getType()->getPointerAddressSpace();
+  }
+
+  unsigned getDestAddressSpace() const {
+return getType()->getPointerAddressSpace();
+  }
+};
+
 } // end namespace llvm
 
 #endif // LLVM_IR_OPERATOR_H
Index: clang/test/SemaCUDA/bad-attributes.cu
===
--- clang/test/SemaCUDA/bad-attributes.cu
+++ clang/test/SemaCUDA/bad-attributes.cu
@@ -70,3 +70,27 @@
 __device__ void device_fn() {
   __constant__ int c; // expected-error {{__constant__ variables must be global}}
 }
+
+typedef __attribute__((device_builtin_surface_type)) unsigned long long s0_ty; // expected-warning {{'device_builtin_surface_type' attribute only applies to classes}}
+typedef __attribute__((device_builtin_texture_type)) unsigned long long t0_ty; // expected-warning {{'device_builtin_texture_type' attribute only applies to classes}}
+
+struct __attribute__((device_builtin_surface_type)) s1_ref {}; // expected-error {{illegal device builtin surface reference type 's1_ref' declared here}}
+// expected-note@-1 {{'s1_ref' needs to be instantiated from a class template with proper template arguments}}
+struct __attribute__((device_builtin_texture_type)) t1_ref {}; // expected-error {{illegal device builtin texture reference type 't1_ref' declared here}}
+// expected-note@-1 {{'t1_ref' needs to be instantiated from a class template with proper template arguments}}
+
+template 
+struct __attribute__((device_builtin_surface_type)) s2_cls_template {}; // expected-error {{illegal device builtin surface reference class template 's2_cls_template' declared here}}
+// expected-note@-1 {{'s2_cls_template' needs to have exactly 2 template parameters}}
+template 
+struct __attribute__((device_builtin_texture_type)) t2_cls_template {}; // expected-error {{illegal device builtin texture reference class template 't2_cls_template' declared here}}
+// expected-note@-1 {{'t2_cls_template' needs to have exactly 3 template parameters}}
+
+template 
+struct __attribute__((device_builtin_surface_type)) s3_cls_template {}; // expected-error {{illegal device builtin surface reference class template 's3_cls_template' declared here}}
+// expected-note@-1 {{the 1st template parameter of 's3_cls_template' needs to be a type}}
+// expected-note@-2 {{the 2nd template parameter of 's3_cls_template' needs to be an integer or enum value}}
+template 
+struct __attribute__((device_builtin_texture_type)) t3_cls_template {}; // expected-error {{illegal device builtin texture reference class template 't3_cls_template' declared here}}
+// expected-note@-1 {{the 1st template parameter of 't3_cls_template' needs to be a type}}
+// expected-note@-2 {{the 3rd template parameter of 't3_cls_template' needs to be an integer or enum value}}
Index: clang/test/SemaCUDA/attributes-on-non-cuda.cu
===
--- clang/test/SemaCUDA/attributes-on-non-cuda.cu
+++ clang/test/SemaCUDA/attributes-on-non-cuda.cu
@@ -7,16 +7,19 @@
 // RUN: %clang_cc1 -DEXPECT_WARNINGS -fsyn

[PATCH] D76365: [cuda][hip] Add CUDA builtin surface/texture reference support.

2020-03-26 Thread Michael Liao via Phabricator via cfe-commits
hliao marked an inline comment as done.
hliao added inline comments.



Comment at: clang/lib/Headers/__clang_cuda_runtime_wrapper.h:82-94
 #undef __CUDACC__
 #if CUDA_VERSION < 9000
 #define __CUDABE__
 #else
+#define __CUDACC__
 #define __CUDA_LIBDEVICE__
 #endif

tra wrote:
> hliao wrote:
> > hliao wrote:
> > > tra wrote:
> > > > hliao wrote:
> > > > > tra wrote:
> > > > > > Please add comments on why __CUDACC__ is needed for driver_types.h 
> > > > > > here? AFAICT, driver_types.h does not have any conditionals that 
> > > > > > depend on __CUDACC__. What happens if it's not defined.
> > > > > > 
> > > > > > 
> > > > > `driver_types.h` includes `host_defines.h`, where macros 
> > > > > `__device_builtin_surface_type__` and 
> > > > > `__device_builtin_texture_type__` are conditional defined if 
> > > > > `__CUDACC__`.
> > > > > 
> > > > > The following is extracted from `cuda/crt/host_defines.h`
> > > > > 
> > > > > ```
> > > > > #if !defined(__CUDACC__)
> > > > > #define __device_builtin__
> > > > > #define __device_builtin_texture_type__
> > > > > #define __device_builtin_surface_type__
> > > > > #define __cudart_builtin__
> > > > > #else /* defined(__CUDACC__) */
> > > > > #define __device_builtin__ \
> > > > > __location__(device_builtin)
> > > > > #define __device_builtin_texture_type__ \
> > > > > __location__(device_builtin_texture_type)
> > > > > #define __device_builtin_surface_type__ \
> > > > > __location__(device_builtin_surface_type)
> > > > > #define __cudart_builtin__ \
> > > > > __location__(cudart_builtin)
> > > > > #endif /* !defined(__CUDACC__) */
> > > > > ```
> > > > My concern is -- what else is going to get defined? There are ~60 
> > > > references to __CUDACC__ in CUDA-10.1 headers. The wrappers are fragile 
> > > > enough that there's a good chance something may break. It does not help 
> > > > that my CUDA build bot decided to die just after we switched to 
> > > > work-from-home, so there will be no early warning if something goes 
> > > > wrong.
> > > > 
> > > > If all we need are the macros above, we may just define them. 
> > > Let me check all CUDA SDK through their dockers. Redefining sounds good 
> > > me as wll.
> > I checked headers from 7.0 to 10.0, `__device_builtin_texture_type__` and 
> > `__builtin_builtin_surface_type__` are only defined with that attributes if 
> > `__CUDACC__` is defined. As we only pre-define `__CUDA_ARCH__` in clang but 
> > flip `__CUDACC__` on and off in the wrapper headers to selectively reuse 
> > CUDA's headers. I would hear your suggestion on that.
> > BTW, macros like `__device__` are defined regardless of `__CUDACC__` from 
> > 7.0 to 10.0 as `__location(device)`. `__location__` is defined if 
> > `__CUDACC__` is present. But, different from `__device__`, 
> > `__device_builtin_texture_type__` is defined only `__CUDACC__` is defined.
> `__device_builtin_texture_type__` is defined in `host_defines.h`, which does 
> not seem to include any other files or does anything suspicious with 
> `__CUDACC__`
> 
> It may be OK to move inclusion of `host_defines.h` to the point before 
> `driver_types.h`, which happens to include the host_defines.h first, and 
> define __CUDACC__ only around `host_defines.h`.
> 
> An alternative is to add the macros just after inclusion of `host_defines.h`
> 
> In either case please verify that these attributes are the only things that's 
> changed by diffing output of `clang++ -x cuda /dev/null --cuda-host-only -dD 
> -E -o -` before and after the change.
With `__CUDACC__`, the only difference is the additional attributes added, such 
as `device_builtin_texture_type`. Attributes like `cudart_builtin` are also 
defined correctly. That should be used to start the support CUDART features.
I revised the change to include `host_defines.h` first and found there's no 
changes from the one using `driver_types.h`. We should be OK for that change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76365



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


[PATCH] D75951: Keep a list of already-included pragma-once files for mods.

2020-03-26 Thread Vy Nguyen via Phabricator via cfe-commits
oontvoo updated this revision to Diff 252760.
oontvoo added a comment.

.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75951

Files:
  clang/include/clang/Lex/HeaderSearch.h
  clang/include/clang/Lex/Preprocessor.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/include/clang/Serialization/ASTReader.h
  clang/include/clang/Serialization/ASTWriter.h
  clang/lib/Lex/HeaderSearch.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp

Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1619,7 +1619,7 @@
   endian::Writer LE(Out, little);
   unsigned KeyLen = key.Filename.size() + 1 + 8 + 8;
   LE.write(KeyLen);
-  unsigned DataLen = 1 + 2 + 4 + 4;
+  unsigned DataLen = 1 + 2 + 4 + 4 + 4;
   for (auto ModInfo : Data.KnownHeaders)
 if (Writer.getLocalOrImportedSubmoduleID(ModInfo.getModule()))
   DataLen += 4;
@@ -1676,6 +1676,9 @@
   }
   LE.write(Offset);
 
+  // Write this file UID.
+  LE.write(Data.HFI.UID);
+
   auto EmitModule = [&](Module *M, ModuleMap::ModuleHeaderRole Role) {
 if (uint32_t ModID = Writer.getLocalOrImportedSubmoduleID(M)) {
   uint32_t Value = (ModID << 2) | (unsigned)Role;
@@ -1703,7 +1706,7 @@
 /// Write the header search block for the list of files that
 ///
 /// \param HS The header search structure to save.
-void ASTWriter::WriteHeaderSearch(const HeaderSearch &HS) {
+void ASTWriter::WriteHeaderSearch(HeaderSearch &HS) {
   HeaderFileInfoTrait GeneratorTrait(*this);
   llvm::OnDiskChainedHashTableGenerator Generator;
   SmallVector SavedStrings;
@@ -1781,8 +1784,7 @@
 // changed since it was loaded. Also skip it if it's for a modular header
 // from a different module; in that case, we rely on the module(s)
 // containing the header to provide this information.
-const HeaderFileInfo *HFI =
-HS.getExistingFileInfo(File, /*WantExternal*/!Chain);
+HeaderFileInfo *HFI = HS.getExistingFileInfo(File, /*WantExternal*/ !Chain);
 if (!HFI || (HFI->isModuleHeader && !HFI->isCompilingModuleHeader))
   continue;
 
@@ -1799,8 +1801,13 @@
 HeaderFileInfoTrait::key_type Key = {
   Filename, File->getSize(), getTimestampForOutput(File)
 };
+// Set the UID for this HFI so that its importers could use it
+// when serializing.
+HFI->UID = UID;
 HeaderFileInfoTrait::data_type Data = {
-  *HFI, HS.getModuleMap().findAllModulesForHeader(File), {}
+*HFI,
+HS.getModuleMap().findAllModulesForHeader(File),
+{},
 };
 Generator.insert(Key, Data, GeneratorTrait);
 ++NumHeaderSearchEntries;
@@ -2634,6 +2641,25 @@
   Stream.EmitRecord(SUBMODULE_IMPORTS, Record);
 }
 
+// Emit the imported header's UIDs.
+{
+  auto it = PP->Submodules.find(Mod);
+  if (it != PP->Submodules.end() && !it->second.IncludedFiles.empty()) {
+RecordData Record;
+for (unsigned UID : it->second.IncludedFiles) {
+  // Only save it if the header is actually import/pragma once.
+  // FIXME When we first see a header, it always goes into the mod's
+  // list of included, regardless of whether it was pragma-once or not.
+  // Maybe better to fix that earlier?
+  auto HFI = PP->getHeaderSearchInfo().FileInfo[UID];
+  if (HFI.isImport || HFI.isPragmaOnce) {
+Record.push_back(HFI->UID);
+  }
+}
+Stream.EmitRecord(SUBMODULE_IMPORTED_HEADERS, Record);
+  }
+}
+
 // Emit the exports.
 if (!Mod->Exports.empty()) {
   RecordData Record;
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -1,3 +1,4 @@
+
 //===- ASTReader.cpp - AST File Reader ===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
@@ -1898,6 +1899,9 @@
 HFI.Framework = HS->getUniqueFrameworkName(FrameworkName);
   }
 
+  // Read the file old UID
+  HFI.UID = endian::readNext(d);
+
   assert((End - d) % 4 == 0 &&
  "Wrong data length in HeaderFileInfo deserialization");
   while (d != End) {
@@ -5615,6 +5619,11 @@
   }
   break;
 
+case SUBMODULE_IMPORTED_HEADERS:
+  for (unsigned Idx = 0; Idx < Record.size(); ++Idx) {
+PendingImportedHeaders[&F].push_back(Record[Idx]);
+  }
+  break;
 case SUBMODULE_EXPORTS:
   for (unsigned Idx = 0; Idx + 1 < Record.size(); Idx += 2) {
 UnresolvedModuleRef Unresolved;
@@ -9271,6 +9280,37 @@
   for (auto *ND : PendingMergedDefinitionsToDeduplicate)
 getContext().deduplicateMergedDefini

[clang] a3f4d17 - [Analyzer] Use note tags to track container begin and and changes

2020-03-26 Thread Adam Balogh via cfe-commits

Author: Adam Balogh
Date: 2020-03-26T07:56:28+01:00
New Revision: a3f4d17a1a53c4144a5bb7c14620a5d2790f36ea

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

LOG: [Analyzer] Use note tags to track container begin and and changes

Container operations such as `push_back()`, `pop_front()`
etc. increment and decrement the abstract begin and end
symbols of containers. This patch introduces note tags
to `ContainerModeling` to track these changes. This helps
the user to better identify the source of errors related
to containers and iterators.

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
clang/test/Analysis/container-modeling.cpp
clang/test/Analysis/iterator-range.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp 
b/clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
index 7d8dc8b8a0ab..b225a61db439 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
@@ -31,47 +31,43 @@ namespace {
 class ContainerModeling
   : public Checker {
 
-  void handleBegin(CheckerContext &C, const Expr *CE, const SVal &RetVal,
-   const SVal &Cont) const;
-  void handleEnd(CheckerContext &C, const Expr *CE, const SVal &RetVal,
- const SVal &Cont) const;
-  void handleAssignment(CheckerContext &C, const SVal &Cont,
-const Expr *CE = nullptr,
-const SVal &OldCont = UndefinedVal()) const;
-  void handleAssign(CheckerContext &C, const SVal &Cont) const;
-  void handleClear(CheckerContext &C, const SVal &Cont) const;
-  void handlePushBack(CheckerContext &C, const SVal &Cont) const;
-  void handlePopBack(CheckerContext &C, const SVal &Cont) const;
-  void handlePushFront(CheckerContext &C, const SVal &Cont) const;
-  void handlePopFront(CheckerContext &C, const SVal &Cont) const;
-  void handleInsert(CheckerContext &C, const SVal &Cont,
-const SVal &Iter) const;
-  void handleErase(CheckerContext &C, const SVal &Cont, const SVal &Iter) 
const;
-  void handleErase(CheckerContext &C, const SVal &Cont, const SVal &Iter1,
-   const SVal &Iter2) const;
-  void handleEraseAfter(CheckerContext &C, const SVal &Cont,
-const SVal &Iter) const;
-  void handleEraseAfter(CheckerContext &C, const SVal &Cont, const SVal &Iter1,
-const SVal &Iter2) const;
+  void handleBegin(CheckerContext &C, const Expr *CE, SVal RetVal,
+   SVal Cont) const;
+  void handleEnd(CheckerContext &C, const Expr *CE, SVal RetVal,
+ SVal Cont) const;
+  void handleAssignment(CheckerContext &C, SVal Cont, const Expr *CE = nullptr,
+SVal OldCont = UndefinedVal()) const;
+  void handleAssign(CheckerContext &C, SVal Cont, const Expr *ContE) const;
+  void handleClear(CheckerContext &C, SVal Cont, const Expr *ContE) const;
+  void handlePushBack(CheckerContext &C, SVal Cont, const Expr *ContE) const;
+  void handlePopBack(CheckerContext &C, SVal Cont, const Expr *ContE) const;
+  void handlePushFront(CheckerContext &C, SVal Cont, const Expr *ContE) const;
+  void handlePopFront(CheckerContext &C, SVal Cont, const Expr *ContE) const;
+  void handleInsert(CheckerContext &C, SVal Cont, SVal Iter) const;
+  void handleErase(CheckerContext &C, SVal Cont, SVal Iter) const;
+  void handleErase(CheckerContext &C, SVal Cont, SVal Iter1, SVal Iter2) const;
+  void handleEraseAfter(CheckerContext &C, SVal Cont, SVal Iter) const;
+  void handleEraseAfter(CheckerContext &C, SVal Cont, SVal Iter1,
+SVal Iter2) const;
+  const NoteTag *getChangeTag(CheckerContext &C, StringRef Text,
+  const MemRegion *ContReg,
+  const Expr *ContE) const;
   void printState(raw_ostream &Out, ProgramStateRef State, const char *NL,
   const char *Sep) const override;
 
 public:
-  ContainerModeling() {}
+  ContainerModeling() = default;
 
   void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
   void checkLiveSymbols(ProgramStateRef State, SymbolReaper &SR) const;
   void checkDeadSymbols(SymbolReaper &SR, CheckerContext &C) const;
 
-  typedef void (ContainerModeling::*NoItParamFn)(CheckerContext &,
- const SVal &) const;
-  typedef void (ContainerModeling::*OneItParamFn)(CheckerContext &,
-  const SVal &,
-  const SVal &) const;
-  typedef void

[clang] a9ab11d - [AST] Build recovery expressions for nonexistent member exprs.

2020-03-26 Thread Haojian Wu via cfe-commits

Author: Haojian Wu
Date: 2020-03-26T08:50:33+01:00
New Revision: a9ab11d40830cc01c3a6ff97633140e8bd1e431e

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

LOG: [AST] Build recovery expressions for nonexistent member exprs.

Summary: Previously, we dropped the AST node for nonexistent member exprs.

Reviewers: sammccall

Subscribers: cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
clang/lib/Parse/ParseExpr.cpp
clang/test/AST/ast-dump-recovery.cpp
clang/test/SemaCXX/constructor-initializer.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp 
b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
index 9334d9d17608..eb4fdc98fb0b 100644
--- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -678,7 +678,7 @@ void foo() {
 TEST(IncludeFixerTest, NoCrashMemebrAccess) {
   Annotations Test(R"cpp(// error-ok
 struct X { int  xyz; };
-void g() { X x; x.$[[xy]] }
+void g() { X x; x.$[[xy]]; }
   )cpp");
   auto TU = TestTU::withCode(Test.code());
   auto Index = buildIndexWithSymbol(

diff  --git a/clang/lib/Parse/ParseExpr.cpp b/clang/lib/Parse/ParseExpr.cpp
index bcd5679cb43f..4cb828cd23cc 100644
--- a/clang/lib/Parse/ParseExpr.cpp
+++ b/clang/lib/Parse/ParseExpr.cpp
@@ -2102,8 +2102,14 @@ Parser::ParsePostfixExpressionSuffix(ExprResult LHS) {
 OpKind, SS, TemplateKWLoc, Name,
  CurParsedObjCImpl ? CurParsedObjCImpl->Dcl
: nullptr);
-  if (!LHS.isInvalid() && Tok.is(tok::less))
-checkPotentialAngleBracket(LHS);
+  if (!LHS.isInvalid()) {
+if (Tok.is(tok::less))
+  checkPotentialAngleBracket(LHS);
+  } else if (OrigLHS && Name.isValid()) {
+// Preserve the LHS if the RHS is an invalid member.
+LHS = Actions.CreateRecoveryExpr(OrigLHS->getBeginLoc(),
+ Name.getEndLoc(), {OrigLHS});
+  }
   break;
 }
 case tok::plusplus:// postfix-expression: postfix-expression '++'

diff  --git a/clang/test/AST/ast-dump-recovery.cpp 
b/clang/test/AST/ast-dump-recovery.cpp
index beb409edc4a6..9ccfc5a106f8 100644
--- a/clang/test/AST/ast-dump-recovery.cpp
+++ b/clang/test/AST/ast-dump-recovery.cpp
@@ -83,3 +83,18 @@ int binary = a + nullptr;
 // CHECK-NEXT:  `-DeclRefExpr {{.*}} 'a'
 // DISABLED-NOT: -RecoveryExpr {{.*}} contains-errors
 int ternary = a ? nullptr : a;
+
+// CHECK: FunctionDecl
+// CHECK-NEXT:|-ParmVarDecl {{.*}} x
+// CHECK-NEXT:`-CompoundStmt
+// CHECK-NEXT: |-RecoveryExpr {{.*}} contains-errors
+// CHECK-NEXT: | `-DeclRefExpr {{.*}} 'foo'
+// CHECK-NEXT: `-CallExpr {{.*}} contains-errors
+// CHECK-NEXT:  |-RecoveryExpr {{.*}} contains-errors
+// CHECK-NEXT:  | `-DeclRefExpr {{.*}} 'foo'
+// CHECK-NEXT:  `-DeclRefExpr {{.*}} 'x'
+struct Foo {} foo;
+void test(int x) {
+  foo.abc;
+  foo->func(x);
+}
\ No newline at end of file

diff  --git a/clang/test/SemaCXX/constructor-initializer.cpp 
b/clang/test/SemaCXX/constructor-initializer.cpp
index 102ff1e80d03..df8991416712 100644
--- a/clang/test/SemaCXX/constructor-initializer.cpp
+++ b/clang/test/SemaCXX/constructor-initializer.cpp
@@ -250,7 +250,7 @@ namespace test3 {
 B(const String& s, int e=0) // expected-error {{unknown type name}} 
   : A(e), m_String(s) , m_ErrorStr(__null) {} // expected-error {{no 
matching constructor}} expected-error {{does not name}}
 B(const B& e)
-  : A(e), m_String(e.m_String), m_ErrorStr(__null) { // expected-error 
{{does not name}} \
+  : A(e), m_String(e.m_String), m_ErrorStr(__null) { // expected-error 
2{{does not name}} \
   // expected-error {{no member named 'm_String' in 'test3::B'}}
 }
   };



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


[PATCH] D76764: [AST] Build recovery expressions for nonexistent member exprs.

2020-03-26 Thread Haojian Wu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa9ab11d40830: [AST] Build recovery expressions for 
nonexistent member exprs. (authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76764

Files:
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/test/AST/ast-dump-recovery.cpp
  clang/test/SemaCXX/constructor-initializer.cpp


Index: clang/test/SemaCXX/constructor-initializer.cpp
===
--- clang/test/SemaCXX/constructor-initializer.cpp
+++ clang/test/SemaCXX/constructor-initializer.cpp
@@ -250,7 +250,7 @@
 B(const String& s, int e=0) // expected-error {{unknown type name}} 
   : A(e), m_String(s) , m_ErrorStr(__null) {} // expected-error {{no 
matching constructor}} expected-error {{does not name}}
 B(const B& e)
-  : A(e), m_String(e.m_String), m_ErrorStr(__null) { // expected-error 
{{does not name}} \
+  : A(e), m_String(e.m_String), m_ErrorStr(__null) { // expected-error 
2{{does not name}} \
   // expected-error {{no member named 'm_String' in 'test3::B'}}
 }
   };
Index: clang/test/AST/ast-dump-recovery.cpp
===
--- clang/test/AST/ast-dump-recovery.cpp
+++ clang/test/AST/ast-dump-recovery.cpp
@@ -83,3 +83,18 @@
 // CHECK-NEXT:  `-DeclRefExpr {{.*}} 'a'
 // DISABLED-NOT: -RecoveryExpr {{.*}} contains-errors
 int ternary = a ? nullptr : a;
+
+// CHECK: FunctionDecl
+// CHECK-NEXT:|-ParmVarDecl {{.*}} x
+// CHECK-NEXT:`-CompoundStmt
+// CHECK-NEXT: |-RecoveryExpr {{.*}} contains-errors
+// CHECK-NEXT: | `-DeclRefExpr {{.*}} 'foo'
+// CHECK-NEXT: `-CallExpr {{.*}} contains-errors
+// CHECK-NEXT:  |-RecoveryExpr {{.*}} contains-errors
+// CHECK-NEXT:  | `-DeclRefExpr {{.*}} 'foo'
+// CHECK-NEXT:  `-DeclRefExpr {{.*}} 'x'
+struct Foo {} foo;
+void test(int x) {
+  foo.abc;
+  foo->func(x);
+}
\ No newline at end of file
Index: clang/lib/Parse/ParseExpr.cpp
===
--- clang/lib/Parse/ParseExpr.cpp
+++ clang/lib/Parse/ParseExpr.cpp
@@ -2102,8 +2102,14 @@
 OpKind, SS, TemplateKWLoc, Name,
  CurParsedObjCImpl ? CurParsedObjCImpl->Dcl
: nullptr);
-  if (!LHS.isInvalid() && Tok.is(tok::less))
-checkPotentialAngleBracket(LHS);
+  if (!LHS.isInvalid()) {
+if (Tok.is(tok::less))
+  checkPotentialAngleBracket(LHS);
+  } else if (OrigLHS && Name.isValid()) {
+// Preserve the LHS if the RHS is an invalid member.
+LHS = Actions.CreateRecoveryExpr(OrigLHS->getBeginLoc(),
+ Name.getEndLoc(), {OrigLHS});
+  }
   break;
 }
 case tok::plusplus:// postfix-expression: postfix-expression '++'
Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -678,7 +678,7 @@
 TEST(IncludeFixerTest, NoCrashMemebrAccess) {
   Annotations Test(R"cpp(// error-ok
 struct X { int  xyz; };
-void g() { X x; x.$[[xy]] }
+void g() { X x; x.$[[xy]]; }
   )cpp");
   auto TU = TestTU::withCode(Test.code());
   auto Index = buildIndexWithSymbol(


Index: clang/test/SemaCXX/constructor-initializer.cpp
===
--- clang/test/SemaCXX/constructor-initializer.cpp
+++ clang/test/SemaCXX/constructor-initializer.cpp
@@ -250,7 +250,7 @@
 B(const String& s, int e=0) // expected-error {{unknown type name}} 
   : A(e), m_String(s) , m_ErrorStr(__null) {} // expected-error {{no matching constructor}} expected-error {{does not name}}
 B(const B& e)
-  : A(e), m_String(e.m_String), m_ErrorStr(__null) { // expected-error {{does not name}} \
+  : A(e), m_String(e.m_String), m_ErrorStr(__null) { // expected-error 2{{does not name}} \
   // expected-error {{no member named 'm_String' in 'test3::B'}}
 }
   };
Index: clang/test/AST/ast-dump-recovery.cpp
===
--- clang/test/AST/ast-dump-recovery.cpp
+++ clang/test/AST/ast-dump-recovery.cpp
@@ -83,3 +83,18 @@
 // CHECK-NEXT:  `-DeclRefExpr {{.*}} 'a'
 // DISABLED-NOT: -RecoveryExpr {{.*}} contains-errors
 int ternary = a ? nullptr : a;
+
+// CHECK: FunctionDecl
+// CHECK-NEXT:|-ParmVarDecl {{.*}} x
+// CHECK-NEXT:`-CompoundStmt
+// CHECK-NEXT: |-RecoveryExpr {{.*}} contains-errors
+// CHECK-NEXT: | `-DeclRefExpr {{.*}} 'foo'
+// CHECK-NEXT: `-CallExpr {{.*}} contains-errors
+// CHECK-NEXT:  |-RecoveryExpr {{.*}} contains-errors
+/

[PATCH] D73720: [Analyzer] Use note tags to track container begin and and changes

2020-03-26 Thread Balogh, Ádám via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa3f4d17a1a53: [Analyzer] Use note tags to track container 
begin and and changes (authored by baloghadamsoftware).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73720

Files:
  clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
  clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
  clang/test/Analysis/container-modeling.cpp
  clang/test/Analysis/iterator-range.cpp

Index: clang/test/Analysis/iterator-range.cpp
===
--- clang/test/Analysis/iterator-range.cpp
+++ clang/test/Analysis/iterator-range.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.IteratorRange -analyzer-config aggressive-binary-operation-simplification=true -analyzer-config c++-container-inlining=false %s -verify
-// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.IteratorRange -analyzer-config aggressive-binary-operation-simplification=true -analyzer-config c++-container-inlining=true -DINLINE=1 %s -verify
+// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.IteratorRange -analyzer-config aggressive-binary-operation-simplification=true -analyzer-config c++-container-inlining=false -analyzer-output=text %s -verify
+// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.IteratorRange -analyzer-config aggressive-binary-operation-simplification=true -analyzer-config c++-container-inlining=true -DINLINE=1 -analyzer-output=text %s -verify
 
 #include "Inputs/system-header-simulator-cxx.h"
 
@@ -32,6 +32,7 @@
 void deref_end(const std::vector &V) {
   auto i = V.end();
   *i; // expected-warning{{Past-the-end iterator dereferenced}}
+  // expected-note@-1{{Past-the-end iterator dereferenced}}
 }
 
 // Prefix increment - operator++()
@@ -59,6 +60,7 @@
 void incr_end(const std::vector &V) {
   auto i = V.end();
   ++i; // expected-warning{{Iterator incremented behind the past-the-end iterator}}
+   // expected-note@-1{{Iterator incremented behind the past-the-end iterator}}
 }
 
 // Postfix increment - operator++(int)
@@ -86,6 +88,7 @@
 void end_incr(const std::vector &V) {
   auto i = V.end();
   i++; // expected-warning{{Iterator incremented behind the past-the-end iterator}}
+   // expected-note@-1{{Iterator incremented behind the past-the-end iterator}}
 }
 
 // Prefix decrement - operator--()
@@ -93,6 +96,7 @@
 void decr_begin(const std::vector &V) {
   auto i = V.begin();
   --i; // expected-warning{{Iterator decremented ahead of its valid range}}
+   // expected-note@-1{{Iterator decremented ahead of its valid range}}
 }
 
 void decr_behind_begin(const std::vector &V) {
@@ -120,6 +124,7 @@
 void begin_decr(const std::vector &V) {
   auto i = V.begin();
   i--; // expected-warning{{Iterator decremented ahead of its valid range}}
+   // expected-note@-1{{Iterator decremented ahead of its valid range}}
 }
 
 void behind_begin_decr(const std::vector &V) {
@@ -168,11 +173,13 @@
 void incr_by_2_ahead_of_end(const std::vector &V) {
   auto i = --V.end();
   i += 2; // expected-warning{{Iterator incremented behind the past-the-end iterator}}
+  // expected-note@-1{{Iterator incremented behind the past-the-end iterator}}
 }
 
 void incr_by_2_end(const std::vector &V) {
   auto i = V.end();
   i += 2; // expected-warning{{Iterator incremented behind the past-the-end iterator}}
+  // expected-note@-1{{Iterator incremented behind the past-the-end iterator}}
 }
 
 // Addition - operator+(int)
@@ -201,11 +208,13 @@
 void incr_by_2_copy_ahead_of_end(const std::vector &V) {
   auto i = --V.end();
   auto j = i + 2; // expected-warning{{Iterator incremented behind the past-the-end iterator}}
+  // expected-note@-1{{Iterator incremented behind the past-the-end iterator}}
 }
 
 void incr_by_2_copy_end(const std::vector &V) {
   auto i = V.end();
   auto j = i + 2; // expected-warning{{Iterator incremented behind the past-the-end iterator}}
+  // expected-note@-1{{Iterator incremented behind the past-the-end iterator}}
 }
 
 // Subtraction assignment - operator-=(int)
@@ -213,11 +222,13 @@
 void decr_by_2_begin(const std::vector &V) {
   auto i = V.begin();
   i -= 2; // expected-warning{{Iterator decremented ahead of its valid range}}
+  // expected-note@-1{{Iterator decremented ahead of its valid range}}
 }
 
 void decr_by_2_behind_begin(const std::vector &V) {
   auto i = ++V.begin();
   i -= 2; // expected-warning{{Iterator decremented ahead of its valid range}}
+  // expected-note@-1{{Iterator decremented ahead of its valid range}}
 }
 
 void decr_by_2_behind_begin_by_2(const std::vector &V) {
@@ -246,11 +257,13 @@
 void decr_by_2_copy_begin(const std::vector &V) {
 

[PATCH] D76801: [AST] Print a> without extra spaces in C++11 or later.

2020-03-26 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision.
kadircet marked an inline comment as done.
kadircet added a comment.
This revision is now accepted and ready to land.

LGTM, thanks !




Comment at: clang/unittests/AST/DeclPrinterTest.cpp:1161
 "A",
-"Z > A"));
-// Should be: with semicolon, without extra space in "> >"
+"Z> A"));
+// Should be: with semicolon

testing scheme here seems to be annoying, had to search for 
`TestTemplateArgumentList8` for a pre-cxx11 case :D

but nothing much to do ...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76801



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


[PATCH] D76572: Replace `T(x)` with `reinterpret_cast(x)` everywhere it means reinterpret_cast. No functional change

2020-03-26 Thread James Henderson via Phabricator via cfe-commits
jhenderson added a comment.

In D76572#1942024 , @Quuxplusone wrote:

> Btw, @jhenderson and @rsmith, if you wanted to land your own files' pieces of 
> this patch while waiting on whoever-it-is to approve the pieces in other 
> files, that'd be cool. As I mentioned initially, I don't have the access 
> necessary to land any part of it myself.


From my point of view, I don't see any particular rush to land this, so let's 
just keep it all as one, and wait for the other bits to be reviewed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76572



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


[PATCH] D53272: Add target requirement to profile remap test.

2020-03-26 Thread Yvan Roux via Phabricator via cfe-commits
yroux closed this revision.
yroux added a comment.
Herald added a subscriber: danielkiss.

This was closed by commit r344593


Repository:
  rC Clang

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

https://reviews.llvm.org/D53272



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


[PATCH] D76770: [CodeComplete] Don't replace the rest of line in #include completion.

2020-03-26 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment.

In D76770#1941603 , @sammccall wrote:

> Does this regress the case where you complete a file rather than a directory?
>  e.g. `#include "foo/^one.h"`
>  If you select "two.h", then we definitely want to replace "one.h", right?


yes, it has a regression, but we don't change the behavior for the above case. 
A regression case is like

  #include "foo/^dir1/one.h"
  // with two candidates: dir2 and foo.h

if we choose 2), after completion, `"foo/foo.h/one.h"` (now) vs `foo/foo.h` 
(before)

> The root here is that we have to decide what to replace independently of the 
> completion item.
>  One compromise might be to replace up to the quote, but if you're completing 
> `"foo/^one.h"` and `bar` is a directory, check if `foo/bar/one.h` exists and 
> offer it as a completion. Seems complicated though.

agree, it doesn't seem to be worth the effort.

> Or we can take the tradeoff here if it's better but it'd be good to 
> understand why.

it was originally reported by one internal user.

I think the new behavior (with this patch)

- is more aligned with mental model of code completion (most people think code 
completion just inserts the code text at the cursor position )
- keeps the consistent behavior with other code completions, e.g. 
`foo.^member()` if you select the `member` in code completion, another `member` 
text will be inserted


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76770



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


[clang] 1a27d63 - [Analyzer] Only add container note tags to the operations of the affected container

2020-03-26 Thread Adam Balogh via cfe-commits

Author: Adam Balogh
Date: 2020-03-26T09:44:16+01:00
New Revision: 1a27d63a8891076ad9176f1a70f372003bc55c2f

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

LOG: [Analyzer] Only add container note tags to the operations of the affected 
container

If an error happens which is related to a container the Container
Modeling checker adds note tags to all the container operations along
the bug path. This may be disturbing if there are other containers
beside the one which is affected by the bug. This patch restricts the
note tags to only the affected container and adjust the debug checkers
to be able to test this change.

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
clang/lib/StaticAnalyzer/Checkers/DebugContainerModeling.cpp
clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
clang/test/Analysis/container-modeling.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp 
b/clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
index b225a61db439..8126fe8260c8 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
@@ -721,6 +721,9 @@ const NoteTag 
*ContainerModeling::getChangeTag(CheckerContext &C,
 
   return C.getNoteTag(
   [Text, Name, ContReg](PathSensitiveBugReport &BR) -> std::string {
+if (!BR.isInteresting(ContReg))
+  return "";
+
 SmallString<256> Msg;
 llvm::raw_svector_ostream Out(Msg);
 Out << "Container " << (!Name.empty() ? ("'" + Name.str() + "' ") : "" 
)

diff  --git a/clang/lib/StaticAnalyzer/Checkers/DebugContainerModeling.cpp 
b/clang/lib/StaticAnalyzer/Checkers/DebugContainerModeling.cpp
index 8d0572723991..ce8dccb22333 100644
--- a/clang/lib/StaticAnalyzer/Checkers/DebugContainerModeling.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/DebugContainerModeling.cpp
@@ -92,7 +92,19 @@ void 
DebugContainerModeling::analyzerContainerDataField(const CallExpr *CE,
   if (Field) {
 State = State->BindExpr(CE, C.getLocationContext(),
 nonloc::SymbolVal(Field));
-C.addTransition(State);
+
+// Progpagate interestingness from the container's data (marked
+// interesting by an `ExprInspection` debug call to the container
+// itself.
+const NoteTag *InterestingTag =
+  C.getNoteTag(
+  [Cont, Field](PathSensitiveBugReport &BR) -> std::string {
+if (BR.isInteresting(Field)) {
+  BR.markInteresting(Cont);
+}
+return "";
+  });
+C.addTransition(State, InterestingTag);
 return;
   }
 }

diff  --git a/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
index 10b27831d89f..97e287e7a221 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
@@ -52,9 +52,12 @@ class ExprInspectionChecker : public Checker ExprVal = None) const;
   ExplodedNode *reportBug(llvm::StringRef Msg, BugReporter &BR,
-  ExplodedNode *N) const;
+  ExplodedNode *N,
+  Optional ExprVal = None) const;
 
 public:
   bool evalCall(const CallEvent &Call, CheckerContext &C) const;
@@ -144,22 +147,28 @@ static const char *getArgumentValueString(const CallExpr 
*CE,
 }
 
 ExplodedNode *ExprInspectionChecker::reportBug(llvm::StringRef Msg,
-   CheckerContext &C) const {
+   CheckerContext &C,
+   Optional ExprVal) const {
   ExplodedNode *N = C.generateNonFatalErrorNode();
-  reportBug(Msg, C.getBugReporter(), N);
+  reportBug(Msg, C.getBugReporter(), N, ExprVal);
   return N;
 }
 
 ExplodedNode *ExprInspectionChecker::reportBug(llvm::StringRef Msg,
BugReporter &BR,
-   ExplodedNode *N) const {
+   ExplodedNode *N,
+   Optional ExprVal) const {
   if (!N)
 return nullptr;
 
   if (!BT)
 BT.reset(new BugType(this, "Checking analyzer assumptions", "debug"));
 
-  BR.emitReport(std::make_unique(*BT, Msg, N));
+  auto R = std::make_unique(*BT, Msg, N);
+  if (ExprVal) {
+R->markInteresting(*ExprVal);
+  }
+  BR.emitReport(std::move(R));
   return N;
 }
 
@@ -406,7 +415,8 @@ void ExprInspectionChecker::analyzerExpress(const CallExp

[clang] 159a9f7 - [AST] Print a> without extra spaces in C++11 or later.

2020-03-26 Thread Sam McCall via cfe-commits

Author: Sam McCall
Date: 2020-03-26T09:53:54+01:00
New Revision: 159a9f7e76307734bcdcae3357640e42e0733194

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

LOG: [AST] Print a> without extra spaces in C++11 or later.

Summary: It's not 1998 anymore.

Reviewers: kadircet

Subscribers: jkorous, arphaman, usaxena95, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clangd/unittests/HoverTests.cpp
clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
clang/include/clang/AST/PrettyPrinter.h
clang/lib/AST/TypePrinter.cpp
clang/test/CXX/expr/expr.prim/expr.prim.req/type-requirement.cpp
clang/test/CXX/temp/temp.arg/temp.arg.template/p3-0x.cpp
clang/test/CodeGenCXX/debug-info-template-explicit-specialization.cpp
clang/test/Misc/diag-aka-types.cpp
clang/test/Modules/ExtDebugInfo.cpp
clang/test/Modules/ModuleDebugInfo.cpp
clang/test/OpenMP/distribute_parallel_for_private_messages.cpp
clang/test/OpenMP/distribute_parallel_for_simd_private_messages.cpp
clang/test/OpenMP/distribute_simd_private_messages.cpp
clang/test/OpenMP/for_private_messages.cpp
clang/test/OpenMP/for_simd_private_messages.cpp
clang/test/OpenMP/master_taskloop_private_messages.cpp
clang/test/OpenMP/master_taskloop_simd_private_messages.cpp
clang/test/OpenMP/parallel_for_private_messages.cpp
clang/test/OpenMP/parallel_for_simd_private_messages.cpp
clang/test/OpenMP/parallel_master_private_messages.cpp
clang/test/OpenMP/parallel_master_taskloop_private_messages.cpp
clang/test/OpenMP/parallel_master_taskloop_simd_private_messages.cpp
clang/test/OpenMP/parallel_sections_private_messages.cpp
clang/test/OpenMP/sections_private_messages.cpp
clang/test/OpenMP/simd_private_messages.cpp
clang/test/OpenMP/single_private_messages.cpp
clang/test/OpenMP/target_firstprivate_messages.cpp
clang/test/OpenMP/target_parallel_for_private_messages.cpp
clang/test/OpenMP/target_parallel_for_simd_private_messages.cpp
clang/test/OpenMP/target_private_messages.cpp
clang/test/OpenMP/target_simd_private_messages.cpp
clang/test/OpenMP/taskloop_private_messages.cpp
clang/test/OpenMP/taskloop_simd_private_messages.cpp
clang/test/SemaTemplate/instantiate-member-expr.cpp
clang/unittests/AST/DeclPrinterTest.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp 
b/clang-tools-extra/clangd/unittests/HoverTests.cpp
index 593fb16e2194..99fd1c572447 100644
--- a/clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -1567,7 +1567,7 @@ TEST(Hover, All) {
 HI.Kind = index::SymbolKind::Variable;
 HI.NamespaceScope = "";
 HI.Name = "foo";
-HI.Type = "cls > >";
+HI.Type = "cls>>";
 HI.Value = "{}";
   }},
   {
@@ -1579,7 +1579,7 @@ TEST(Hover, All) {
 HI.Definition = "template <> struct cls>> {}";
 HI.Kind = index::SymbolKind::Struct;
 HI.NamespaceScope = "";
-HI.Name = "cls > >";
+HI.Name = "cls>>";
 HI.Documentation = "type of nested templates.";
   }},
   {

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp 
b/clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
index eb3316a4a04e..2eccc9066fa6 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
@@ -247,19 +247,19 @@ typedef Q Q3_t;
 
 typedef TwoArgTemplate >, S<(0 < 0), Q > > Nested_t;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: using Nested_t = TwoArgTemplate >, S<(0 < 0), Q > >;
+// CHECK-FIXES: using Nested_t = TwoArgTemplate>, S<(0 < 0), Q>>;
 
 template 
 class Variadic {};
 
 typedef Variadic >, S<(0 < 0), 
Variadic > > > Variadic_t;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: using Variadic_t = Variadic 
>, S<(0 < 0), Variadic > > >
+// CHECK-FIXES: using Variadic_t = Variadic>, S<(0 < 0), Variadic>>>
 
 typedef Variadic >, S<(0 < 0), 
Variadic > > > Variadic_t, *Variadic_p;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
 // CHECK-MESSAGES: :[[@LINE-2]]:103: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: using Variadic_t = Variadic 
>, S<(0 < 0), Variadic > > >;
+// CHECK-FIXES: using Variadic_t = Variadic>, S<(0 < 0), Variadic>>>;
 // CHECK-FIXES-NEXT: using Variadic_p = Variadic_t*;
 
 typedef struct { int a; } R_t, *R_p;

diff  --git a/clang/include/cla

[clang-tools-extra] 6324912 - [clangd] Simplify "preferred" vs "definition" logic a bit in XRefs AST code.

2020-03-26 Thread Sam McCall via cfe-commits

Author: Sam McCall
Date: 2020-03-26T09:52:48+01:00
New Revision: 6324912592a1ff8d672e55e02ca63f769decb154

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

LOG: [clangd] Simplify "preferred" vs "definition" logic a bit in XRefs AST 
code.

Summary:
Now Preferred is always the canonical (first) decl, Definition is always the def
if available.

In practice the index was already forcing this behaviour anyway, so there's no
change. (Unless you weren't using this index, in which case this patch makes
textDocument/declaration and toggling work as expected).

Reviewers: kadircet

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

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clangd/XRefs.cpp
clang-tools-extra/clangd/unittests/XRefsTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/XRefs.cpp 
b/clang-tools-extra/clangd/XRefs.cpp
index 7d55a372905c..7e27be38bcc3 100644
--- a/clang-tools-extra/clangd/XRefs.cpp
+++ b/clang-tools-extra/clangd/XRefs.cpp
@@ -226,23 +226,21 @@ locateASTReferent(SourceLocation CurLoc, const 
syntax::Token *TouchedIdentifier,
   llvm::DenseMap ResultIndex;
 
   auto AddResultDecl = [&](const NamedDecl *D) {
-const NamedDecl *Def = getDefinition(D);
-const NamedDecl *Preferred = Def ? Def : D;
-
-auto Loc = makeLocation(AST.getASTContext(), nameLocation(*Preferred, SM),
-MainFilePath);
+D = llvm::cast(D->getCanonicalDecl());
+auto Loc =
+makeLocation(AST.getASTContext(), nameLocation(*D, SM), MainFilePath);
 if (!Loc)
   return;
 
 Result.emplace_back();
-Result.back().Name = printName(AST.getASTContext(), *Preferred);
+Result.back().Name = printName(AST.getASTContext(), *D);
 Result.back().PreferredDeclaration = *Loc;
-// Preferred is always a definition if possible, so this check works.
-if (Def == Preferred)
-  Result.back().Definition = *Loc;
+if (const NamedDecl *Def = getDefinition(D))
+  Result.back().Definition = makeLocation(
+  AST.getASTContext(), nameLocation(*Def, SM), MainFilePath);
 
 // Record SymbolID for index lookup later.
-if (auto ID = getSymbolID(Preferred))
+if (auto ID = getSymbolID(D))
   ResultIndex[*ID] = Result.size() - 1;
   };
 

diff  --git a/clang-tools-extra/clangd/unittests/XRefsTests.cpp 
b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
index fc36dfa42d7f..6b568456ba02 100644
--- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -358,15 +358,15 @@ TEST(LocateSymbol, All) {
   )cpp",
 
   R"cpp(// Forward class declaration
-class Foo;
-class [[Foo]] {};
+class $decl[[Foo]];
+class $def[[Foo]] {};
 F^oo* foo();
   )cpp",
 
   R"cpp(// Function declaration
-void foo();
+void $decl[[foo]]();
 void g() { f^oo(); }
-void [[foo]]() {}
+void $def[[foo]]() {}
   )cpp",
 
   R"cpp(



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


[PATCH] D76770: [CodeComplete] Don't replace the rest of line in #include completion.

2020-03-26 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

In D76770#1943077 , @hokein wrote:

> yes, it has a regression, but we don't change the behavior for the above 
> case. A regression case is like


Oops, right :-)

>> Or we can take the tradeoff here if it's better but it'd be good to 
>> understand why.
> 
> it was originally reported by one internal user.
> 
> I think the new behavior (with this patch)
> 
> - is more aligned with mental model of code completion (most people think 
> code completion just inserts the code text at the cursor position )
> - keeps the consistent behavior with other code completions, e.g. 
> `foo.^member()` if you select the `member` in code completion, another 
> `member` text will be inserted

OK, let's try it and see if people complain.
(There's also an argument for only replacing the text on the left of the 
cursor, that's more consistent with completion in general though I think less 
helpful too)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76770



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


[PATCH] D75951: Keep a list of already-included pragma-once files for mods.

2020-03-26 Thread Vy Nguyen via Phabricator via cfe-commits
oontvoo updated this revision to Diff 252768.
oontvoo added a comment.

Handle textual headers


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75951

Files:
  clang/include/clang/Lex/HeaderSearch.h
  clang/include/clang/Lex/Preprocessor.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/include/clang/Serialization/ASTReader.h
  clang/include/clang/Serialization/ASTWriter.h
  clang/lib/Lex/HeaderSearch.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp

Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1619,7 +1619,7 @@
   endian::Writer LE(Out, little);
   unsigned KeyLen = key.Filename.size() + 1 + 8 + 8;
   LE.write(KeyLen);
-  unsigned DataLen = 1 + 2 + 4 + 4;
+  unsigned DataLen = 1 + 2 + 4 + 4 + 4;
   for (auto ModInfo : Data.KnownHeaders)
 if (Writer.getLocalOrImportedSubmoduleID(ModInfo.getModule()))
   DataLen += 4;
@@ -1676,6 +1676,9 @@
   }
   LE.write(Offset);
 
+  // Write this file UID.
+  LE.write(Data.HFI.UID);
+
   auto EmitModule = [&](Module *M, ModuleMap::ModuleHeaderRole Role) {
 if (uint32_t ModID = Writer.getLocalOrImportedSubmoduleID(M)) {
   uint32_t Value = (ModID << 2) | (unsigned)Role;
@@ -1703,7 +1706,7 @@
 /// Write the header search block for the list of files that
 ///
 /// \param HS The header search structure to save.
-void ASTWriter::WriteHeaderSearch(const HeaderSearch &HS) {
+void ASTWriter::WriteHeaderSearch(HeaderSearch &HS) {
   HeaderFileInfoTrait GeneratorTrait(*this);
   llvm::OnDiskChainedHashTableGenerator Generator;
   SmallVector SavedStrings;
@@ -1781,8 +1784,7 @@
 // changed since it was loaded. Also skip it if it's for a modular header
 // from a different module; in that case, we rely on the module(s)
 // containing the header to provide this information.
-const HeaderFileInfo *HFI =
-HS.getExistingFileInfo(File, /*WantExternal*/!Chain);
+HeaderFileInfo *HFI = HS.getExistingFileInfo(File, /*WantExternal*/ !Chain);
 if (!HFI || (HFI->isModuleHeader && !HFI->isCompilingModuleHeader))
   continue;
 
@@ -1799,8 +1801,13 @@
 HeaderFileInfoTrait::key_type Key = {
   Filename, File->getSize(), getTimestampForOutput(File)
 };
+// Set the UID for this HFI so that its importers could use it
+// when serializing.
+HFI->UID = UID;
 HeaderFileInfoTrait::data_type Data = {
-  *HFI, HS.getModuleMap().findAllModulesForHeader(File), {}
+*HFI,
+HS.getModuleMap().findAllModulesForHeader(File),
+{},
 };
 Generator.insert(Key, Data, GeneratorTrait);
 ++NumHeaderSearchEntries;
@@ -2634,6 +2641,25 @@
   Stream.EmitRecord(SUBMODULE_IMPORTS, Record);
 }
 
+// Emit the imported header's UIDs.
+{
+  auto it = PP->Submodules.find(Mod);
+  if (it != PP->Submodules.end() && !it->second.IncludedFiles.empty()) {
+RecordData Record;
+for (unsigned UID : it->second.IncludedFiles) {
+  // Only save it if the header is actually import/pragma once.
+  // FIXME When we first see a header, it always goes into the mod's
+  // list of included, regardless of whether it was pragma-once or not.
+  // Maybe better to fix that earlier?
+  auto HFI = PP->getHeaderSearchInfo().FileInfo[UID];
+  if (HFI.isImport || HFI.isPragmaOnce) {
+Record.push_back(HFI.UID);
+  }
+}
+Stream.EmitRecord(SUBMODULE_IMPORTED_HEADERS, Record);
+  }
+}
+
 // Emit the exports.
 if (!Mod->Exports.empty()) {
   RecordData Record;
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -1,3 +1,4 @@
+
 //===- ASTReader.cpp - AST File Reader ===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
@@ -1898,6 +1899,9 @@
 HFI.Framework = HS->getUniqueFrameworkName(FrameworkName);
   }
 
+  // Read the file old UID
+  HFI.UID = endian::readNext(d);
+
   assert((End - d) % 4 == 0 &&
  "Wrong data length in HeaderFileInfo deserialization");
   while (d != End) {
@@ -5615,6 +5619,11 @@
   }
   break;
 
+case SUBMODULE_IMPORTED_HEADERS:
+  for (unsigned Idx = 0; Idx < Record.size(); ++Idx) {
+PendingImportedHeaders[&F].push_back(Record[Idx]);
+  }
+  break;
 case SUBMODULE_EXPORTS:
   for (unsigned Idx = 0; Idx + 1 < Record.size(); Idx += 2) {
 UnresolvedModuleRef Unresolved;
@@ -9271,6 +9280,37 @@
   for (auto *ND : PendingMergedDefinitionsToDeduplicate)
 getContext().ded

[PATCH] D76830: [Analyzer][MallocChecker] No warning for kfree of ZERO_SIZE_PTR.

2020-03-26 Thread Balázs Kéri via Phabricator via cfe-commits
balazske marked an inline comment as done.
balazske added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp:146
 
   return IntValue.getSExtValue();
 }

The function was changed to get the numeric value from the end of the macro in 
any case. This way it recognizes a `(void *)16` as 16 (but maybe `16+16` too as 
16).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76830



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


[PATCH] D75514: [Analyzer] Only add container note tags to the operations of the affected container

2020-03-26 Thread Balogh, Ádám via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1a27d63a8891: [Analyzer] Only add container note tags to the 
operations of the affected… (authored by baloghadamsoftware).
Herald added a subscriber: ASDenysPetrov.

Changed prior to commit:
  https://reviews.llvm.org/D75514?vs=250248&id=252770#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75514

Files:
  clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
  clang/lib/StaticAnalyzer/Checkers/DebugContainerModeling.cpp
  clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
  clang/test/Analysis/container-modeling.cpp

Index: clang/test/Analysis/container-modeling.cpp
===
--- clang/test/Analysis/container-modeling.cpp
+++ clang/test/Analysis/container-modeling.cpp
@@ -76,8 +76,7 @@
   clang_analyzer_denote(clang_analyzer_container_begin(V), "$V.begin()");
   clang_analyzer_denote(clang_analyzer_container_end(V), "$V.end()");
 
-  V.push_back(n); // expected-note{{Container 'V' extended to the back by 1 position}}
-  // expected-note@-1{{Container 'V' extended to the back by 1 position}}
+  V.push_back(n); // expected-note 2{{Container 'V' extended to the back by 1 position}}
 
   clang_analyzer_express(clang_analyzer_container_begin(V)); // expected-warning{{$V.begin()}}
  // expected-note@-1{{$V.begin()}}
@@ -99,7 +98,6 @@
 
   V.emplace_back(n); // expected-note 2{{Container 'V' extended to the back by 1 position}}
 
-
   clang_analyzer_express(clang_analyzer_container_begin(V)); // expected-warning{{$V.begin()}}
  // expected-note@-1{{$V.begin()}}
   clang_analyzer_express(clang_analyzer_container_end(V)); // expected-warning{{$V.end() + 1}}
@@ -217,10 +215,10 @@
 
   clang_analyzer_denote(clang_analyzer_container_begin(V1), "$V1.begin()");
 
-  V2.push_back(n); // expected-note{{Container 'V2' extended to the back by 1 position}} FIXME: This note should not appear since `V2` is not affected in the "bug"
+  V2.push_back(n); // no-note
 
   clang_analyzer_express(clang_analyzer_container_begin(V1)); // expected-warning{{$V1.begin()}}
- // expected-note@-1{{$V1.begin()}}
+  // expected-note@-1{{$V1.begin()}}
 }
 
 void push_back2(std::vector &V1, std::vector &V2, int n) {
@@ -232,15 +230,14 @@
   clang_analyzer_denote(clang_analyzer_container_begin(V1), "$V1.begin()");
   clang_analyzer_denote(clang_analyzer_container_begin(V2), "$V2.begin()");
 
-  V1.push_back(n); // expected-note 2{{Container 'V1' extended to the back by 1 position}}
-   // FIXME: This should appear only once since there is only
-   // one "bug" where `V1` is affected
+  V1.push_back(n); // expected-note{{Container 'V1' extended to the back by 1 position}}
+   // Only once!
 
   clang_analyzer_express(clang_analyzer_container_begin(V1)); // expected-warning{{$V1.begin()}}
- // expected-note@-1{{$V1.begin()}}
+  // expected-note@-1{{$V1.begin()}}
 
   clang_analyzer_express(clang_analyzer_container_begin(V2)); // expected-warning{{$V2.begin()}}
- // expected-note@-1{{$V2.begin()}}
+  // expected-note@-1{{$V2.begin()}}
 }
 
 /// Print Container Data as Part of the Program State
Index: clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
@@ -52,9 +52,12 @@
   typedef void (ExprInspectionChecker::*FnCheck)(const CallExpr *,
  CheckerContext &C) const;
 
-  ExplodedNode *reportBug(llvm::StringRef Msg, CheckerContext &C) const;
+  // Optional parameter `ExprVal` for expression value to be marked interesting.
+  ExplodedNode *reportBug(llvm::StringRef Msg, CheckerContext &C,
+  Optional ExprVal = None) const;
   ExplodedNode *reportBug(llvm::StringRef Msg, BugReporter &BR,
-  ExplodedNode *N) const;
+  ExplodedNode *N,
+  Optional ExprVal = None) const;
 
 public:
   bool evalCall(const CallEvent &Call, CheckerContext &C) const;
@@ -144,22 +147,28 @@
 }
 
 ExplodedNode *ExprInspectionChecker::reportBug(llvm::StringRef Msg,
-   CheckerContext &C) const {
+   

[PATCH] D76830: [Analyzer][MallocChecker] No warning for kfree of ZERO_SIZE_PTR.

2020-03-26 Thread Balázs Kéri via Phabricator via cfe-commits
balazske created this revision.
Herald added subscribers: cfe-commits, ASDenysPetrov, martong, Charusso, 
gamesh411, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, 
baloghadamsoftware, xazax.hun.
Herald added a reviewer: Szelethus.
Herald added a project: clang.
balazske added a comment.
balazske marked an inline comment as done.

FIXME: There is a test file "kmalloc-linux.c" but it seems to be non-maintained 
and buggy (has no //-verify// option so it passes always but produces multiple 
warnings).




Comment at: clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp:146
 
   return IntValue.getSExtValue();
 }

The function was changed to get the numeric value from the end of the macro in 
any case. This way it recognizes a `(void *)16` as 16 (but maybe `16+16` too as 
16).


The kernel kmalloc function may return a constant value ZERO_SIZE_PTR
if a zero-sized block is allocated. This special value is allowed to
be passed to kfree and should produce no warning.

This is a simple version but should be no problem. The macro is always
detected independent of if this is a kernel source code or any other
code. And it is recognized in any kind of free function, not only kfree.
(These functions are used already intermixed, at least in the tests.)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76830

Files:
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp
  clang/test/Analysis/kmalloc-linux-1.c

Index: clang/test/Analysis/kmalloc-linux-1.c
===
--- /dev/null
+++ clang/test/Analysis/kmalloc-linux-1.c
@@ -0,0 +1,27 @@
+// RUN: %clang_analyze_cc1 -triple x86_64-unknown-linux -analyzer-checker=unix.Malloc -verify %s
+
+#define ZERO_SIZE_PTR ((void *)16)
+
+void *__kmalloc(int size, int flags);
+
+// Linux kmalloc looks like this.
+// (simplified)
+void *kmalloc(int size, int flags) {
+  if (size == 0)
+return ZERO_SIZE_PTR;
+  return __kmalloc(size, flags);
+}
+
+// FIXME: malloc checker expects kfree with 2 arguments, is this correct?
+// (recent kernel source code contains a `kfree(const void *)`)
+void kfree(void *, int);
+
+void test_kmalloc_zero_sized_block_fixed_value_address() {
+  void *ptr = kmalloc(0, 0); // kmalloc returns a constant address
+  kfree(ptr, 0); // no warning about freeing a constant value
+}
+
+void test_kfree_constant_value() {
+  void *ptr = (void *)1;
+  kfree(ptr, 0); // expected-warning{{Argument to kfree() is a constant address (1)}}
+}
Index: clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp
===
--- clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp
+++ clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp
@@ -126,9 +126,6 @@
 if (!T.isOneOf(tok::l_paren, tok::r_paren))
   FilteredTokens.push_back(T);
 
-  if (FilteredTokens.size() > 2)
-return llvm::None;
-
   // Parse an integer at the end of the macro definition.
   const Token &T = FilteredTokens.back();
   if (!T.isLiteral())
@@ -140,11 +137,10 @@
 return llvm::None;
 
   // Parse an optional minus sign.
-  if (FilteredTokens.size() == 2) {
-if (FilteredTokens.front().is(tok::minus))
+  size_t Size = FilteredTokens.size();
+  if (Size >= 2) {
+if (FilteredTokens[Size - 2].is(tok::minus))
   IntValue = -IntValue;
-else
-  return llvm::None;
   }
 
   return IntValue.getSExtValue();
Index: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -58,6 +58,7 @@
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
@@ -389,6 +390,11 @@
   // TODO: Remove mutable by moving the initializtaion to the registry function.
   mutable Optional KernelZeroFlagVal;
 
+  /// Store value of "ZERO_SIZE_PTR" macro if found.
+  /// It requires two-level Optional: Was lazy initialization done yet, and if
+  /// yes was the value obtained or not.
+  mutable Optional> KernelZeroSizePtrValue;
+
   /// Process C++ operator new()'s allocation, which is the part of C++
   /// new-expression that goes before the constructor.
   void processNewAllocation(const CXXNewExpr *NE, CheckerContext &C,
@@ -1672,6 +1678,20 @@
   if (ArgVal.isUnknownOrUndef())
 return nullptr;
 
+  // If there is a macro called ZERO_SIZE_PTR, it could be a kernel source code
+  // and this value indicates a special va

[PATCH] D76830: [Analyzer][MallocChecker] No warning for kfree of ZERO_SIZE_PTR.

2020-03-26 Thread Balázs Kéri via Phabricator via cfe-commits
balazske added a comment.

FIXME: There is a test file "kmalloc-linux.c" but it seems to be non-maintained 
and buggy (has no //-verify// option so it passes always but produces multiple 
warnings).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76830



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


[PATCH] D76801: [AST] Print a> without extra spaces in C++11 or later.

2020-03-26 Thread Sam McCall via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG159a9f7e7630: [AST] Print a> without extra 
spaces in C++11 or later. (authored by sammccall).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76801

Files:
  clang-tools-extra/clangd/unittests/HoverTests.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
  clang/include/clang/AST/PrettyPrinter.h
  clang/lib/AST/TypePrinter.cpp
  clang/test/CXX/expr/expr.prim/expr.prim.req/type-requirement.cpp
  clang/test/CXX/temp/temp.arg/temp.arg.template/p3-0x.cpp
  clang/test/CodeGenCXX/debug-info-template-explicit-specialization.cpp
  clang/test/Misc/diag-aka-types.cpp
  clang/test/Modules/ExtDebugInfo.cpp
  clang/test/Modules/ModuleDebugInfo.cpp
  clang/test/OpenMP/distribute_parallel_for_private_messages.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_private_messages.cpp
  clang/test/OpenMP/distribute_simd_private_messages.cpp
  clang/test/OpenMP/for_private_messages.cpp
  clang/test/OpenMP/for_simd_private_messages.cpp
  clang/test/OpenMP/master_taskloop_private_messages.cpp
  clang/test/OpenMP/master_taskloop_simd_private_messages.cpp
  clang/test/OpenMP/parallel_for_private_messages.cpp
  clang/test/OpenMP/parallel_for_simd_private_messages.cpp
  clang/test/OpenMP/parallel_master_private_messages.cpp
  clang/test/OpenMP/parallel_master_taskloop_private_messages.cpp
  clang/test/OpenMP/parallel_master_taskloop_simd_private_messages.cpp
  clang/test/OpenMP/parallel_sections_private_messages.cpp
  clang/test/OpenMP/sections_private_messages.cpp
  clang/test/OpenMP/simd_private_messages.cpp
  clang/test/OpenMP/single_private_messages.cpp
  clang/test/OpenMP/target_firstprivate_messages.cpp
  clang/test/OpenMP/target_parallel_for_private_messages.cpp
  clang/test/OpenMP/target_parallel_for_simd_private_messages.cpp
  clang/test/OpenMP/target_private_messages.cpp
  clang/test/OpenMP/target_simd_private_messages.cpp
  clang/test/OpenMP/taskloop_private_messages.cpp
  clang/test/OpenMP/taskloop_simd_private_messages.cpp
  clang/test/SemaTemplate/instantiate-member-expr.cpp
  clang/unittests/AST/DeclPrinterTest.cpp

Index: clang/unittests/AST/DeclPrinterTest.cpp
===
--- clang/unittests/AST/DeclPrinterTest.cpp
+++ clang/unittests/AST/DeclPrinterTest.cpp
@@ -1158,8 +1158,8 @@
 "template struct X {};"
 "Z> A;",
 "A",
-"Z > A"));
-// Should be: with semicolon, without extra space in "> >"
+"Z> A"));
+// Should be: with semicolon
 }
 
 TEST(DeclPrinter, TestTemplateArgumentList5) {
Index: clang/test/SemaTemplate/instantiate-member-expr.cpp
===
--- clang/test/SemaTemplate/instantiate-member-expr.cpp
+++ clang/test/SemaTemplate/instantiate-member-expr.cpp
@@ -16,7 +16,7 @@
 
  template 
  void registerCheck(CHECKER *check) {
-   Checkers.push_back(S()); // expected-note {{in instantiation of member function 'vector >::push_back' requested here}}
+   Checkers.push_back(S()); // expected-note {{in instantiation of member function 'vector>::push_back' requested here}}
  }
 };
 
Index: clang/test/OpenMP/taskloop_simd_private_messages.cpp
===
--- clang/test/OpenMP/taskloop_simd_private_messages.cpp
+++ clang/test/OpenMP/taskloop_simd_private_messages.cpp
@@ -253,7 +253,7 @@
 si = k + 1;
 
   s6 = s6_0; // expected-note {{in instantiation of member function 'S6::operator=' requested here}}
-  s7 = s7_0; // expected-note {{in instantiation of member function 'S7 >::operator=' requested here}}
+  s7 = s7_0; // expected-note {{in instantiation of member function 'S7>::operator=' requested here}}
   return foomain(argc, argv); // expected-note {{in instantiation of function template specialization 'foomain' requested here}}
 }
 
Index: clang/test/OpenMP/taskloop_private_messages.cpp
===
--- clang/test/OpenMP/taskloop_private_messages.cpp
+++ clang/test/OpenMP/taskloop_private_messages.cpp
@@ -253,7 +253,7 @@
 si = k + 1;
 
   s6 = s6_0; // expected-note {{in instantiation of member function 'S6::operator=' requested here}}
-  s7 = s7_0; // expected-note {{in instantiation of member function 'S7 >::operator=' requested here}}
+  s7 = s7_0; // expected-note {{in instantiation of member function 'S7>::operator=' requested here}}
   return foomain(argc, argv); // expected-note {{in instantiation of function template specialization 'foomain' requested here}}
 }
 
Index: clang/test/OpenMP/target_simd_private_messages.cpp
===
--- clang/test/OpenMP/target_simd_private_messages.cpp
+++ clang/test/OpenMP/target_simd_private_messages.cpp
@@ -237,7 +237,7 @@
 m = k + 2;
 
   s6 = s6_0; // e

[PATCH] D73369: [clangd] Simplify "preferred" vs "definition" logic a bit in XRefs AST code.

2020-03-26 Thread Sam McCall via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6324912592a1: [clangd] Simplify "preferred" vs 
"definition" logic a bit in XRefs AST code. (authored by sammccall).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73369

Files:
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp


Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -358,15 +358,15 @@
   )cpp",
 
   R"cpp(// Forward class declaration
-class Foo;
-class [[Foo]] {};
+class $decl[[Foo]];
+class $def[[Foo]] {};
 F^oo* foo();
   )cpp",
 
   R"cpp(// Function declaration
-void foo();
+void $decl[[foo]]();
 void g() { f^oo(); }
-void [[foo]]() {}
+void $def[[foo]]() {}
   )cpp",
 
   R"cpp(
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -226,23 +226,21 @@
   llvm::DenseMap ResultIndex;
 
   auto AddResultDecl = [&](const NamedDecl *D) {
-const NamedDecl *Def = getDefinition(D);
-const NamedDecl *Preferred = Def ? Def : D;
-
-auto Loc = makeLocation(AST.getASTContext(), nameLocation(*Preferred, SM),
-MainFilePath);
+D = llvm::cast(D->getCanonicalDecl());
+auto Loc =
+makeLocation(AST.getASTContext(), nameLocation(*D, SM), MainFilePath);
 if (!Loc)
   return;
 
 Result.emplace_back();
-Result.back().Name = printName(AST.getASTContext(), *Preferred);
+Result.back().Name = printName(AST.getASTContext(), *D);
 Result.back().PreferredDeclaration = *Loc;
-// Preferred is always a definition if possible, so this check works.
-if (Def == Preferred)
-  Result.back().Definition = *Loc;
+if (const NamedDecl *Def = getDefinition(D))
+  Result.back().Definition = makeLocation(
+  AST.getASTContext(), nameLocation(*Def, SM), MainFilePath);
 
 // Record SymbolID for index lookup later.
-if (auto ID = getSymbolID(Preferred))
+if (auto ID = getSymbolID(D))
   ResultIndex[*ID] = Result.size() - 1;
   };
 


Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -358,15 +358,15 @@
   )cpp",
 
   R"cpp(// Forward class declaration
-class Foo;
-class [[Foo]] {};
+class $decl[[Foo]];
+class $def[[Foo]] {};
 F^oo* foo();
   )cpp",
 
   R"cpp(// Function declaration
-void foo();
+void $decl[[foo]]();
 void g() { f^oo(); }
-void [[foo]]() {}
+void $def[[foo]]() {}
   )cpp",
 
   R"cpp(
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -226,23 +226,21 @@
   llvm::DenseMap ResultIndex;
 
   auto AddResultDecl = [&](const NamedDecl *D) {
-const NamedDecl *Def = getDefinition(D);
-const NamedDecl *Preferred = Def ? Def : D;
-
-auto Loc = makeLocation(AST.getASTContext(), nameLocation(*Preferred, SM),
-MainFilePath);
+D = llvm::cast(D->getCanonicalDecl());
+auto Loc =
+makeLocation(AST.getASTContext(), nameLocation(*D, SM), MainFilePath);
 if (!Loc)
   return;
 
 Result.emplace_back();
-Result.back().Name = printName(AST.getASTContext(), *Preferred);
+Result.back().Name = printName(AST.getASTContext(), *D);
 Result.back().PreferredDeclaration = *Loc;
-// Preferred is always a definition if possible, so this check works.
-if (Def == Preferred)
-  Result.back().Definition = *Loc;
+if (const NamedDecl *Def = getDefinition(D))
+  Result.back().Definition = makeLocation(
+  AST.getASTContext(), nameLocation(*Def, SM), MainFilePath);
 
 // Record SymbolID for index lookup later.
-if (auto ID = getSymbolID(Preferred))
+if (auto ID = getSymbolID(D))
   ResultIndex[*ID] = Result.size() - 1;
   };
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 71ae267 - [PATCH] [ARM] ARMv8.6-a command-line + BFloat16 Asm Support

2020-03-26 Thread Ties Stuij via cfe-commits

Author: Ties Stuij
Date: 2020-03-26T09:17:20Z
New Revision: 71ae267d1f4117473eb00d9fd3391733b843ca3c

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

LOG: [PATCH] [ARM] ARMv8.6-a command-line + BFloat16 Asm Support

Summary:
This patch introduces command-line support for the Armv8.6-a architecture and 
assembly support for BFloat16. Details can be found
https://community.arm.com/developer/ip-products/processors/b/processors-ip-blog/posts/arm-architecture-developments-armv8-6-a

in addition to the GCC patch for the 8..6-a CLI:
https://gcc.gnu.org/legacy-ml/gcc-patches/2019-11/msg02647.html

In detail this patch

- march options for armv8.6-a
- BFloat16 assembly

This is part of a patch series, starting with command-line and Bfloat16
assembly support. The subsequent patches will upstream intrinsics
support for BFloat16, followed by Matrix Multiplication and the
remaining Virtualization features of the armv8.6-a architecture.

Based on work by:
- labrinea
- MarkMurrayARM
- Luke Cheeseman
- Javed Asbar
- Mikhail Maltsev
- Luke Geeson

Reviewers: SjoerdMeijer, craig.topper, rjmccall, jfb, LukeGeeson

Reviewed By: SjoerdMeijer

Subscribers: stuij, kristof.beyls, hiraditya, dexonsmith, danielkiss, 
cfe-commits, llvm-commits

Tags: #clang, #llvm

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

Added: 
llvm/test/MC/AArch64/SVE/bfcvt-diagnostics.s
llvm/test/MC/AArch64/SVE/bfcvt.s
llvm/test/MC/AArch64/SVE/bfcvtnt-diagnostics.s
llvm/test/MC/AArch64/SVE/bfcvtnt.s
llvm/test/MC/AArch64/SVE/bfdot-diagnostics.s
llvm/test/MC/AArch64/SVE/bfdot.s
llvm/test/MC/AArch64/SVE/bfmlal-diagnostics.s
llvm/test/MC/AArch64/SVE/bfmlal.s
llvm/test/MC/AArch64/SVE/bfmmla-diagnostics.s
llvm/test/MC/AArch64/SVE/bfmmla.s
llvm/test/MC/AArch64/armv8.6a-bf16.s
llvm/test/MC/ARM/bfloat16-a32-errors.s
llvm/test/MC/ARM/bfloat16-a32-errors2.s
llvm/test/MC/ARM/bfloat16-a32.s
llvm/test/MC/ARM/bfloat16-t32-errors.s
llvm/test/MC/ARM/bfloat16-t32.s
llvm/test/MC/Disassembler/AArch64/armv8.6a-bf16.txt
llvm/test/MC/Disassembler/ARM/bfloat16-a32_1.txt
llvm/test/MC/Disassembler/ARM/bfloat16-a32_2.txt
llvm/test/MC/Disassembler/ARM/bfloat16-t32.txt
llvm/test/MC/Disassembler/ARM/bfloat16-t32_errors.txt

Modified: 
clang/lib/Basic/Targets/AArch64.cpp
clang/lib/Basic/Targets/AArch64.h
clang/lib/Basic/Targets/ARM.cpp
clang/test/Driver/aarch64-cpus.c
clang/test/Driver/arm-cortex-cpus.c
clang/test/Preprocessor/arm-target-features.c
llvm/include/llvm/ADT/Triple.h
llvm/include/llvm/Support/AArch64TargetParser.def
llvm/include/llvm/Support/AArch64TargetParser.h
llvm/include/llvm/Support/ARMTargetParser.def
llvm/include/llvm/Support/ARMTargetParser.h
llvm/lib/Support/AArch64TargetParser.cpp
llvm/lib/Support/ARMTargetParser.cpp
llvm/lib/Support/Triple.cpp
llvm/lib/Target/AArch64/AArch64.td
llvm/lib/Target/AArch64/AArch64InstrFormats.td
llvm/lib/Target/AArch64/AArch64InstrInfo.td
llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td
llvm/lib/Target/AArch64/AArch64Subtarget.h
llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
llvm/lib/Target/AArch64/SVEInstrFormats.td
llvm/lib/Target/ARM/ARM.td
llvm/lib/Target/ARM/ARMInstrNEON.td
llvm/lib/Target/ARM/ARMInstrVFP.td
llvm/lib/Target/ARM/ARMPredicates.td
llvm/lib/Target/ARM/ARMSubtarget.h
llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
llvm/lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp
llvm/unittests/Support/TargetParserTest.cpp

Removed: 




diff  --git a/clang/lib/Basic/Targets/AArch64.cpp 
b/clang/lib/Basic/Targets/AArch64.cpp
index 74bb6900b19e..e7bfff504a7c 100644
--- a/clang/lib/Basic/Targets/AArch64.cpp
+++ b/clang/lib/Basic/Targets/AArch64.cpp
@@ -151,6 +151,7 @@ void AArch64TargetInfo::fillValidCPUList(
 
 void AArch64TargetInfo::getTargetDefinesARMV81A(const LangOptions &Opts,
 MacroBuilder &Builder) const {
+  // FIXME: Armv8.1 makes __ARM_FEATURE_CRC32 mandatory. Handle it here.
   Builder.defineMacro("__ARM_FEATURE_QRDMX", "1");
 }
 
@@ -171,17 +172,26 @@ void AArch64TargetInfo::getTargetDefinesARMV83A(const 
LangOptions &Opts,
 void AArch64TargetInfo::getTargetDefinesARMV84A(const LangOptions &Opts,
 MacroBuilder &Builder) const {
   // Also include the Armv8.3 defines
-  // FIXME: Armv8.4 makes some extensions mandatory. Handle them here.
+  // FIXME: Armv8.4 makes __ARM_FEATURE_ATOMICS, defined in GCC, mandatory.
+  // Add and handle it here.
   getTargetDefinesARMV83A(Opts, Builder);
 }
 
 void AArch64TargetInfo::getTargetDefinesARMV85A(const LangOptions &Opts,
  

[clang] 4bd1d55 - [AST] Fix thinlto testcase missed in 159a9f7e76307734bcdcae3357640e42e0733194

2020-03-26 Thread Sam McCall via cfe-commits

Author: Sam McCall
Date: 2020-03-26T10:28:54+01:00
New Revision: 4bd1d55884aaeb582aa8f313e45823fe0f60b32d

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

LOG: [AST] Fix thinlto testcase missed in 
159a9f7e76307734bcdcae3357640e42e0733194

Added: 


Modified: 
clang/test/CodeGen/thinlto-distributed-newpm.ll

Removed: 




diff  --git a/clang/test/CodeGen/thinlto-distributed-newpm.ll 
b/clang/test/CodeGen/thinlto-distributed-newpm.ll
index bac9c4322045..23ba917de7a3 100644
--- a/clang/test/CodeGen/thinlto-distributed-newpm.ll
+++ b/clang/test/CodeGen/thinlto-distributed-newpm.ll
@@ -34,7 +34,7 @@
 ; CHECK-O: Running analysis: OptimizationRemarkEmitterAnalysis on main
 ; CHECK-O: Running analysis: PassInstrumentationAnalysis on main
 ; CHECK-O: Running pass: InferFunctionAttrsPass
-; CHECK-O: Running pass: 
ModuleToFunctionPassAdaptor<{{.*}}PassManager<{{.*}}Function> >
+; CHECK-O: Running pass: 
ModuleToFunctionPassAdaptor<{{.*}}PassManager<{{.*}}Function>>
 ; CHECK-O: Starting {{.*}}Function pass manager run.
 ; CHECK-O: Running pass: SimplifyCFGPass on main
 ; CHECK-O: Running analysis: TargetIRAnalysis on main
@@ -57,7 +57,7 @@
 ; CHECK-O: Running analysis: PassInstrumentationAnalysis on main
 ; CHECK-O: Running analysis: AssumptionAnalysis on main
 ; CHECK-O: Running pass: DeadArgumentEliminationPass
-; CHECK-O: Running pass: 
ModuleToFunctionPassAdaptor<{{.*}}PassManager<{{.*}}Function> >
+; CHECK-O: Running pass: 
ModuleToFunctionPassAdaptor<{{.*}}PassManager<{{.*}}Function>>
 ; CHECK-O: Starting {{.*}}Function pass manager run.
 ; CHECK-O: Running pass: InstCombinePass on main
 ; CHECK-O: Running analysis: TargetLibraryAnalysis on main
@@ -179,7 +179,7 @@
 ; CHECK-O: Running pass: ReversePostOrderFunctionAttrsPass
 ; CHECK-O: Running analysis: CallGraphAnalysis
 ; CHECK-O: Running pass: RequireAnalysisPass<{{.*}}GlobalsAA
-; CHECK-O: Running pass: 
ModuleToFunctionPassAdaptor<{{.*}}PassManager<{{.*}}Function> >
+; CHECK-O: Running pass: 
ModuleToFunctionPassAdaptor<{{.*}}PassManager<{{.*}}Function>>
 ; CHECK-O: Starting {{.*}}Function pass manager run.
 ; CHECK-O: Running pass: Float2IntPass on main
 ; CHECK-O: Running pass: LowerConstantIntrinsicsPass on main



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


[PATCH] D76830: [Analyzer][MallocChecker] No warning for kfree of ZERO_SIZE_PTR.

2020-03-26 Thread Gabor Marton via Phabricator via cfe-commits
martong added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:396
+  /// yes was the value obtained or not.
+  mutable Optional> KernelZeroSizePtrValue;
+

Which one is referred to the lazy initialization? The inner or the outer?
These questions actually made me to come up with a more explanatory construct 
here:
Could we do something like this?
```
using LazyInitialized = Optional;
mutable Optional KernelZeroSizePtrValue; // Or 
Lazy>
```



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1687
+  if (ArgValKnown) {
+if (!KernelZeroSizePtrValue)
+  KernelZeroSizePtrValue =

This is a bit confusing for me. Perhaps alternatively we could have a free 
function `isInitialized(KernelZero...)` instead. Or maybe having a separate 
bool variable to indicate whether it was initialized could be cleaner?



Comment at: clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp:146
 
   return IntValue.getSExtValue();
 }

balazske wrote:
> The function was changed to get the numeric value from the end of the macro 
> in any case. This way it recognizes a `(void *)16` as 16 (but maybe `16+16` 
> too as 16).
Ok.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76830



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


[PATCH] D76062: [PATCH] [ARM] ARMv8.6-a command-line + BFloat16 Asm Support

2020-03-26 Thread Ties Stuij via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG71ae267d1f41: [PATCH] [ARM] ARMv8.6-a command-line + 
BFloat16 Asm Support (authored by stuij).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76062

Files:
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/AArch64.h
  clang/lib/Basic/Targets/ARM.cpp
  clang/test/Driver/aarch64-cpus.c
  clang/test/Driver/arm-cortex-cpus.c
  clang/test/Preprocessor/arm-target-features.c
  llvm/include/llvm/ADT/Triple.h
  llvm/include/llvm/Support/AArch64TargetParser.def
  llvm/include/llvm/Support/AArch64TargetParser.h
  llvm/include/llvm/Support/ARMTargetParser.def
  llvm/include/llvm/Support/ARMTargetParser.h
  llvm/lib/Support/AArch64TargetParser.cpp
  llvm/lib/Support/ARMTargetParser.cpp
  llvm/lib/Support/Triple.cpp
  llvm/lib/Target/AArch64/AArch64.td
  llvm/lib/Target/AArch64/AArch64InstrFormats.td
  llvm/lib/Target/AArch64/AArch64InstrInfo.td
  llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td
  llvm/lib/Target/AArch64/AArch64Subtarget.h
  llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
  llvm/lib/Target/AArch64/SVEInstrFormats.td
  llvm/lib/Target/ARM/ARM.td
  llvm/lib/Target/ARM/ARMInstrNEON.td
  llvm/lib/Target/ARM/ARMInstrVFP.td
  llvm/lib/Target/ARM/ARMPredicates.td
  llvm/lib/Target/ARM/ARMSubtarget.h
  llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
  llvm/lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp
  llvm/test/MC/AArch64/SVE/bfcvt-diagnostics.s
  llvm/test/MC/AArch64/SVE/bfcvt.s
  llvm/test/MC/AArch64/SVE/bfcvtnt-diagnostics.s
  llvm/test/MC/AArch64/SVE/bfcvtnt.s
  llvm/test/MC/AArch64/SVE/bfdot-diagnostics.s
  llvm/test/MC/AArch64/SVE/bfdot.s
  llvm/test/MC/AArch64/SVE/bfmlal-diagnostics.s
  llvm/test/MC/AArch64/SVE/bfmlal.s
  llvm/test/MC/AArch64/SVE/bfmmla-diagnostics.s
  llvm/test/MC/AArch64/SVE/bfmmla.s
  llvm/test/MC/AArch64/armv8.6a-bf16.s
  llvm/test/MC/ARM/bfloat16-a32-errors.s
  llvm/test/MC/ARM/bfloat16-a32-errors2.s
  llvm/test/MC/ARM/bfloat16-a32.s
  llvm/test/MC/ARM/bfloat16-t32-errors.s
  llvm/test/MC/ARM/bfloat16-t32.s
  llvm/test/MC/Disassembler/AArch64/armv8.6a-bf16.txt
  llvm/test/MC/Disassembler/ARM/bfloat16-a32_1.txt
  llvm/test/MC/Disassembler/ARM/bfloat16-a32_2.txt
  llvm/test/MC/Disassembler/ARM/bfloat16-t32.txt
  llvm/test/MC/Disassembler/ARM/bfloat16-t32_errors.txt
  llvm/unittests/Support/TargetParserTest.cpp

Index: llvm/unittests/Support/TargetParserTest.cpp
===
--- llvm/unittests/Support/TargetParserTest.cpp
+++ llvm/unittests/Support/TargetParserTest.cpp
@@ -26,9 +26,9 @@
 "armv7e-m","armv7em",  "armv8-a", "armv8","armv8a",
 "armv8l",  "armv8.1-a","armv8.1a","armv8.2-a","armv8.2a",
 "armv8.3-a",   "armv8.3a", "armv8.4-a",   "armv8.4a", "armv8.5-a",
-"armv8.5a", "armv8-r", "armv8r",  "armv8-m.base", "armv8m.base",
-"armv8-m.main", "armv8m.main", "iwmmxt",  "iwmmxt2",  "xscale",
-"armv8.1-m.main",
+"armv8.5a", "armv8.6-a",   "armv8.6a", "armv8-r", "armv8r",
+"armv8-m.base", "armv8m.base", "armv8-m.main", "armv8m.main", "iwmmxt",
+"iwmmxt2",  "xscale",  "armv8.1-m.main",
 };
 
 bool testARMCPU(StringRef CPUName, StringRef ExpectedArch,
@@ -411,6 +411,9 @@
   testARMArch("armv8.5-a", "generic", "v8.5a",
   ARMBuildAttrs::CPUArch::v8_A));
   EXPECT_TRUE(
+  testARMArch("armv8.6-a", "generic", "v8.6a",
+  ARMBuildAttrs::CPUArch::v8_A));
+  EXPECT_TRUE(
   testARMArch("armv8-r", "cortex-r52", "v8r",
   ARMBuildAttrs::CPUArch::v8_R));
   EXPECT_TRUE(
@@ -678,7 +681,7 @@
   "v7",   "v7a","v7ve",  "v7hl",   "v7l",   "v7-r",   "v7r",   "v7-m",
   "v7m",  "v7k","v7s",   "v7e-m",  "v7em",  "v8-a",   "v8","v8a",
   "v8l",  "v8.1-a", "v8.1a", "v8.2-a", "v8.2a", "v8.3-a", "v8.3a", "v8.4-a",
-  "v8.4a", "v8.5-a","v8.5a", "v8-r",   "v8m.base", "v8m.main", "v8.1m.main"
+  "v8.4a", "v8.5-a","v8.5a", "v8.6-a", "v8.6a", "v8-r",   "v8m.base", "v8m.main", "v8.1m.main"
   };
 
   for (unsigned i = 0; i < array_lengthof(Arch); i++) {
@@ -743,6 +746,7 @@
 case ARM::ArchKind::ARMV8_3A:
 case ARM::ArchKind::ARMV8_4A:
 case ARM::ArchKind::ARMV8_5A:
+case ARM::ArchKind::ARMV8_6A:
   EXPECT_EQ(ARM::ProfileKind::A, ARM::parseArchProfile(ARMArch[i]));
   break;
 default:
@@ -1008,6 +1012,8 @@
   ARMBuildAttrs::CPUArch::v8_A));
   EXPECT_TRUE(testAArch64Arch("armv8.5-a", "generic", "v8.5a",
   ARMBuildAttrs::CPUArch::v8_A));
+  EXPECT_TRUE(testAArch64Arch("armv8.6-a", "generic", "v8.6a",
+  ARMBuildAttrs::CPUArch::v8_A));
 }
 
 bool testAArch64Extension(StringRef CPUName, AArch64::ArchKind AK,
Index: llvm/test/MC/

[PATCH] D69585: PerformPendingInstatiations() already in the PCH

2020-03-26 Thread Luboš Luňák via Phabricator via cfe-commits
llunak added a comment.

In D69585#1942431 , @rsmith wrote:

> This needs to be done behind a flag. It's an explicit design goal that 
> compilation behavior using a PCH or precompiled preamble behaves identically 
> to compilation not using a PCH / precompiled preamble. The behavior of this 
> patch is also non-conforming: we're only allowed to perform function template 
> instantiations at their points of instantiation (essentially, at points of 
> use + at the end of the translation unit).


How is that different from -fmodules? Given that AFAICT this makes PCHs work 
the same way as modules, this should mean -fmodules also both fails the 
identical behaviour goal and is non-conforming.

And to make sure I understand correctly, by behaving identically you mean that 
all the compiler output should be identical without even benign differences?


Repository:
  rC Clang

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

https://reviews.llvm.org/D69585



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


[PATCH] D76830: [Analyzer][MallocChecker] No warning for kfree of ZERO_SIZE_PTR.

2020-03-26 Thread Gabor Marton via Phabricator via cfe-commits
martong added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1687
+  if (ArgValKnown) {
+if (!KernelZeroSizePtrValue)
+  KernelZeroSizePtrValue =

martong wrote:
> This is a bit confusing for me. Perhaps alternatively we could have a free 
> function `isInitialized(KernelZero...)` instead. Or maybe having a separate 
> bool variable to indicate whether it was initialized could be cleaner?
Another idea: Adding a helper struct to contain the bool `initialized`? E.g. 
(draft):
```
struct LazyOptional {
  bool initialized = false;
  Opt value;
  Opt& get();
  void set(const Opt&);
};
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76830



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


[PATCH] D76831: [AST] Preserve the DeclRefExpr when it refers to an invalid decl.

2020-03-26 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision.
hokein added reviewers: sammccall, adamcz.
Herald added a project: clang.

Previously, clang refused to build the AST nodes for them.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76831

Files:
  clang/lib/AST/ComputeDependence.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/AST/ast-dump-expr-errors.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.spec.auto/p6.cpp
  clang/test/CXX/drs/dr3xx.cpp
  clang/test/CodeCompletion/error-covery.cpp
  clang/test/Modules/submodules-merge-defs.cpp
  clang/test/OpenMP/openmp_check.cpp
  clang/test/SemaCXX/block-call.cpp
  clang/test/SemaCXX/conversion-function.cpp
  clang/test/SemaCXX/cxx11-crashes.cpp
  clang/test/SemaCXX/cxx2a-explicit-bool.cpp
  clang/test/SemaCXX/for-range-examples.cpp

Index: clang/test/SemaCXX/for-range-examples.cpp
===
--- clang/test/SemaCXX/for-range-examples.cpp
+++ clang/test/SemaCXX/for-range-examples.cpp
@@ -187,9 +187,9 @@
   void f() {
 for (auto x : undeclared_identifier) // expected-error {{undeclared identifier}}
   for (auto y : x->foo)
-y->bar();
+y->bar(); // expected-error {{member reference type 'auto' is not a pointer}}
 for (auto x : 123) // expected-error {{no viable 'begin'}}
-  x->foo();
+  x->foo(); // expected-error {{member reference type 'auto' is not a pointer}}
   }
 }
 
Index: clang/test/SemaCXX/cxx2a-explicit-bool.cpp
===
--- clang/test/SemaCXX/cxx2a-explicit-bool.cpp
+++ clang/test/SemaCXX/cxx2a-explicit-bool.cpp
@@ -124,7 +124,7 @@
 A && a6{ 0};
 A a7 = { 0}; // expected-error {{chosen constructor is explicit in copy-initialization}}
 
-a0 = 0;
+a0 = 0; // expected-error {{no viable overloaded '='}}
 a1 = { 0}; // expected-error {{no viable overloaded '='}}
 a2 = A( 0);
 a3 = A{ 0};
Index: clang/test/SemaCXX/cxx11-crashes.cpp
===
--- clang/test/SemaCXX/cxx11-crashes.cpp
+++ clang/test/SemaCXX/cxx11-crashes.cpp
@@ -70,7 +70,7 @@
 for (auto x : s) {
   // We used to attempt to evaluate the initializer of this variable,
   // and crash because it has an undeduced type.
-  const int &n(x);
+  const int &n(x); // expected-error {{reference to type 'const int' could not bind}}
 }
   }
 }
Index: clang/test/SemaCXX/conversion-function.cpp
===
--- clang/test/SemaCXX/conversion-function.cpp
+++ clang/test/SemaCXX/conversion-function.cpp
@@ -440,7 +440,7 @@
 #endif
   } a;
   A::S s = a; // expected-error {{no viable conversion from 'struct A' to 'A::S'}}
-  A::E e = a;
+  A::E e = a; // expected-note {{'e' declared here}}
   bool k1 = e == A::e; // expected-error {{no member named 'e'}}
   bool k2 = e.n == 0;
 }
Index: clang/test/SemaCXX/block-call.cpp
===
--- clang/test/SemaCXX/block-call.cpp
+++ clang/test/SemaCXX/block-call.cpp
@@ -33,7 +33,7 @@
   int (^IPCC6) (int, char (^CArg) (float))  = IPCC4; // expected-error {{cannot initialize a variable of type 'int (^)(int, char (^)(float))' with an lvalue of type}}
 
   IPCC2 = 0;
-  IPCC2 = 1; 
+  IPCC2 = 1; // expected-error {{invalid block pointer conversion assigning}} 
   int (^x)() = 0;
   int (^y)() = 3;   // expected-error {{cannot initialize a variable of type 'int (^)()' with an rvalue of type 'int'}}
   int a = 1;
Index: clang/test/OpenMP/openmp_check.cpp
===
--- clang/test/OpenMP/openmp_check.cpp
+++ clang/test/OpenMP/openmp_check.cpp
@@ -28,5 +28,8 @@
 }
   };
   F();
+#if __cplusplus <= 199711L
+  // expected-error@-2 {{called object type 'auto' is not a function}}
+#endif
   return a;
 }
Index: clang/test/Modules/submodules-merge-defs.cpp
===
--- clang/test/Modules/submodules-merge-defs.cpp
+++ clang/test/Modules/submodules-merge-defs.cpp
@@ -31,7 +31,9 @@
 // expected-note@defs.h:4 +{{here}}
 // expected-note@defs.h:17 +{{here}}
 void pre_bfi(B b) { // expected-error +{{must be imported}}
-  b.f();
+  b.f(); // expected-error {{member reference base type}} \
+ expected-error {{expected '(' for function-style}} \
+ expected-error {{expected expression}}
 }
 
 C_Base<1> pre_cb1; // expected-error +{{must be imported}}
Index: clang/test/CodeCompletion/error-covery.cpp
===
--- /dev/null
+++ clang/test/CodeCompletion/error-covery.cpp
@@ -0,0 +1,7 @@
+struct Foo { Foo(int); int abc; };
+void invalidDeclRefExpr() {
+  Foo foo;
+  foo.;
+  // RUN: not %clang_cc1 -fsyntax-only -code-completion-at=%s:5:7 %s -o - | FileCheck %s
+  // CHECK: COMPLETION: abc
+}
Index: clang/test/CXX/drs/dr3xx.cpp
==

[clang-tools-extra] 297a9da - [CodeComplete] Don't replace the rest of line in #include completion.

2020-03-26 Thread Haojian Wu via cfe-commits

Author: Haojian Wu
Date: 2020-03-26T11:03:04+01:00
New Revision: 297a9dac43f31cdbc811de5ec63ad20812433f98

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

LOG: [CodeComplete] Don't replace the rest of line in #include completion.

Summary:
The previous behavior was aggressive,
  #include "abc/f^/abc.h"
foo/  -> candidate
"f/abc.h" is replaced with "foo/", this patch will preserve the "abc.h".

Reviewers: sammccall

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

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
clang/lib/Lex/Lexer.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp 
b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
index f5c90a4677cb..c691c6d9e61a 100644
--- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -1860,6 +1860,7 @@ TEST(CompletionTest, RenderWithFixItNonMerged) {
 TEST(CompletionTest, CompletionTokenRange) {
   MockFSProvider FS;
   MockCompilationDatabase CDB;
+  FS.Files["foo/abc/foo.h"] = "";
   ClangdServer Server(CDB, FS, ClangdServer::optsForTest());
 
   constexpr const char *TestCodes[] = {
@@ -1882,7 +1883,14 @@ TEST(CompletionTest, CompletionTokenRange) {
   Auxilary x;
   x.[[]]^;
 }
-  )cpp"};
+  )cpp",
+  R"cpp(
+#include "foo/[[a^/]]foo.h"
+  )cpp",
+  R"cpp(
+#include "foo/abc/[[fo^o.h"]]
+  )cpp",
+  };
   for (const auto &Text : TestCodes) {
 Annotations TestCode(Text);
 auto Results = completions(Server, TestCode.code(), TestCode.point());

diff  --git a/clang/lib/Lex/Lexer.cpp b/clang/lib/Lex/Lexer.cpp
index a51745697b11..f0d2d4332b37 100644
--- a/clang/lib/Lex/Lexer.cpp
+++ b/clang/lib/Lex/Lexer.cpp
@@ -29,6 +29,7 @@
 #include "clang/Basic/TokenKinds.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/StringRef.h"
@@ -2092,7 +2093,8 @@ void Lexer::codeCompleteIncludedFile(const char 
*PathStart,
  bool IsAngled) {
   // Completion only applies to the filename, after the last slash.
   StringRef PartialPath(PathStart, CompletionPoint - PathStart);
-  auto Slash = PartialPath.find_last_of(LangOpts.MSVCCompat ? "/\\" : "/");
+  llvm::StringRef SlashChars = LangOpts.MSVCCompat ? "/\\" : "/";
+  auto Slash = PartialPath.find_last_of(SlashChars);
   StringRef Dir =
   (Slash == StringRef::npos) ? "" : PartialPath.take_front(Slash);
   const char *StartOfFilename =
@@ -2100,7 +2102,8 @@ void Lexer::codeCompleteIncludedFile(const char 
*PathStart,
   // Code completion filter range is the filename only, up to completion point.
   PP->setCodeCompletionIdentifierInfo(&PP->getIdentifierTable().get(
   StringRef(StartOfFilename, CompletionPoint - StartOfFilename)));
-  // We should replace the characters up to the closing quote, if any.
+  // We should replace the characters up to the closing quote or closest slash,
+  // if any.
   while (CompletionPoint < BufferEnd) {
 char Next = *(CompletionPoint + 1);
 if (Next == 0 || Next == '\r' || Next == '\n')
@@ -2108,7 +2111,10 @@ void Lexer::codeCompleteIncludedFile(const char 
*PathStart,
 ++CompletionPoint;
 if (Next == (IsAngled ? '>' : '"'))
   break;
+if (llvm::is_contained(SlashChars, Next))
+  break;
   }
+
   PP->setCodeCompletionTokenRange(
   FileLoc.getLocWithOffset(StartOfFilename - BufferStart),
   FileLoc.getLocWithOffset(CompletionPoint - BufferStart));



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


[clang-tools-extra] 7055cd4 - Remove extra ';', NFC

2020-03-26 Thread Karl-Johan Karlsson via cfe-commits

Author: Karl-Johan Karlsson
Date: 2020-03-26T11:23:59+01:00
New Revision: 7055cd42b5ffc88757eb3d12192047abe63a3714

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

LOG: Remove extra ';', NFC

Fixed gcc -Wpedantic warning about extra ';'

Added: 


Modified: 
clang-tools-extra/clangd/Hover.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/Hover.cpp 
b/clang-tools-extra/clangd/Hover.cpp
index 1d41f0a3e046..63fe6b1d756c 100644
--- a/clang-tools-extra/clangd/Hover.cpp
+++ b/clang-tools-extra/clangd/Hover.cpp
@@ -525,13 +525,13 @@ bool isParagraphLineBreak(llvm::StringRef Str, size_t 
LineBreakIndex) {
   return Str.substr(LineBreakIndex + 1)
   .drop_while([](auto C) { return C == ' ' || C == '\t'; })
   .startswith("\n");
-};
+}
 
 bool isPunctuationLineBreak(llvm::StringRef Str, size_t LineBreakIndex) {
   constexpr llvm::StringLiteral Punctuation = R"txt(.:,;!?)txt";
 
   return LineBreakIndex > 0 && Punctuation.contains(Str[LineBreakIndex - 1]);
-};
+}
 
 bool isFollowedByHardLineBreakIndicator(llvm::StringRef Str,
 size_t LineBreakIndex) {
@@ -556,7 +556,7 @@ bool isFollowedByHardLineBreakIndicator(llvm::StringRef Str,
Str[NextNonSpaceCharIndex + 1] == ')');
 
   return FollowedBySingleCharIndicator || FollowedByNumberedListIndicator;
-};
+}
 
 bool isHardLineBreak(llvm::StringRef Str, size_t LineBreakIndex) {
   return isPunctuationLineBreak(Str, LineBreakIndex) ||



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


[clang] a945037 - Tools emit the bug report URL on crash

2020-03-26 Thread via cfe-commits

Author: gbreynoo
Date: 2020-03-26T10:26:59Z
New Revision: a945037e8fd0c30e250a62211469eea6765a36ae

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

LOG: Tools emit the bug report URL on crash

When Clang crashes a useful message is output:

"PLEASE submit a bug report to https://bugs.llvm.org/ and include the
crash backtrace, preprocessed source, and associated run script."

A similar message is now output for all tools.

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

Added: 


Modified: 
clang/lib/Driver/Driver.cpp
clang/tools/driver/driver.cpp
llvm/include/llvm/Support/PrettyStackTrace.h
llvm/lib/Support/PrettyStackTrace.cpp

Removed: 




diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 37761893f137..e3b7793ca266 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -1267,10 +1267,6 @@ void Driver::generateCompilationDiagnostics(
   // Print the version of the compiler.
   PrintVersion(C, llvm::errs());
 
-  Diag(clang::diag::note_drv_command_failed_diag_msg)
-  << "PLEASE submit a bug report to " BUG_REPORT_URL " and include the "
- "crash backtrace, preprocessed source, and associated run script.";
-
   // Suppress driver output and emit preprocessor output to temp file.
   Mode = CPPMode;
   CCGenDiagnostics = true;

diff  --git a/clang/tools/driver/driver.cpp b/clang/tools/driver/driver.cpp
index c59752178b66..f037a821f9d7 100644
--- a/clang/tools/driver/driver.cpp
+++ b/clang/tools/driver/driver.cpp
@@ -38,6 +38,7 @@
 #include "llvm/Support/Host.h"
 #include "llvm/Support/InitLLVM.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/Process.h"
 #include "llvm/Support/Program.h"
 #include "llvm/Support/Regex.h"
@@ -342,6 +343,9 @@ static int ExecuteCC1Tool(SmallVectorImpl 
&ArgV) {
 int main(int argc_, const char **argv_) {
   noteBottomOfStack();
   llvm::InitLLVM X(argc_, argv_);
+  llvm::setBugReportMsg("PLEASE submit a bug report to " BUG_REPORT_URL
+" and include the crash backtrace, preprocessed "
+"source, and associated run script.\n");
   SmallVector argv(argv_, argv_ + argc_);
 
   if (llvm::sys::Process::FixupStandardFileDescriptors())

diff  --git a/llvm/include/llvm/Support/PrettyStackTrace.h 
b/llvm/include/llvm/Support/PrettyStackTrace.h
index 6eb070b2297e..32975f3c5a35 100644
--- a/llvm/include/llvm/Support/PrettyStackTrace.h
+++ b/llvm/include/llvm/Support/PrettyStackTrace.h
@@ -37,6 +37,10 @@ namespace llvm {
   /// \see PrettyStackTraceEntry
   void EnablePrettyStackTraceOnSigInfoForThisThread(bool ShouldEnable = true);
 
+  /// Replaces the generic bug report message that is output upon
+  /// a crash.
+  void setBugReportMsg(const char *Msg);
+
   /// PrettyStackTraceEntry - This class is used to represent a frame of the
   /// "pretty" stack trace that is dumped when a program crashes. You can 
define
   /// subclasses of this and declare them on the program stack: when they are

diff  --git a/llvm/lib/Support/PrettyStackTrace.cpp 
b/llvm/lib/Support/PrettyStackTrace.cpp
index 30a12f65966a..e1b834fe1347 100644
--- a/llvm/lib/Support/PrettyStackTrace.cpp
+++ b/llvm/lib/Support/PrettyStackTrace.cpp
@@ -142,9 +142,17 @@ using CrashHandlerStringStorage =
 static CrashHandlerStringStorage crashHandlerStringStorage;
 #endif
 
+static const char *BugReportMsg =
+"PLEASE submit a bug report to " BUG_REPORT_URL
+" and include the crash backtrace.\n";
+
+void llvm::setBugReportMsg(const char *Msg) { BugReportMsg = Msg; }
+
 /// This callback is run if a fatal signal is delivered to the process, it
 /// prints the pretty stack trace.
 static void CrashHandler(void *) {
+  errs() << BugReportMsg ;
+
 #ifndef __APPLE__
   // On non-apple systems, just emit the crash stack trace to stderr.
   PrintCurStackTrace(errs());



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


[PATCH] D74324: Tools emit the bug report URL on crash

2020-03-26 Thread Owen Reynolds via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa945037e8fd0: Tools emit the bug report URL on crash 
(authored by gbreynoo).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74324

Files:
  clang/lib/Driver/Driver.cpp
  clang/tools/driver/driver.cpp
  llvm/include/llvm/Support/PrettyStackTrace.h
  llvm/lib/Support/PrettyStackTrace.cpp


Index: llvm/lib/Support/PrettyStackTrace.cpp
===
--- llvm/lib/Support/PrettyStackTrace.cpp
+++ llvm/lib/Support/PrettyStackTrace.cpp
@@ -142,9 +142,17 @@
 static CrashHandlerStringStorage crashHandlerStringStorage;
 #endif
 
+static const char *BugReportMsg =
+"PLEASE submit a bug report to " BUG_REPORT_URL
+" and include the crash backtrace.\n";
+
+void llvm::setBugReportMsg(const char *Msg) { BugReportMsg = Msg; }
+
 /// This callback is run if a fatal signal is delivered to the process, it
 /// prints the pretty stack trace.
 static void CrashHandler(void *) {
+  errs() << BugReportMsg ;
+
 #ifndef __APPLE__
   // On non-apple systems, just emit the crash stack trace to stderr.
   PrintCurStackTrace(errs());
Index: llvm/include/llvm/Support/PrettyStackTrace.h
===
--- llvm/include/llvm/Support/PrettyStackTrace.h
+++ llvm/include/llvm/Support/PrettyStackTrace.h
@@ -37,6 +37,10 @@
   /// \see PrettyStackTraceEntry
   void EnablePrettyStackTraceOnSigInfoForThisThread(bool ShouldEnable = true);
 
+  /// Replaces the generic bug report message that is output upon
+  /// a crash.
+  void setBugReportMsg(const char *Msg);
+
   /// PrettyStackTraceEntry - This class is used to represent a frame of the
   /// "pretty" stack trace that is dumped when a program crashes. You can 
define
   /// subclasses of this and declare them on the program stack: when they are
Index: clang/tools/driver/driver.cpp
===
--- clang/tools/driver/driver.cpp
+++ clang/tools/driver/driver.cpp
@@ -38,6 +38,7 @@
 #include "llvm/Support/Host.h"
 #include "llvm/Support/InitLLVM.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/Process.h"
 #include "llvm/Support/Program.h"
 #include "llvm/Support/Regex.h"
@@ -342,6 +343,9 @@
 int main(int argc_, const char **argv_) {
   noteBottomOfStack();
   llvm::InitLLVM X(argc_, argv_);
+  llvm::setBugReportMsg("PLEASE submit a bug report to " BUG_REPORT_URL
+" and include the crash backtrace, preprocessed "
+"source, and associated run script.\n");
   SmallVector argv(argv_, argv_ + argc_);
 
   if (llvm::sys::Process::FixupStandardFileDescriptors())
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -1267,10 +1267,6 @@
   // Print the version of the compiler.
   PrintVersion(C, llvm::errs());
 
-  Diag(clang::diag::note_drv_command_failed_diag_msg)
-  << "PLEASE submit a bug report to " BUG_REPORT_URL " and include the "
- "crash backtrace, preprocessed source, and associated run script.";
-
   // Suppress driver output and emit preprocessor output to temp file.
   Mode = CPPMode;
   CCGenDiagnostics = true;


Index: llvm/lib/Support/PrettyStackTrace.cpp
===
--- llvm/lib/Support/PrettyStackTrace.cpp
+++ llvm/lib/Support/PrettyStackTrace.cpp
@@ -142,9 +142,17 @@
 static CrashHandlerStringStorage crashHandlerStringStorage;
 #endif
 
+static const char *BugReportMsg =
+"PLEASE submit a bug report to " BUG_REPORT_URL
+" and include the crash backtrace.\n";
+
+void llvm::setBugReportMsg(const char *Msg) { BugReportMsg = Msg; }
+
 /// This callback is run if a fatal signal is delivered to the process, it
 /// prints the pretty stack trace.
 static void CrashHandler(void *) {
+  errs() << BugReportMsg ;
+
 #ifndef __APPLE__
   // On non-apple systems, just emit the crash stack trace to stderr.
   PrintCurStackTrace(errs());
Index: llvm/include/llvm/Support/PrettyStackTrace.h
===
--- llvm/include/llvm/Support/PrettyStackTrace.h
+++ llvm/include/llvm/Support/PrettyStackTrace.h
@@ -37,6 +37,10 @@
   /// \see PrettyStackTraceEntry
   void EnablePrettyStackTraceOnSigInfoForThisThread(bool ShouldEnable = true);
 
+  /// Replaces the generic bug report message that is output upon
+  /// a crash.
+  void setBugReportMsg(const char *Msg);
+
   /// PrettyStackTraceEntry - This class is used to represent a frame of the
   /// "pretty" stack trace that is dumped when a program crashes. You can define
   /// subclasses of this and decla

[PATCH] D76830: [Analyzer][MallocChecker] No warning for kfree of ZERO_SIZE_PTR.

2020-03-26 Thread Balázs Kéri via Phabricator via cfe-commits
balazske marked 2 inline comments as done.
balazske added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:396
+  /// yes was the value obtained or not.
+  mutable Optional> KernelZeroSizePtrValue;
+

martong wrote:
> Which one is referred to the lazy initialization? The inner or the outer?
> These questions actually made me to come up with a more explanatory construct 
> here:
> Could we do something like this?
> ```
> using LazyInitialized = Optional;
> mutable Optional KernelZeroSizePtrValue; // Or 
> Lazy>
> ```
Probably use a `std::unique_ptr>` instead (like at the bug types, 
they could be optional too)?
If there is a code like
```
bool IsSomethingInitialized;
int Something;
```
that looks as a clear case to use an optional (or unique_ptr)? And if yes, is a 
reason for not using this construct if `int` is replaced by `Optional`?



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1687
+  if (ArgValKnown) {
+if (!KernelZeroSizePtrValue)
+  KernelZeroSizePtrValue =

martong wrote:
> martong wrote:
> > This is a bit confusing for me. Perhaps alternatively we could have a free 
> > function `isInitialized(KernelZero...)` instead. Or maybe having a separate 
> > bool variable to indicate whether it was initialized could be cleaner?
> Another idea: Adding a helper struct to contain the bool `initialized`? E.g. 
> (draft):
> ```
> struct LazyOptional {
>   bool initialized = false;
>   Opt value;
>   Opt& get();
>   void set(const Opt&);
> };
> ```
It may be OK to have a function `lazyInitKernelZeroSizePtrValue`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76830



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


[PATCH] D76770: [CodeComplete] Don't replace the rest of line in #include completion.

2020-03-26 Thread Haojian Wu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG297a9dac43f3: [CodeComplete] Don't replace the rest of 
line in #include completion. (authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76770

Files:
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang/lib/Lex/Lexer.cpp


Index: clang/lib/Lex/Lexer.cpp
===
--- clang/lib/Lex/Lexer.cpp
+++ clang/lib/Lex/Lexer.cpp
@@ -29,6 +29,7 @@
 #include "clang/Basic/TokenKinds.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/StringRef.h"
@@ -2092,7 +2093,8 @@
  bool IsAngled) {
   // Completion only applies to the filename, after the last slash.
   StringRef PartialPath(PathStart, CompletionPoint - PathStart);
-  auto Slash = PartialPath.find_last_of(LangOpts.MSVCCompat ? "/\\" : "/");
+  llvm::StringRef SlashChars = LangOpts.MSVCCompat ? "/\\" : "/";
+  auto Slash = PartialPath.find_last_of(SlashChars);
   StringRef Dir =
   (Slash == StringRef::npos) ? "" : PartialPath.take_front(Slash);
   const char *StartOfFilename =
@@ -2100,7 +2102,8 @@
   // Code completion filter range is the filename only, up to completion point.
   PP->setCodeCompletionIdentifierInfo(&PP->getIdentifierTable().get(
   StringRef(StartOfFilename, CompletionPoint - StartOfFilename)));
-  // We should replace the characters up to the closing quote, if any.
+  // We should replace the characters up to the closing quote or closest slash,
+  // if any.
   while (CompletionPoint < BufferEnd) {
 char Next = *(CompletionPoint + 1);
 if (Next == 0 || Next == '\r' || Next == '\n')
@@ -2108,7 +2111,10 @@
 ++CompletionPoint;
 if (Next == (IsAngled ? '>' : '"'))
   break;
+if (llvm::is_contained(SlashChars, Next))
+  break;
   }
+
   PP->setCodeCompletionTokenRange(
   FileLoc.getLocWithOffset(StartOfFilename - BufferStart),
   FileLoc.getLocWithOffset(CompletionPoint - BufferStart));
Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -1860,6 +1860,7 @@
 TEST(CompletionTest, CompletionTokenRange) {
   MockFSProvider FS;
   MockCompilationDatabase CDB;
+  FS.Files["foo/abc/foo.h"] = "";
   ClangdServer Server(CDB, FS, ClangdServer::optsForTest());
 
   constexpr const char *TestCodes[] = {
@@ -1882,7 +1883,14 @@
   Auxilary x;
   x.[[]]^;
 }
-  )cpp"};
+  )cpp",
+  R"cpp(
+#include "foo/[[a^/]]foo.h"
+  )cpp",
+  R"cpp(
+#include "foo/abc/[[fo^o.h"]]
+  )cpp",
+  };
   for (const auto &Text : TestCodes) {
 Annotations TestCode(Text);
 auto Results = completions(Server, TestCode.code(), TestCode.point());


Index: clang/lib/Lex/Lexer.cpp
===
--- clang/lib/Lex/Lexer.cpp
+++ clang/lib/Lex/Lexer.cpp
@@ -29,6 +29,7 @@
 #include "clang/Basic/TokenKinds.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/StringRef.h"
@@ -2092,7 +2093,8 @@
  bool IsAngled) {
   // Completion only applies to the filename, after the last slash.
   StringRef PartialPath(PathStart, CompletionPoint - PathStart);
-  auto Slash = PartialPath.find_last_of(LangOpts.MSVCCompat ? "/\\" : "/");
+  llvm::StringRef SlashChars = LangOpts.MSVCCompat ? "/\\" : "/";
+  auto Slash = PartialPath.find_last_of(SlashChars);
   StringRef Dir =
   (Slash == StringRef::npos) ? "" : PartialPath.take_front(Slash);
   const char *StartOfFilename =
@@ -2100,7 +2102,8 @@
   // Code completion filter range is the filename only, up to completion point.
   PP->setCodeCompletionIdentifierInfo(&PP->getIdentifierTable().get(
   StringRef(StartOfFilename, CompletionPoint - StartOfFilename)));
-  // We should replace the characters up to the closing quote, if any.
+  // We should replace the characters up to the closing quote or closest slash,
+  // if any.
   while (CompletionPoint < BufferEnd) {
 char Next = *(CompletionPoint + 1);
 if (Next == 0 || Next == '\r' || Next == '\n')
@@ -2108,7 +2111,10 @@
 ++CompletionPoint;
 if (Next == (IsAngled ? '>' : '"'))
   break;
+if (llvm::is_contained(SlashChars, Next))
+  break;
   }
+
   PP->setCodeCompletionTokenRange(
   FileLoc.getLocWithOffset(StartOfFilename - BufferStart),
   FileLoc.getLocWithOffset(CompletionPoint - BufferStart));
Index

[PATCH] D76830: [Analyzer][MallocChecker] No warning for kfree of ZERO_SIZE_PTR.

2020-03-26 Thread Gabor Marton via Phabricator via cfe-commits
martong added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1687
+  if (ArgValKnown) {
+if (!KernelZeroSizePtrValue)
+  KernelZeroSizePtrValue =

balazske wrote:
> martong wrote:
> > martong wrote:
> > > This is a bit confusing for me. Perhaps alternatively we could have a 
> > > free function `isInitialized(KernelZero...)` instead. Or maybe having a 
> > > separate bool variable to indicate whether it was initialized could be 
> > > cleaner?
> > Another idea: Adding a helper struct to contain the bool `initialized`? 
> > E.g. (draft):
> > ```
> > struct LazyOptional {
> >   bool initialized = false;
> >   Opt value;
> >   Opt& get();
> >   void set(const Opt&);
> > };
> > ```
> It may be OK to have a function `lazyInitKernelZeroSizePtrValue`?
I don't insist, given we have a better described type for 
`KernelZeroSizePtrValue` (e.g. having a `using` `template`)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76830



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


[PATCH] D76830: [Analyzer][MallocChecker] No warning for kfree of ZERO_SIZE_PTR.

2020-03-26 Thread Gabor Marton via Phabricator via cfe-commits
martong added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:396
+  /// yes was the value obtained or not.
+  mutable Optional> KernelZeroSizePtrValue;
+

balazske wrote:
> martong wrote:
> > Which one is referred to the lazy initialization? The inner or the outer?
> > These questions actually made me to come up with a more explanatory 
> > construct here:
> > Could we do something like this?
> > ```
> > using LazyInitialized = Optional;
> > mutable Optional KernelZeroSizePtrValue; // Or 
> > Lazy>
> > ```
> Probably use a `std::unique_ptr>` instead (like at the bug 
> types, they could be optional too)?
> If there is a code like
> ```
> bool IsSomethingInitialized;
> int Something;
> ```
> that looks as a clear case to use an optional (or unique_ptr)? And if yes, is 
> a reason for not using this construct if `int` is replaced by `Optional`?
Now I see that the lazy initialization is represented by the outer optional.
So IMHO a using template could be the best to describe cleanly this construct:
```
template 
using LazyInitialized = Optional;
mutable LazyInitialized> KernelZeroSizePtrValue;
```



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76830



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


[PATCH] D75951: Keep a list of already-included pragma-once files for mods.

2020-03-26 Thread Vy Nguyen via Phabricator via cfe-commits
oontvoo updated this revision to Diff 252791.
oontvoo added a comment.

Handle includes/import from outer mods


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75951

Files:
  clang/include/clang/Lex/HeaderSearch.h
  clang/include/clang/Lex/Preprocessor.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/include/clang/Serialization/ASTReader.h
  clang/include/clang/Serialization/ASTWriter.h
  clang/lib/Lex/HeaderSearch.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp

Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1619,7 +1619,7 @@
   endian::Writer LE(Out, little);
   unsigned KeyLen = key.Filename.size() + 1 + 8 + 8;
   LE.write(KeyLen);
-  unsigned DataLen = 1 + 2 + 4 + 4;
+  unsigned DataLen = 1 + 2 + 4 + 4 + 4;
   for (auto ModInfo : Data.KnownHeaders)
 if (Writer.getLocalOrImportedSubmoduleID(ModInfo.getModule()))
   DataLen += 4;
@@ -1676,6 +1676,9 @@
   }
   LE.write(Offset);
 
+  // Write this file UID.
+  LE.write(Data.HFI.UID);
+
   auto EmitModule = [&](Module *M, ModuleMap::ModuleHeaderRole Role) {
 if (uint32_t ModID = Writer.getLocalOrImportedSubmoduleID(M)) {
   uint32_t Value = (ModID << 2) | (unsigned)Role;
@@ -1703,7 +1706,7 @@
 /// Write the header search block for the list of files that
 ///
 /// \param HS The header search structure to save.
-void ASTWriter::WriteHeaderSearch(const HeaderSearch &HS) {
+void ASTWriter::WriteHeaderSearch(HeaderSearch &HS) {
   HeaderFileInfoTrait GeneratorTrait(*this);
   llvm::OnDiskChainedHashTableGenerator Generator;
   SmallVector SavedStrings;
@@ -1781,8 +1784,7 @@
 // changed since it was loaded. Also skip it if it's for a modular header
 // from a different module; in that case, we rely on the module(s)
 // containing the header to provide this information.
-const HeaderFileInfo *HFI =
-HS.getExistingFileInfo(File, /*WantExternal*/!Chain);
+HeaderFileInfo *HFI = HS.getExistingFileInfo(File, /*WantExternal*/ !Chain);
 if (!HFI || (HFI->isModuleHeader && !HFI->isCompilingModuleHeader))
   continue;
 
@@ -1799,8 +1801,13 @@
 HeaderFileInfoTrait::key_type Key = {
   Filename, File->getSize(), getTimestampForOutput(File)
 };
+// Set the UID for this HFI so that its importers could use it
+// when serializing.
+HFI->UID = UID;
 HeaderFileInfoTrait::data_type Data = {
-  *HFI, HS.getModuleMap().findAllModulesForHeader(File), {}
+*HFI,
+HS.getModuleMap().findAllModulesForHeader(File),
+{},
 };
 Generator.insert(Key, Data, GeneratorTrait);
 ++NumHeaderSearchEntries;
@@ -2634,6 +2641,25 @@
   Stream.EmitRecord(SUBMODULE_IMPORTS, Record);
 }
 
+// Emit the imported header's UIDs.
+{
+  auto it = PP->Submodules.find(Mod);
+  if (it != PP->Submodules.end() && !it->second.IncludedFiles.empty()) {
+RecordData Record;
+for (unsigned UID : it->second.IncludedFiles) {
+  // Only save it if the header is actually import/pragma once.
+  // FIXME When we first see a header, it always goes into the mod's
+  // list of included, regardless of whether it was pragma-once or not.
+  // Maybe better to fix that earlier?
+  auto HFI = PP->getHeaderSearchInfo().FileInfo[UID];
+  if (HFI.isImport || HFI.isPragmaOnce) {
+Record.push_back(HFI.UID);
+  }
+}
+Stream.EmitRecord(SUBMODULE_IMPORTED_HEADERS, Record);
+  }
+}
+
 // Emit the exports.
 if (!Mod->Exports.empty()) {
   RecordData Record;
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -1,3 +1,4 @@
+
 //===- ASTReader.cpp - AST File Reader ===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
@@ -1898,6 +1899,9 @@
 HFI.Framework = HS->getUniqueFrameworkName(FrameworkName);
   }
 
+  // Read the file old UID
+  HFI.UID = endian::readNext(d);
+
   assert((End - d) % 4 == 0 &&
  "Wrong data length in HeaderFileInfo deserialization");
   while (d != End) {
@@ -5615,6 +5619,11 @@
   }
   break;
 
+case SUBMODULE_IMPORTED_HEADERS:
+  for (unsigned Idx = 0; Idx < Record.size(); ++Idx) {
+PendingImportedHeaders[&F].push_back(Record[Idx]);
+  }
+  break;
 case SUBMODULE_EXPORTS:
   for (unsigned Idx = 0; Idx + 1 < Record.size(); Idx += 2) {
 UnresolvedModuleRef Unresolved;
@@ -9271,6 +9280,37 @@
   for (auto *ND : PendingMergedDefinitionsToDeduplicate)
 

[clang] 38798d0 - Revert "[AST] Fix thinlto testcase missed in 159a9f7e76307734bcdcae3357640e42e0733194"

2020-03-26 Thread Sam McCall via cfe-commits

Author: Sam McCall
Date: 2020-03-26T12:38:33+01:00
New Revision: 38798d03061ad400e0b900288cb9613907b2011a

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

LOG: Revert "[AST] Fix thinlto testcase missed in 
159a9f7e76307734bcdcae3357640e42e0733194"

This reverts commit 4bd1d55884aaeb582aa8f313e45823fe0f60b32d.
Cure is worse than the disease: "> >" is still expected in most configs.
Working on fixing the fuchsia builder.

Added: 


Modified: 
clang/test/CodeGen/thinlto-distributed-newpm.ll

Removed: 




diff  --git a/clang/test/CodeGen/thinlto-distributed-newpm.ll 
b/clang/test/CodeGen/thinlto-distributed-newpm.ll
index 23ba917de7a3..bac9c4322045 100644
--- a/clang/test/CodeGen/thinlto-distributed-newpm.ll
+++ b/clang/test/CodeGen/thinlto-distributed-newpm.ll
@@ -34,7 +34,7 @@
 ; CHECK-O: Running analysis: OptimizationRemarkEmitterAnalysis on main
 ; CHECK-O: Running analysis: PassInstrumentationAnalysis on main
 ; CHECK-O: Running pass: InferFunctionAttrsPass
-; CHECK-O: Running pass: 
ModuleToFunctionPassAdaptor<{{.*}}PassManager<{{.*}}Function>>
+; CHECK-O: Running pass: 
ModuleToFunctionPassAdaptor<{{.*}}PassManager<{{.*}}Function> >
 ; CHECK-O: Starting {{.*}}Function pass manager run.
 ; CHECK-O: Running pass: SimplifyCFGPass on main
 ; CHECK-O: Running analysis: TargetIRAnalysis on main
@@ -57,7 +57,7 @@
 ; CHECK-O: Running analysis: PassInstrumentationAnalysis on main
 ; CHECK-O: Running analysis: AssumptionAnalysis on main
 ; CHECK-O: Running pass: DeadArgumentEliminationPass
-; CHECK-O: Running pass: 
ModuleToFunctionPassAdaptor<{{.*}}PassManager<{{.*}}Function>>
+; CHECK-O: Running pass: 
ModuleToFunctionPassAdaptor<{{.*}}PassManager<{{.*}}Function> >
 ; CHECK-O: Starting {{.*}}Function pass manager run.
 ; CHECK-O: Running pass: InstCombinePass on main
 ; CHECK-O: Running analysis: TargetLibraryAnalysis on main
@@ -179,7 +179,7 @@
 ; CHECK-O: Running pass: ReversePostOrderFunctionAttrsPass
 ; CHECK-O: Running analysis: CallGraphAnalysis
 ; CHECK-O: Running pass: RequireAnalysisPass<{{.*}}GlobalsAA
-; CHECK-O: Running pass: 
ModuleToFunctionPassAdaptor<{{.*}}PassManager<{{.*}}Function>>
+; CHECK-O: Running pass: 
ModuleToFunctionPassAdaptor<{{.*}}PassManager<{{.*}}Function> >
 ; CHECK-O: Starting {{.*}}Function pass manager run.
 ; CHECK-O: Running pass: Float2IntPass on main
 ; CHECK-O: Running pass: LowerConstantIntrinsicsPass on main



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


[clang] 13dc21e - [AST] Make thinlto testcase robust to 159a9f7e76307734bcdcae3357640e42e0733194

2020-03-26 Thread Sam McCall via cfe-commits

Author: Sam McCall
Date: 2020-03-26T12:47:39+01:00
New Revision: 13dc21e84168b73c0157fd909ba7f130f06ae0f5

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

LOG: [AST] Make thinlto testcase robust to 
159a9f7e76307734bcdcae3357640e42e0733194

Ultimately it relies on the output of __PRETTY_FUNCTION__ which isn't reliable.

Added: 


Modified: 
clang/test/CodeGen/thinlto-distributed-newpm.ll

Removed: 




diff  --git a/clang/test/CodeGen/thinlto-distributed-newpm.ll 
b/clang/test/CodeGen/thinlto-distributed-newpm.ll
index bac9c4322045..5740c6f616be 100644
--- a/clang/test/CodeGen/thinlto-distributed-newpm.ll
+++ b/clang/test/CodeGen/thinlto-distributed-newpm.ll
@@ -34,7 +34,7 @@
 ; CHECK-O: Running analysis: OptimizationRemarkEmitterAnalysis on main
 ; CHECK-O: Running analysis: PassInstrumentationAnalysis on main
 ; CHECK-O: Running pass: InferFunctionAttrsPass
-; CHECK-O: Running pass: 
ModuleToFunctionPassAdaptor<{{.*}}PassManager<{{.*}}Function> >
+; CHECK-O: Running pass: 
ModuleToFunctionPassAdaptor<{{.*}}PassManager<{{.*}}Function>{{ ?}}>
 ; CHECK-O: Starting {{.*}}Function pass manager run.
 ; CHECK-O: Running pass: SimplifyCFGPass on main
 ; CHECK-O: Running analysis: TargetIRAnalysis on main
@@ -57,7 +57,7 @@
 ; CHECK-O: Running analysis: PassInstrumentationAnalysis on main
 ; CHECK-O: Running analysis: AssumptionAnalysis on main
 ; CHECK-O: Running pass: DeadArgumentEliminationPass
-; CHECK-O: Running pass: 
ModuleToFunctionPassAdaptor<{{.*}}PassManager<{{.*}}Function> >
+; CHECK-O: Running pass: 
ModuleToFunctionPassAdaptor<{{.*}}PassManager<{{.*}}Function>{{ ?}}>
 ; CHECK-O: Starting {{.*}}Function pass manager run.
 ; CHECK-O: Running pass: InstCombinePass on main
 ; CHECK-O: Running analysis: TargetLibraryAnalysis on main
@@ -179,7 +179,7 @@
 ; CHECK-O: Running pass: ReversePostOrderFunctionAttrsPass
 ; CHECK-O: Running analysis: CallGraphAnalysis
 ; CHECK-O: Running pass: RequireAnalysisPass<{{.*}}GlobalsAA
-; CHECK-O: Running pass: 
ModuleToFunctionPassAdaptor<{{.*}}PassManager<{{.*}}Function> >
+; CHECK-O: Running pass: 
ModuleToFunctionPassAdaptor<{{.*}}PassManager<{{.*}}Function>{{ ?}}>
 ; CHECK-O: Starting {{.*}}Function pass manager run.
 ; CHECK-O: Running pass: Float2IntPass on main
 ; CHECK-O: Running pass: LowerConstantIntrinsicsPass on main



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


[PATCH] D73649: [CodeComplete] Member completion for concept-constrained types.

2020-03-26 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments.



Comment at: clang/include/clang/Sema/Scope.h:323
   /// declared in.
-  bool isDeclScope(Decl *D) {
-return DeclsInScope.count(D) != 0;
-  }
+  bool isDeclScope(const Decl *D) { return DeclsInScope.count(D) != 0; }
 

also mark the member as `const` ?



Comment at: clang/lib/Sema/SemaCodeComplete.cpp:4905
+  // A requires(){...} lets us infer members from each requirement.
+  for (const concepts::Requirement *Req : RE->getRequirements()) {
+if (!Req->isDependent())

nit s/concepts::Req../auto



Comment at: clang/lib/Sema/SemaCodeComplete.cpp:4910
+  // Do a full traversal so we get `foo` from `typename T::foo::bar`.
+  QualType AssertedType = TR->getType()->getType();
+  ValidVisitor(this, T).TraverseType(AssertedType);

TR->getType might fail if there was a substitution failure. check for it before 
?

if(TR->isSubstitutionFailure()) continue;



Comment at: clang/lib/Sema/SemaCodeComplete.cpp:4984
+if (Q && isApprox(Q->getAsType(), T))
+  addType(NNS->getAsIdentifier());
+  }

as T is dependent i suppose NNS should always be an identifier, but would be 
nice to spell it out explicitly with a comment.



Comment at: clang/lib/Sema/SemaCodeComplete.cpp:5007
+  if (/*Inserted*/ R.second ||
+  std::make_tuple(M.Operator, M.ArgTypes.hasValue(),
+  M.ResultType != nullptr) >

so we'll give up result in case of (syntax might be wrong):
```
... concept C requires { T::foo; { t.foo() }-> bar }
```

assuming we first traversed t.foo and then T::foo ? I would rather move 
operator to the end.



Comment at: clang/lib/Sema/SemaCodeComplete.cpp:5077
+  // variables associated with DC (as returned by getTemplatedEntity()).
+  static ::SmallVector
+  constraintsForTemplatedEntity(DeclContext *DC) {

s/::SmallVector/llvm::SmallVector



Comment at: clang/lib/Sema/SemaCodeComplete.cpp:5094
+
+  static QualType extractExactType(const TypeConstraint &T) {
+// Assume a same_as return type constraint is std::same_as or 
equivalent.

maybe rename this to `deduceType`?

Also some comments explaining that this is a `beautify heuristic` might be nice.



Comment at: clang/lib/Sema/SemaCodeComplete.cpp:5169
+  } else if (BaseType->isObjCObjectPointerType() ||
+ BaseType->isTemplateTypeParmType())
 /*Do nothing*/;

nit:
```
// ObjcPointers(properties) and TTPT(concepts) are handled below, bail out for 
the resst.
else if (!objcPointer && ! TTPT) return false;
```



Comment at: clang/lib/Sema/SemaCodeComplete.cpp:5831
+  NestedNameSpecifier *NNS = SS.getScopeRep();
+  if (SS.isNotEmpty() && SS.isValid() && !NNS->isDependent()) {
+if (Ctx == nullptr || RequireCompleteDeclContext(SS, Ctx))

SS.isNotEmpty is same as NNS!=nullptr, maybe replace with that to make it more 
clear ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73649



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


[PATCH] D75286: [clangd] Handle clang-tidy suppression comments for diagnostics inside macro expansions

2020-03-26 Thread 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 for taking a while to come back to this again. A bit chaotic right now :-)

Looks good, some further simplification possible.




Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:298
 
+static const char *getCharacterData(const SourceManager &SM, SourceLocation 
Loc,
+bool *Invalid, bool AvoidIO) {

This seems more complicated than necessary - e.g. it replicates the "invalid" 
handling from SourceManager::getCharacterData, when we just bail out in this 
case. And keeping the raw char* interfaces seems unfortunate if we're going to 
the trouble of reimplementing most of this anyway.

What about implementing `llvm::Optional getBuffer(FileID File, bool 
AllowIO)`?
Where AllowIO determines whether you call getbuffer or getrawbuffer.

This is simpler than the current function, and the caller can be a bit more 
expressive/simpler like:
```
unsigned Offset = SM.getFileOffset(Loc);
StringRef RestOfLine = Data.substr(Offset).split('\n').first;
StringRef PrevLine = Data.substr(0, 
Offset).rsplit('\n').first.rsplit('\n').second;
```




Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h:220
   const Diagnostic &Info, ClangTidyContext 
&Context,
-  bool CheckMacroExpansion = true);
+  bool AvoidIO = false);
 

nit: I'd use `AllowIO=true` to avoid the negative sense, up to you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75286



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


[PATCH] D76830: [Analyzer][MallocChecker] No warning for kfree of ZERO_SIZE_PTR.

2020-03-26 Thread Balázs Kéri via Phabricator via cfe-commits
balazske marked an inline comment as done.
balazske added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:396
+  /// yes was the value obtained or not.
+  mutable Optional> KernelZeroSizePtrValue;
+

martong wrote:
> balazske wrote:
> > martong wrote:
> > > Which one is referred to the lazy initialization? The inner or the outer?
> > > These questions actually made me to come up with a more explanatory 
> > > construct here:
> > > Could we do something like this?
> > > ```
> > > using LazyInitialized = Optional;
> > > mutable Optional KernelZeroSizePtrValue; // Or 
> > > Lazy>
> > > ```
> > Probably use a `std::unique_ptr>` instead (like at the bug 
> > types, they could be optional too)?
> > If there is a code like
> > ```
> > bool IsSomethingInitialized;
> > int Something;
> > ```
> > that looks as a clear case to use an optional (or unique_ptr)? And if yes, 
> > is a reason for not using this construct if `int` is replaced by 
> > `Optional`?
> Now I see that the lazy initialization is represented by the outer optional.
> So IMHO a using template could be the best to describe cleanly this construct:
> ```
> template 
> using LazyInitialized = Optional;
> mutable LazyInitialized> KernelZeroSizePtrValue;
> ```
> 
With this I would wait for opinion of somebody else.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76830



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


[PATCH] D76830: [Analyzer][MallocChecker] No warning for kfree of ZERO_SIZE_PTR.

2020-03-26 Thread Balázs Kéri via Phabricator via cfe-commits
balazske updated this revision to Diff 252804.
balazske added a comment.

Improved documentation and handling of `KernelZeroSizePtrValue`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76830

Files:
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp
  clang/test/Analysis/kmalloc-linux-1.c

Index: clang/test/Analysis/kmalloc-linux-1.c
===
--- /dev/null
+++ clang/test/Analysis/kmalloc-linux-1.c
@@ -0,0 +1,27 @@
+// RUN: %clang_analyze_cc1 -triple x86_64-unknown-linux -analyzer-checker=unix.Malloc -verify %s
+
+#define ZERO_SIZE_PTR ((void *)16)
+
+void *__kmalloc(int size, int flags);
+
+// Linux kmalloc looks like this.
+// (simplified)
+void *kmalloc(int size, int flags) {
+  if (size == 0)
+return ZERO_SIZE_PTR;
+  return __kmalloc(size, flags);
+}
+
+// FIXME: malloc checker expects kfree with 2 arguments, is this correct?
+// (recent kernel source code contains a `kfree(const void *)`)
+void kfree(void *, int);
+
+void test_kmalloc_zero_sized_block_fixed_value_address() {
+  void *ptr = kmalloc(0, 0); // kmalloc returns a constant address
+  kfree(ptr, 0); // no warning about freeing a constant value
+}
+
+void test_kfree_constant_value() {
+  void *ptr = (void *)1;
+  kfree(ptr, 0); // expected-warning{{Argument to kfree() is a constant address (1)}}
+}
Index: clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp
===
--- clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp
+++ clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp
@@ -126,9 +126,6 @@
 if (!T.isOneOf(tok::l_paren, tok::r_paren))
   FilteredTokens.push_back(T);
 
-  if (FilteredTokens.size() > 2)
-return llvm::None;
-
   // Parse an integer at the end of the macro definition.
   const Token &T = FilteredTokens.back();
   if (!T.isLiteral())
@@ -140,11 +137,10 @@
 return llvm::None;
 
   // Parse an optional minus sign.
-  if (FilteredTokens.size() == 2) {
-if (FilteredTokens.front().is(tok::minus))
+  size_t Size = FilteredTokens.size();
+  if (Size >= 2) {
+if (FilteredTokens[Size - 2].is(tok::minus))
   IntValue = -IntValue;
-else
-  return llvm::None;
   }
 
   return IntValue.getSExtValue();
Index: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -58,6 +58,7 @@
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
@@ -389,6 +390,13 @@
   // TODO: Remove mutable by moving the initializtaion to the registry function.
   mutable Optional KernelZeroFlagVal;
 
+  // This type stores obtained value of macro `ZERO_SIZE_PTR`, and if it could
+  // be obtained.
+  using KernelZeroSizePtrValueTy = Optional;
+  /// Store the optional value of macro called `ZERO_SIZE_PTR`.
+  /// The value is initialized at first use.
+  mutable Optional KernelZeroSizePtrValue;
+
   /// Process C++ operator new()'s allocation, which is the part of C++
   /// new-expression that goes before the constructor.
   void processNewAllocation(const CXXNewExpr *NE, CheckerContext &C,
@@ -658,6 +666,13 @@
 CheckerContext &C);
 
   void reportLeak(SymbolRef Sym, ExplodedNode *N, CheckerContext &C) const;
+
+  void initKernelZeroSizePtrValue(Preprocessor &PP) const {
+// If not initialized yet,
+if (!KernelZeroSizePtrValue)
+  // try to get the value.
+  KernelZeroSizePtrValue = tryExpandAsInteger("ZERO_SIZE_PTR", PP);
+  }
 };
 
 //===--===//
@@ -1672,6 +1687,18 @@
   if (ArgVal.isUnknownOrUndef())
 return nullptr;
 
+  // If there is a macro called ZERO_SIZE_PTR, it could be a kernel source code
+  // and this value indicates a special value used for a zero-sized memory
+  // block. It is a constant value that is allowed to be freed.
+  const llvm::APSInt *ArgValKnown =
+  C.getSValBuilder().getKnownValue(State, ArgVal);
+  if (ArgValKnown) {
+initKernelZeroSizePtrValue(C.getPreprocessor());
+if (*KernelZeroSizePtrValue &&
+ArgValKnown->getSExtValue() == **KernelZeroSizePtrValue)
+  return nullptr;
+  }
+
   const MemRegion *R = ArgVal.getAsRegion();
 
   // Nonlocs can't be freed, of course.
___
cfe-

[PATCH] D75685: Add MS Mangling for OpenCL Pipe types, add mangling test.

2020-03-26 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia added inline comments.



Comment at: clang/test/CodeGenOpenCL/pipe_types_mangling.cl:12
+// WINDOWS: define dso_local void @"?test1@@YAXU?$ocl_pipe@H$00@__clang@@@Z"
+// UNMANGLED: define {{.*}}void @test1(
+}

Anastasia wrote:
> erichkeane wrote:
> > Anastasia wrote:
> > > erichkeane wrote:
> > > > Anastasia wrote:
> > > > > Any reason to test unmangled?
> > > > Because you asked to validate the OpenCL cases as well.
> > > Ok, I guess we should add overloadable attribute otherwise it doesn't go 
> > > through mangling? Then I guess you only need a check for Linux or Windows 
> > > cases.
> > Done, I wasn't able to remove OCLWINDOWS, since windows mangles attribute 
> > 'overloadable' in C only (and thus OpenCL).
> Sorry what I meant is we should probably add overloadable attr to all 
> functions so that we can check mangling in OpenCL C too. Then we won't need 
> to check `UNMANGLED`. 
> 
> I am very confused of why right now you only do it in some cases but not 
> all...
I just noticed that my earlier comment has been addressed in 
https://reviews.llvm.org/rGfe5c719eaf57 but it didn't show on this review. So 
we are fine now! Thanks for looking at this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75685



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


[PATCH] D75685: Add MS Mangling for OpenCL Pipe types, add mangling test.

2020-03-26 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia added inline comments.



Comment at: clang/test/CodeGenOpenCL/pipe_types_mangling.cl:12
+// WINDOWS: define dso_local void @"?test1@@YAXU?$ocl_pipe@H$00@__clang@@@Z"
+// UNMANGLED: define {{.*}}void @test1(
+}

erichkeane wrote:
> Anastasia wrote:
> > erichkeane wrote:
> > > Anastasia wrote:
> > > > Any reason to test unmangled?
> > > Because you asked to validate the OpenCL cases as well.
> > Ok, I guess we should add overloadable attribute otherwise it doesn't go 
> > through mangling? Then I guess you only need a check for Linux or Windows 
> > cases.
> Done, I wasn't able to remove OCLWINDOWS, since windows mangles attribute 
> 'overloadable' in C only (and thus OpenCL).
Sorry what I meant is we should probably add overloadable attr to all functions 
so that we can check mangling in OpenCL C too. Then we won't need to check 
`UNMANGLED`. 

I am very confused of why right now you only do it in some cases but not all...



Comment at: clang/test/CodeGenOpenCL/pipe_types_mangling.cl:20
+// Note: overloadable attribute makes OpenCL Linux still mangle this,
+// but we cannot overload on pipe still.
+// OCLLINUX: define void @_Z5test28ocl_pipe

Do you mean overload on pipe element type?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75685



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


[PATCH] D76663: [clangd] Support new semanticTokens request from LSP 3.16.

2020-03-26 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment.

In D76663#1942273 , @sammccall wrote:

> For some context here, I was doing some digging as Heyward Fann is looking at 
> hooking up our existing syntax highlighting to coc.nvim, and he and Jack Guo 
> (of jackguo380/vim-lsp-cxx-highlight) were asking about the protocol.
>
> The new LSP protocol looks really solid, including better incremental 
> highlight support than the Theia proposal, though this patch doesn't 
> implement incremental yet. (In particular, no sending thousands of 
> re-highlights when inserting a new line). It's also request-response rather 
> than notification-based, which is easier to implement on the server side. 
> Also the VSCode client-side of our highlighting feels like significant 
> technical debt we could be rid of.
>
> So I think we should try to support the new LSP and drop the older Theia one 
> ASAP (clangd 12?), even if semantic highlighting isn't a really high priority 
> for us.


Thanks for the summary, sounds a good plan.

The patch is mostly good, just a few nits. I will try to play it with VSCode.




Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:597
+ llvm::json::Object{
+ {"documentProvider", true},
+ {"legend",

nit: shall we explicitly set the `rangeProvider` to false as we don't support 
the semantic token range request now.



Comment at: clang-tools-extra/clangd/Protocol.h:1377
+};
+llvm::json::Value toJSON(const SemanticTokens &FStatus);
+

nit: FStatus => Tokens.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:449
+std::vector
+toSemanticTokens(llvm::ArrayRef Tokens) {
+  std::vector Result;

nit: assert the Tokens is sorted?



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:452
+  const HighlightingToken *Last = nullptr;
+  for (const HighlightingToken &Tok : Tokens) {
+Result.emplace_back();

note that we don't calculate the column offset for `InactiveCode` token 
(`{(line, 0), (line, 0)}`), I think we probably calculate the range with the 
new implementation, maybe add a FIXME.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:489
+return "member";
+  case HighlightingKind::StaticMethod:
+return "function";

It seems ok now, but I think for static method, we should set the modifier to 
`static`, maybe add a FIXME to support token modifiers


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76663



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


[PATCH] D76772: [AMDGPU] Add __builtin_amdgcn_workgroup_size_x/y/z

2020-03-26 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm accepted this revision.
arsenm added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/test/CodeGenCUDA/amdgpu-workgroup-size.cu:2
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa \
+// RUN: -fcuda-is-device -emit-llvm -o - -x hip %s \
+// RUN: | FileCheck %s

I assume the addrspacecast got optimized out? Should this disable llvm passes?


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

https://reviews.llvm.org/D76772



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


[PATCH] D76365: [cuda][hip] Add CUDA builtin surface/texture reference support.

2020-03-26 Thread Michael Liao via Phabricator via cfe-commits
hliao updated this revision to Diff 252828.
hliao added a comment.

Rebase to the master code


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76365

Files:
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/Type.cpp
  clang/lib/CodeGen/CGCUDANV.cpp
  clang/lib/CodeGen/CGCUDARuntime.h
  clang/lib/CodeGen/CGExprAgg.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/CodeGen/TargetInfo.h
  clang/lib/Headers/__clang_cuda_runtime_wrapper.h
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CodeGenCUDA/surface.cu
  clang/test/CodeGenCUDA/texture.cu
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaCUDA/attr-declspec.cu
  clang/test/SemaCUDA/attributes-on-non-cuda.cu
  clang/test/SemaCUDA/bad-attributes.cu
  llvm/include/llvm/IR/Operator.h

Index: llvm/include/llvm/IR/Operator.h
===
--- llvm/include/llvm/IR/Operator.h
+++ llvm/include/llvm/IR/Operator.h
@@ -599,6 +599,25 @@
   }
 };
 
+class AddrSpaceCastOperator
+: public ConcreteOperator {
+  friend class AddrSpaceCastInst;
+  friend class ConstantExpr;
+
+public:
+  Value *getPointerOperand() { return getOperand(0); }
+
+  const Value *getPointerOperand() const { return getOperand(0); }
+
+  unsigned getSrcAddressSpace() const {
+return getPointerOperand()->getType()->getPointerAddressSpace();
+  }
+
+  unsigned getDestAddressSpace() const {
+return getType()->getPointerAddressSpace();
+  }
+};
+
 } // end namespace llvm
 
 #endif // LLVM_IR_OPERATOR_H
Index: clang/test/SemaCUDA/bad-attributes.cu
===
--- clang/test/SemaCUDA/bad-attributes.cu
+++ clang/test/SemaCUDA/bad-attributes.cu
@@ -70,3 +70,27 @@
 __device__ void device_fn() {
   __constant__ int c; // expected-error {{__constant__ variables must be global}}
 }
+
+typedef __attribute__((device_builtin_surface_type)) unsigned long long s0_ty; // expected-warning {{'device_builtin_surface_type' attribute only applies to classes}}
+typedef __attribute__((device_builtin_texture_type)) unsigned long long t0_ty; // expected-warning {{'device_builtin_texture_type' attribute only applies to classes}}
+
+struct __attribute__((device_builtin_surface_type)) s1_ref {}; // expected-error {{illegal device builtin surface reference type 's1_ref' declared here}}
+// expected-note@-1 {{'s1_ref' needs to be instantiated from a class template with proper template arguments}}
+struct __attribute__((device_builtin_texture_type)) t1_ref {}; // expected-error {{illegal device builtin texture reference type 't1_ref' declared here}}
+// expected-note@-1 {{'t1_ref' needs to be instantiated from a class template with proper template arguments}}
+
+template 
+struct __attribute__((device_builtin_surface_type)) s2_cls_template {}; // expected-error {{illegal device builtin surface reference class template 's2_cls_template' declared here}}
+// expected-note@-1 {{'s2_cls_template' needs to have exactly 2 template parameters}}
+template 
+struct __attribute__((device_builtin_texture_type)) t2_cls_template {}; // expected-error {{illegal device builtin texture reference class template 't2_cls_template' declared here}}
+// expected-note@-1 {{'t2_cls_template' needs to have exactly 3 template parameters}}
+
+template 
+struct __attribute__((device_builtin_surface_type)) s3_cls_template {}; // expected-error {{illegal device builtin surface reference class template 's3_cls_template' declared here}}
+// expected-note@-1 {{the 1st template parameter of 's3_cls_template' needs to be a type}}
+// expected-note@-2 {{the 2nd template parameter of 's3_cls_template' needs to be an integer or enum value}}
+template 
+struct __attribute__((device_builtin_texture_type)) t3_cls_template {}; // expected-error {{illegal device builtin texture reference class template 't3_cls_template' declared here}}
+// expected-note@-1 {{the 1st template parameter of 't3_cls_template' needs to be a type}}
+// expected-note@-2 {{the 3rd template parameter of 't3_cls_template' needs to be an integer or enum value}}
Index: clang/test/SemaCUDA/attributes-on-non-cuda.cu
===
--- clang/test/SemaCUDA/attributes-on-non-cuda.cu
+++ clang/test/SemaCUDA/attributes-on-non-cuda.cu
@@ -7,16 +7,19 @@
 // RUN: %clang_cc1 -DEXPECT_WARNINGS -fsyntax-only -verify -x c %s
 
 #if defined(EXPECT_WARNINGS)
-// expected-warning@+12 {{'device' attribute ignored}}
-// expected-warning@+12 {{'global' attribute ignored}}
-// expected-warning@+12 {{'constant' attribute ignored}}
-// expected-warning@+12 {{'shared' attribute ignored}}
-// expected-warning@+12 

[PATCH] D76546: [Hexagon] MaxAtomicPromoteWidth, MaxAtomicInlineWidth are not getting set.

2020-03-26 Thread Brian Cain via Phabricator via cfe-commits
bcain accepted this revision.
bcain added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76546



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


[PATCH] D76830: [Analyzer][MallocChecker] No warning for kfree of ZERO_SIZE_PTR.

2020-03-26 Thread Gabor Marton via Phabricator via cfe-commits
martong accepted this revision.
martong added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: rnkovacs.

LGTM!




Comment at: clang/test/Analysis/kmalloc-linux-1.c:15
+
+// FIXME: malloc checker expects kfree with 2 arguments, is this correct?
+// (recent kernel source code contains a `kfree(const void *)`)

Do you plan to sniff around a bit about the arguments (as part of another 
patch)? Would be good to handle both old and new signature kernel allocation 
functions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76830



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


[PATCH] D76696: [AST] Build recovery expressions by default for C++.

2020-03-26 Thread Bevin Hansson via Phabricator via cfe-commits
ebevhan added a comment.

We found an odd case in our downstream fork after this patch landed. We have a 
diagnostic in `CheckVariableDeclarationType` that checks for automatic 
variables that are too large for the stack address space, and it chokes on the 
testcase `Parser/objcxx11-invalid-lambda.cpp`:

  void foo() {  // expected-note {{to match this '{'}}
int bar;
auto baz = [
bar(  // expected-note {{to match this '('}} expected-note {{to match 
this '('}}
  foo_undeclared() // expected-error{{use of undeclared identifier 
'foo_undeclared'}}
/* ) */
  ] () { };   // expected-error{{expected ')'}}
  }   // expected-error{{expected ')'}} expected-error {{expected 
',' or ']'}} expected-error{{expected ';' at end of declaration}} 
expected-error{{expected '}'}}

When the lambda is parsed, the parsing of the initializer expression of the 
'bar' capture fails since the paren is unbalanced, so it will build:

  ParenListExpr 0xc592ce8 'NULL TYPE' contains-errors
  `-RecoveryExpr 0xc592cc0 '' contains-errors lvalue
`-UnresolvedLookupExpr 0xc592c38 '' lvalue (ADL) 
= 'foo_undeclared' empty

Then, when building the lambda closure type, it will be given an auto member 
for `bar` with its type deduced to `AutoType 0xc592d40 'auto' dependent`:

  CXXRecordDecl 0xdfeb798 <...> col:14 implicit class definition
  |-DefinitionData lambda pass_in_registers standard_layout trivially_copyable 
can_const_default_init
  | |-DefaultConstructor
  | |-CopyConstructor simple trivial has_const_param needs_implicit 
implicit_has_const_param
  | |-MoveConstructor exists simple trivial needs_implicit
  | |-CopyAssignment trivial has_const_param needs_implicit 
implicit_has_const_param
  | |-MoveAssignment
  | `-Destructor simple irrelevant trivial
  |-CXXMethodDecl 0xdfeb8d0  line:5:14 operator() 'void () 
const' inline
  | `-CompoundStmt 0xdfeba20 
  |-FieldDecl 0xdfeba58  col:7 implicit 'auto'
  `-CXXDestructorDecl 0xdfebb08  col:14 implicit referenced ~ 'void 
() noexcept' inline default trivial

However, this just means that `baz` has a very odd type; it's a variable of 
closure type that contains a dependent member that will never be resolved. Our 
diagnostic breaks, since the variable's type isn't dependent, but it has a 
member which is, and we can't take the size of that.

Is the lambda parser really supposed to build this kind of odd type when faced 
with RecoveryExpr?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76696



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


[PATCH] D74054: [clangd] Include the underlying decls in go-to-definition.

2020-03-26 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

Sorry about being late coming back to this.




Comment at: clang-tools-extra/clangd/XRefs.cpp:307
+// alias.
+if (llvm::isa(D)) {
+  llvm::for_each(targetDecl(ast_type_traits::DynTypedNode::create(*D),

I believe this is going to do the wrong thing on overload sets.

```
namespace ns { int x(char); int x(double); }
using ns::x;
int y = ^x('a');
```

getDeclAtPosition(..., TemplatePattern|Alias) will yield UsingDecl, and then 
targetDecl(UsingDecl, Underlying) will yield both overloads of x.
However getDeclAtPosition(..., Underlying) would correctly return only the 
desired overload.
(Please add a test like this!)

This is awkward to work around, it's a failure of the targetDecl API. Maybe for 
now,  call getDeclAtPosition with TemplatePattern|Alias, and if there's exactly 
one result, replace D with it? And leave a fixme to address more complicated 
cases somehow.



Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:695
+  namespace ns { class [[Foo]] {}; }
+  using ns::[[F^oo]];
+)cpp",

nridge wrote:
> Why is it useful to give the using-declaration itself as a result?
> 
> It seems like the more useful behaviour from the user's point of view is to 
> jump directly to the definition of `class Foo`, without having to choose from 
> a popup first.
+1, that would mean adding a continue after the addresultdecl


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74054



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


[PATCH] D76850: clang-format: Fix pointer alignment for overloaded operators (PR45107)

2020-03-26 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision.
MyDeveloperDay added a comment.
This revision is now accepted and ready to land.

LGTM


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

https://reviews.llvm.org/D76850



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


[PATCH] D76725: [clangd] Build ASTs only with fresh preambles or after building a new preamble

2020-03-26 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 252841.
kadircet added a comment.

- Invalidate cached AST in case of preamble/input changes when building golden 
ASTs


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76725

Files:
  clang-tools-extra/clangd/Preamble.cpp
  clang-tools-extra/clangd/Preamble.h
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/unittests/FileIndexTests.cpp
  clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
  clang-tools-extra/clangd/unittests/TestTU.cpp

Index: clang-tools-extra/clangd/unittests/TestTU.cpp
===
--- clang-tools-extra/clangd/unittests/TestTU.cpp
+++ clang-tools-extra/clangd/unittests/TestTU.cpp
@@ -66,8 +66,7 @@
   auto CI = buildCompilerInvocation(Inputs, Diags);
   assert(CI && "Failed to build compilation invocation.");
   auto Preamble =
-  buildPreamble(FullFilename, *CI,
-/*OldPreamble=*/nullptr, Inputs,
+  buildPreamble(FullFilename, *CI, Inputs,
 /*StoreInMemory=*/true, /*PreambleCallback=*/nullptr);
   auto AST =
   buildAST(FullFilename, std::move(CI), Diags.take(), Inputs, Preamble);
Index: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
===
--- clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -283,6 +283,7 @@
 S.runWithPreamble("StaleRead", Path, TUScheduler::Stale,
   [&](Expected Pre) {
 ASSERT_TRUE(bool(Pre));
+ASSERT_TRUE(Pre->Preamble);
 EXPECT_EQ(Pre->Preamble->Version, "A");
 EXPECT_THAT(includes(Pre->Preamble),
 ElementsAre(""));
@@ -292,11 +293,13 @@
 S.runWithPreamble("ConsistentRead", Path, TUScheduler::Consistent,
   [&](Expected Pre) {
 ASSERT_TRUE(bool(Pre));
+ASSERT_TRUE(Pre->Preamble);
 EXPECT_EQ(Pre->Preamble->Version, "B");
 EXPECT_THAT(includes(Pre->Preamble),
 ElementsAre(""));
 ++CallbackCount;
   });
+S.blockUntilIdle(timeoutSeconds(10));
   }
   EXPECT_EQ(2, CallbackCount);
 }
@@ -853,15 +856,19 @@
   TUState(PreambleAction::Idle, ASTAction::RunningAction),
   // We build the preamble
   TUState(PreambleAction::Building, ASTAction::RunningAction),
-  // Preamble worker goes idle
+  // We built the preamble, and issued ast built on ASTWorker
+  // thread. Preambleworker goes idle afterwards.
   TUState(PreambleAction::Idle, ASTAction::RunningAction),
-  // We start building the ast
+  // Start task for building the ast, as a result of building
+  // preamble, on astworker thread.
+  TUState(PreambleAction::Idle, ASTAction::RunningAction),
+  // AST build starts.
   TUState(PreambleAction::Idle, ASTAction::Building),
-  // Built finished succesffully
+  // AST built finished successfully
   TUState(PreambleAction::Idle, ASTAction::Building),
-  // Rnning go to def
+  // Running go to def
   TUState(PreambleAction::Idle, ASTAction::RunningAction),
-  // both workers go idle
+  // ASTWorker goes idle.
   TUState(PreambleAction::Idle, ASTAction::Idle)));
 }
 
Index: clang-tools-extra/clangd/unittests/FileIndexTests.cpp
===
--- clang-tools-extra/clangd/unittests/FileIndexTests.cpp
+++ clang-tools-extra/clangd/unittests/FileIndexTests.cpp
@@ -286,7 +286,7 @@
 
   FileIndex Index;
   bool IndexUpdated = false;
-  buildPreamble(FooCpp, *CI, /*OldPreamble=*/nullptr, PI,
+  buildPreamble(FooCpp, *CI, PI,
 /*StoreInMemory=*/true,
 [&](ASTContext &Ctx, std::shared_ptr PP,
 const CanonicalIncludes &CanonIncludes) {
@@ -424,7 +424,7 @@
 }
 
 TEST(FileIndexTest, MergeMainFileSymbols) {
-  const char* CommonHeader = "void foo();";
+  const char *CommonHeader = "void foo();";
   TestTU Header = TestTU::withCode(CommonHeader);
   TestTU Cpp = TestTU::withCode("void foo() {}");
   Cpp.Filename = "foo.cpp";
Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -6,16 +6,46 @@
 //
 //===-

[PATCH] D76850: clang-format: Fix pointer alignment for overloaded operators (PR45107)

2020-03-26 Thread Hans Wennborg via Phabricator via cfe-commits
hans created this revision.
hans added reviewers: sammccall, MyDeveloperDay, thakis.
MyDeveloperDay accepted this revision.
MyDeveloperDay added a comment.
This revision is now accepted and ready to land.

LGTM


This fixes a regression from D69573  which 
broke the following example:

  $ echo 'operator C*();' | bin/clang-format --style=Chromium
  operator C *();

(There should be no space between before the asterisk.)

It seems the problem is in TokenAnnotator::spaceRequiredBetween(), which only 
looked at the token to the left of the * to see if it was a type or not. That 
code only handled simple types or identifiers, not templates or qualified 
types. This patch addresses that.

Please take a look. I'm not familiar with the code here, so comments are 
welcome.


https://reviews.llvm.org/D76850

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -15386,6 +15386,11 @@
   verifyFormat("Foo::operator&&();", Style);
   verifyFormat("operator&&(int(&&)(), class Foo);", Style);
 
+  // PR45107
+  verifyFormat("operator Vector&();", Style);
+  verifyFormat("operator foo::Bar*();", Style);
+  verifyFormat("operator const Foo::Bar*();", Style);
+
   Style.PointerAlignment = FormatStyle::PAS_Middle;
   verifyFormat("Foo::operator*();", Style);
   verifyFormat("Foo::operator void *();", Style);
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2799,20 +2799,38 @@
 tok::l_square));
   if (Right.is(tok::star) && Left.is(tok::l_paren))
 return false;
-  if (Right.isOneOf(tok::star, tok::amp, tok::ampamp) &&
-  (Left.is(tok::identifier) || Left.isSimpleTypeSpecifier()) &&
-  // Space between the type and the * in:
-  //   operator void*()
-  //   operator char*()
-  //   operator /*comment*/ const char*()
-  //   operator volatile /*comment*/ char*()
-  //   operator Foo*()
-  // dependent on PointerAlignment style.
-  Left.Previous &&
-  (Left.Previous->endsSequence(tok::kw_operator) ||
-   Left.Previous->endsSequence(tok::kw_const, tok::kw_operator) ||
-   Left.Previous->endsSequence(tok::kw_volatile, tok::kw_operator)))
-return (Style.PointerAlignment != FormatStyle::PAS_Left);
+  if (Right.isOneOf(tok::star, tok::amp, tok::ampamp)) {
+const FormatToken *Previous = &Left;
+while (Previous && !Previous->is(tok::kw_operator)) {
+  if (Previous->is(tok::identifier) || Previous->isSimpleTypeSpecifier()) {
+Previous = Previous->Previous;
+continue;
+  }
+  if (Previous->is(TT_TemplateCloser) && Previous->MatchingParen) {
+Previous = Previous->MatchingParen->Previous;
+continue;
+  }
+  if (Previous->is(tok::coloncolon)) {
+Previous = Previous->Previous;
+continue;
+  }
+  break;
+}
+// Space between the type and the * in:
+//   operator void*()
+//   operator char*()
+//   operator /*comment*/ const char*()
+//   operator volatile /*comment*/ char*()
+//   operator Foo*()
+//   operator C*()
+//   operator std::Foo*()
+//   operator C::D*()
+// dependent on PointerAlignment style.
+if (Previous && (Previous->endsSequence(tok::kw_operator) ||
+   Previous->endsSequence(tok::kw_const, tok::kw_operator) ||
+   Previous->endsSequence(tok::kw_volatile, tok::kw_operator)))
+  return (Style.PointerAlignment != FormatStyle::PAS_Left);
+  }
   const auto SpaceRequiredForArrayInitializerLSquare =
   [](const FormatToken &LSquareTok, const FormatStyle &Style) {
 return Style.SpacesInContainerLiterals ||


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -15386,6 +15386,11 @@
   verifyFormat("Foo::operator&&();", Style);
   verifyFormat("operator&&(int(&&)(), class Foo);", Style);
 
+  // PR45107
+  verifyFormat("operator Vector&();", Style);
+  verifyFormat("operator foo::Bar*();", Style);
+  verifyFormat("operator const Foo::Bar*();", Style);
+
   Style.PointerAlignment = FormatStyle::PAS_Middle;
   verifyFormat("Foo::operator*();", Style);
   verifyFormat("Foo::operator void *();", Style);
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2799,20 +2799,38 @@
 tok::l_square));
   if (Right.is(tok::star) && Left.is(tok::l_paren))
 return false;
-  if 

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

2020-03-26 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment.

Could the default be `Style.IndentExternBlock = 
Style.BraceWrapping.AfterExternBlock`

Then it would match the prior behaviour off AddLevel being `true` when 
AfterExternBlock is `true`

  extern "C" {
  int a;
  }
  
  extern "C" 
  {
int a;
  }


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

https://reviews.llvm.org/D75791



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


[PATCH] D74183: [IRGen] Add an alignment attribute to underaligned sret parameters

2020-03-26 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment.

In D74183#1941741 , @efriedma wrote:

> If it's just tramp3d-v4, I'm not that concerned... but that's a weird result. 
>  On x86 in particular, alignment markings have almost no effect on 
> optimization, generally.


I've just looked at the IR diff for tramp3d-v4 and it turns out that the root 
cause is an old friend of mine: The insertion of alignment assumptions during 
inlining 
(https://github.com/llvm/llvm-project/blob/b58902bc72c2b479b5ed27ec0d3422ba9782edbb/llvm/lib/Transforms/Utils/InlineFunction.cpp#L1139-L1173).
 That is, the IR now contains many instances of this sequence:

  %ptrint = ptrtoint %class.GuardLayers* %guards_m to i64
  %maskedptr = and i64 %ptrint, 3
  %maskcond = icmp eq i64 %maskedptr, 0
  tail call void @llvm.assume(i1 %maskcond)

to preserve the alignment information. From a cursory look I cannot tell 
whether these additional assumes also regress optimization (due to multi-use), 
but given the size increase on the final binary it seems pretty likely that 
this is the case.

This preservation of alignment during inlining is the reason why we used to not 
emit alignment information for pointer arguments in Rust for a long time: It 
caused serious regressions in optimization and increased compile-time. Nowadays 
we do emit alignment information, but set 
`-preserve-alignment-assumptions-during-inlining=false` to prevent this 
inlining behavior.

I think for the purposes of this revision, this means that we should probably 
either a) default `preserve-alignment-assumptions-during-inlining` to false (I 
would prefer this) or b) not emit the alignment unless it is smaller than the 
ABI alignment (I guess this was what this patch originally did?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74183



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


[PATCH] D74183: [IRGen] Add an alignment attribute to underaligned sret parameters

2020-03-26 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

Yeah, if emitting alignment assumptions in inlining is causing regressions when 
frontends provide better information, those assumptions need to be reverted 
until they can be fixed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74183



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


[PATCH] D74183: [IRGen] Add an alignment attribute to underaligned sret parameters

2020-03-26 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added subscribers: hfinkel, jdoerfert.
efriedma added a comment.

That makes sense.

I would slightly lean towards not generating the assumptions, given the current 
state of assumptions and the likely benefit in this context.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74183



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


[clang] 0cff81c - Add a release note for attribute plugins

2020-03-26 Thread John Brawn via cfe-commits

Author: John Brawn
Date: 2020-03-26T15:01:57Z
New Revision: 0cff81cff05d8e0a24391e3dec5b97174351ea55

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

LOG: Add a release note for attribute plugins

Added: 


Modified: 
clang/docs/ReleaseNotes.rst

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index ad13fb1b3e95..a8163cad9fde 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -111,7 +111,9 @@ New Pragmas in Clang
 Attribute Changes in Clang
 --
 
-- ...
+- Attributes can now be specified by clang plugins. See the
+  `Clang Plugins `_ documentation for
+  details.
 
 Windows Support
 ---



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


[PATCH] D76197: clang-format: Use block indentation for braced initializations

2020-03-26 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay requested changes to this revision.
MyDeveloperDay added a comment.
This revision now requires changes to proceed.

> New implementation that breaks fewer tests.

`fewer` is not an option, `no` is all thats going to get accepted.

Please add tests for you own use case


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

https://reviews.llvm.org/D76197



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


[PATCH] D76724: Prevent immediate evaluations inside of decltype

2020-03-26 Thread Wyatt Childers via Phabricator via cfe-commits
wchilders abandoned this revision.
wchilders added a comment.

Thanks Tyker, you're right, there is a general problem that needs handled, if 
this is to be handled; I had a bit of tunnel vision here. However, I'm 
retracting/abandoning this patch entirely as upon further review, it's not 
standard compliant. The current situation, is actually the correct one (for 
now?).

From [7.7.13] of the working draft:

> Note: A manifestly constant-evaluated expression is evaluated even in an 
> unevaluated operand.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76724



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


[PATCH] D76818: [clang-tidy] Add check llvmlibc-implementation-in-namespace.

2020-03-26 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment.

If you want to make it a general check, you should consider making the default 
module options set the correct namespace
RequiredNamespace

  ClangTidyOptions getModuleOptions() override {
ClangTidyOptions Options;

Options.CheckOptions["llvmlibc-implementation-in-namespace.RequiredNamespace"] 
= "__llvm_libc";
return Options;
  }




Comment at: 
clang-tools-extra/test/clang-tidy/checkers/llvmlibc-implementation-in-namespace.cpp:6
+class ClassB;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: CXXRecord is not defined in a 
namespace, please wrap implentation in '__llvm_libc' namespace.
+struct StructC {};

Small nit : Not a fan of the diagnostic saying `CXX Record`. Maybe a pain but 
`getDeclKindName` isn't the best to expose to users. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76818



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


[PATCH] D75677: [Analyzer] Only add iterator note tags to the operations of the affected iterators

2020-03-26 Thread Balogh, Ádám via Phabricator via cfe-commits
baloghadamsoftware marked an inline comment as done.
baloghadamsoftware added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp:541-542
+BR.markInteresting(It1);
+if (const auto &LCV1 = It1.getAs()) {
+  BR.markInteresting(LCV1->getRegion());
+}

Szelethus wrote:
> NoQ wrote:
> > baloghadamsoftware wrote:
> > > NoQ wrote:
> > > > I'm opposed to this code for the same reason that i'm opposed to it in 
> > > > the debug checker. Parent region is an undocumented implementation 
> > > > detail of `RegionStore`. It is supposed to be immaterial to the user. 
> > > > You should not rely on its exact value.
> > > > 
> > > > @baloghadamsoftware Can we eliminate all such code from iterator 
> > > > checkers and instead identify all iterators by regions in which they're 
> > > > stored? Does my improved C++ support help with this anyhow whenever it 
> > > > kicks in?
> > > How to find the region where it is stored? I am open to find better 
> > > solutions, but it was the only one I could find so far. If we ignore 
> > > `LazyCompoundVal` then everything falls apart, we can remove all the 
> > > iterator-related checkers.
> > It's the region from which you loaded it. If you obtained it as 
> > `Call.getArgSVal()` then it's the parameter region for the call. If you 
> > obtained it as `Call.getReturnValue()` then it's the target region can be 
> > obtained by looking at the //construction context// for the call.
> `LazyCompoundVal` and friends seem to be a constantly emerging headache for 
> almost everyone. For how long I've spent in the analyzer, and have 
> religiously studied conversations and your workbook about it, I still feel 
> anxious whenever it comes up.
> 
> It would be great to have this documented in the code sometime.
Do you mean `CallEvent::getParameterLocation()` for arguments? What is the 
//construction context// for the call? How can it be obtained?


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

https://reviews.llvm.org/D75677



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


[PATCH] D76696: [AST] Build recovery expressions by default for C++.

2020-03-26 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

In D76696#1943657 , @ebevhan wrote:

> However, this just means that `baz` has a very odd type; it's a variable of 
> closure type that contains a dependent member that will never be resolved. 
> Our diagnostic breaks, since the variable's type isn't dependent, but it has 
> a member which is, and we can't take the size of that.


Interesting. Simply turning this off for lambdas isn't enough. The following 
crashes clang for me:

  class X {
decltype(unresolved()) foo;
  };
  constexpr int s = sizeof(X);

@hokein I think we need to revert until we find a solution for this. I suspect 
this is going to mean adding the error bit to type and... dropping members that 
have it set? Or marking them as invalid?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76696



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


[clang] 2aac0c4 - Reland "[analyzer][NFC] Tie CheckerRegistry to CheckerManager, allow CheckerManager to be constructed for non-analysis purposes"

2020-03-26 Thread Kirstóf Umann via cfe-commits

Author: Kristóf Umann
Date: 2020-03-26T16:12:38+01:00
New Revision: 2aac0c47aed8d1eff7ab6d858173c532b881d948

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

LOG: Reland "[analyzer][NFC] Tie CheckerRegistry to CheckerManager, allow 
CheckerManager to be constructed for non-analysis purposes"

Originally commited in rG57b8a407493c34c3680e7e1e4cb82e097f43744a, but
it broke the modules bot. This is solved by putting the contructors of
the CheckerManager class to the Frontend library.

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

Added: 
clang/include/clang/StaticAnalyzer/Frontend/AnalyzerHelpFlags.h
clang/lib/StaticAnalyzer/Frontend/AnalyzerHelpFlags.cpp
clang/lib/StaticAnalyzer/Frontend/CreateCheckerManager.cpp

Modified: 
clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
clang/include/clang/StaticAnalyzer/Frontend/AnalysisConsumer.h
clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
clang/include/clang/StaticAnalyzer/Frontend/FrontendActions.h
clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
clang/lib/StaticAnalyzer/Frontend/CMakeLists.txt
clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp

Removed: 
clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistration.h
clang/lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp



diff  --git a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h 
b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
index 4454d7603b27..4635ca35736a 100644
--- a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
+++ b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
@@ -14,6 +14,7 @@
 #define LLVM_CLANG_STATICANALYZER_CORE_CHECKERMANAGER_H
 
 #include "clang/Analysis/ProgramPoint.h"
+#include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState_Fwd.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/Store.h"
@@ -121,14 +122,36 @@ enum class ObjCMessageVisitKind {
 };
 
 class CheckerManager {
-  ASTContext &Context;
+  ASTContext *Context;
   const LangOptions LangOpts;
   AnalyzerOptions &AOptions;
   CheckerNameRef CurrentCheckerName;
+  DiagnosticsEngine &Diags;
+  std::unique_ptr Registry;
 
 public:
+  // These constructors are defined in the Frontend library, because
+  // CheckerRegistry, a crucial component of the initialization is in there.
+  // CheckerRegistry cannot be moved to the Core library, because the checker
+  // registration functions are defined in the Checkers library, and the 
library
+  // dependencies look like this: Core -> Checkers -> Frontend.
+
+  CheckerManager(
+  ASTContext &Context, AnalyzerOptions &AOptions,
+  ArrayRef plugins,
+  ArrayRef> checkerRegistrationFns);
+
+  /// Constructs a CheckerManager that ignores all non TblGen-generated
+  /// checkers. Useful for unit testing, unless the checker infrastructure
+  /// itself is tested.
   CheckerManager(ASTContext &Context, AnalyzerOptions &AOptions)
-  : Context(Context), LangOpts(Context.getLangOpts()), AOptions(AOptions) 
{}
+  : CheckerManager(Context, AOptions, {}, {}) {}
+
+  /// Constructs a CheckerManager without requiring an AST. No checker
+  /// registration will take place. Only useful for retrieving the
+  /// CheckerRegistry and print for help flags where the AST is unavalaible.
+  CheckerManager(AnalyzerOptions &AOptions, const LangOptions &LangOpts,
+ DiagnosticsEngine &Diags, ArrayRef plugins);
 
   ~CheckerManager();
 
@@ -141,7 +164,12 @@ class CheckerManager {
 
   const LangOptions &getLangOpts() const { return LangOpts; }
   AnalyzerOptions &getAnalyzerOptions() const { return AOptions; }
-  ASTContext &getASTContext() const { return Context; }
+  const CheckerRegistry &getCheckerRegistry() const { return *Registry; }
+  DiagnosticsEngine &getDiagnostics() const { return Diags; }
+  ASTContext &getASTContext() const {
+assert(Context);
+return *Context;
+  }
 
   /// Emits an error through a DiagnosticsEngine about an invalid user supplied
   /// checker option value.

diff  --git a/clang/include/clang/StaticAnalyzer/Frontend/AnalysisConsumer.h 
b/clang/include/clang/StaticAnalyzer/Frontend/AnalysisConsumer.h
index 2d24e6a9586b..bcc29a60ad70 100644
--- a/clang/include/clang/StaticAnalyzer/Frontend/AnalysisConsumer.h
+++ b/clang/include/clang/StaticAnalyzer/Frontend/AnalysisConsumer.h
@@ -55,7 +55,7 @@ class AnalysisASTConsumer : public ASTConsumer {
 std::unique_ptr
 CreateAnalysisConsumer(CompilerInstance &CI);
 
-} // end GR namespace
+} // namespace ento
 
 } // end clang namespace
 

diff  --git a/clang/include/cla

[clang] 62dea6e - Revert "[AST] Build recovery expressions by default for C++."

2020-03-26 Thread Haojian Wu via cfe-commits

Author: Haojian Wu
Date: 2020-03-26T16:25:32+01:00
New Revision: 62dea6e9be31b100962f9ad41c1b79467a53f6cd

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

LOG: Revert "[AST] Build recovery expressions by default for C++."

This reverts commit 0788acbccbec094903a3425ffe5a98f8d55cbd64.
This reverts commit c2d7a1f79cedfc9fcb518596aa839da4de0adb69:  Revert "[clangd] 
Add test for FindTarget+RecoveryExpr (which already works). NFC"

It causes a crash on invalid code:

class X {
  decltype(unresolved()) foo;
};
constexpr int s = sizeof(X);

Added: 


Modified: 
clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
clang-tools-extra/clangd/unittests/FindTargetTests.cpp
clang/include/clang/Basic/LangOptions.def
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaExpr.cpp
clang/test/OpenMP/target_update_from_messages.cpp
clang/test/OpenMP/target_update_to_messages.cpp
clang/test/Parser/objcxx0x-lambda-expressions.mm
clang/test/Parser/objcxx11-invalid-lambda.cpp
clang/test/SemaCXX/builtins.cpp
clang/test/SemaCXX/cast-conversion.cpp
clang/test/SemaCXX/constructor-initializer.cpp
clang/test/SemaCXX/cxx1z-copy-omission.cpp
clang/test/SemaCXX/decltype-crash.cpp
clang/test/SemaCXX/varargs.cpp
clang/test/SemaOpenCLCXX/address-space-references.cl
clang/test/SemaTemplate/instantiate-init.cpp
clang/unittests/Sema/CodeCompleteTest.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp 
b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
index c691c6d9e61a..24197485f68a 100644
--- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -1195,9 +1195,7 @@ TEST(SignatureHelpTest, OpeningParen) {
 int foo(int a, int b, int c);
 int main() {
 #define ID(X) X
-  // FIXME: figure out why ID(foo (foo(10), )) doesn't work when preserving
-  // the recovery expression.
-  ID(foo $p^( 10, ^ ))
+  ID(foo $p^( foo(10), ^ ))
 })cpp"};
 
   for (auto Test : Tests) {

diff  --git a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp 
b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
index cd6f2039c888..c38ccc3f9441 100644
--- a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -132,16 +132,6 @@ TEST_F(TargetDeclTest, Exprs) {
   EXPECT_DECLS("CXXOperatorCallExpr", "void operator()(int n)");
 }
 
-TEST_F(TargetDeclTest, Recovery) {
-  Code = R"cpp(
-// error-ok: testing behavior on broken code
-int f();
-int f(int, int);
-int x = [[f]](42);
-  )cpp";
-  EXPECT_DECLS("UnresolvedLookupExpr", "int f()", "int f(int, int)");
-}
-
 TEST_F(TargetDeclTest, UsingDecl) {
   Code = R"cpp(
 namespace foo {
@@ -695,15 +685,6 @@ TEST_F(FindExplicitReferencesTest, All) {
 )cpp",
 "0: targets = {x}\n"
 "1: targets = {X::a}\n"},
-   {R"cpp(
-   // error-ok: testing with broken code
-   int bar();
-   int foo() {
- return $0^bar() + $1^bar(42);
-   }
-   )cpp",
-   "0: targets = {bar}\n"
-   "1: targets = {bar}\n"},
// Namespaces and aliases.
{R"cpp(
   namespace ns {}

diff  --git a/clang/include/clang/Basic/LangOptions.def 
b/clang/include/clang/Basic/LangOptions.def
index 22c4a87918e9..6152e227d599 100644
--- a/clang/include/clang/Basic/LangOptions.def
+++ b/clang/include/clang/Basic/LangOptions.def
@@ -148,7 +148,7 @@ LANGOPT(RelaxedTemplateTemplateArgs, 1, 0, "C++17 relaxed 
matching of template t
 
 LANGOPT(DoubleSquareBracketAttributes, 1, 0, "'[[]]' attributes extension for 
all language standard modes")
 
-COMPATIBLE_LANGOPT(RecoveryAST, 1, CPlusPlus, "Preserve expressions in AST 
when encountering errors")
+COMPATIBLE_LANGOPT(RecoveryAST, 1, 0, "Preserve expressions in AST when 
encountering errors")
 
 BENIGN_LANGOPT(ThreadsafeStatics , 1, 1, "thread-safe static initializers")
 LANGOPT(POSIXThreads  , 1, 0, "POSIX thread support")

diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp 
b/clang/lib/Frontend/CompilerInvocation.cpp
index 08e0700671a8..dc5e932c9460 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -2909,7 +2909,7 @@ static void ParseLangArgs(LangOptions &Opts, ArgList 
&Args, InputKind IK,
   if (Args.hasArg(OPT_fconcepts_ts))
 Diags.Report(diag::warn_fe_concepts_ts_flag);
   Opts.RecoveryAST =
-  Args.hasFlag(OPT_frecovery_ast, OPT_fno_recovery_ast, Opts.CPlusPlus);
+  Args.hasFlag(OPT_frecovery_ast, OPT_fno_recovery_ast, false);
   Opts.HeinousExtensions = Args.hasArg(OPT_fheinous_gnu_extensions);
   Opts.AccessControl = !Args.has

[PATCH] D75360: [analyzer][NFC] Tie CheckerRegistry to CheckerManager, allow CheckerManager to be constructed for non-analysis purposes

2020-03-26 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment.

Tried to recommit -- I'll keep a close eye on this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75360



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


[PATCH] D31343: Add an attribute plugin example

2020-03-26 Thread John Brawn via Phabricator via cfe-commits
john.brawn added a comment.

Release note added: https://reviews.llvm.org/rG0cff81cff05d


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D31343



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


[PATCH] D76197: clang-format: Use block indentation for braced initializations

2020-03-26 Thread Daan De Meyer via Phabricator via cfe-commits
DaanDeMeyer added a comment.

Of course, if there's interest in adding this I'll fix all the tests but I 
wanted to make sure there was interest in adding this since it changes 
clang-format's behavior. Can you confirm that this change in behavior is a good 
thing and might be merged if I fix all the tests? If the existing output cannot 
be changed in any way then this revision can be closed since it fundamentally 
requires changing clang-format's output.


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

https://reviews.llvm.org/D76197



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


[PATCH] D75677: [Analyzer] Only add iterator note tags to the operations of the affected iterators

2020-03-26 Thread Balogh, Ádám via Phabricator via cfe-commits
baloghadamsoftware marked an inline comment as done.
baloghadamsoftware added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp:541-542
+BR.markInteresting(It1);
+if (const auto &LCV1 = It1.getAs()) {
+  BR.markInteresting(LCV1->getRegion());
+}

baloghadamsoftware wrote:
> Szelethus wrote:
> > NoQ wrote:
> > > baloghadamsoftware wrote:
> > > > NoQ wrote:
> > > > > I'm opposed to this code for the same reason that i'm opposed to it 
> > > > > in the debug checker. Parent region is an undocumented implementation 
> > > > > detail of `RegionStore`. It is supposed to be immaterial to the user. 
> > > > > You should not rely on its exact value.
> > > > > 
> > > > > @baloghadamsoftware Can we eliminate all such code from iterator 
> > > > > checkers and instead identify all iterators by regions in which 
> > > > > they're stored? Does my improved C++ support help with this anyhow 
> > > > > whenever it kicks in?
> > > > How to find the region where it is stored? I am open to find better 
> > > > solutions, but it was the only one I could find so far. If we ignore 
> > > > `LazyCompoundVal` then everything falls apart, we can remove all the 
> > > > iterator-related checkers.
> > > It's the region from which you loaded it. If you obtained it as 
> > > `Call.getArgSVal()` then it's the parameter region for the call. If you 
> > > obtained it as `Call.getReturnValue()` then it's the target region can be 
> > > obtained by looking at the //construction context// for the call.
> > `LazyCompoundVal` and friends seem to be a constantly emerging headache for 
> > almost everyone. For how long I've spent in the analyzer, and have 
> > religiously studied conversations and your workbook about it, I still feel 
> > anxious whenever it comes up.
> > 
> > It would be great to have this documented in the code sometime.
> Do you mean `CallEvent::getParameterLocation()` for arguments? What is the 
> //construction context// for the call? How can it be obtained?
I do not know exactly how many place `LazyCompoundVal`  appears, but one place 
for sure is in `MaterializeTemporaryExpr::getSubExpr()`. What to use there 
instead?


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

https://reviews.llvm.org/D75677



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


[PATCH] D76696: [AST] Build recovery expressions by default for C++.

2020-03-26 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment.

Thanks for the test case. Reverted in 
https://github.com/llvm/llvm-project/commit/62dea6e9be31b100962f9ad41c1b79467a53f6cd
 for now. Adding the error-bit to type looks like a right direction.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76696



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


[PATCH] D76850: clang-format: Fix pointer alignment for overloaded operators (PR45107)

2020-03-26 Thread Krasimir Georgiev via Phabricator via cfe-commits
krasimir accepted this revision.
krasimir added a comment.

I think this is an improvement. Accepting with a nit. Thank you!




Comment at: clang/lib/Format/TokenAnnotator.cpp:2806
+  if (Previous->is(tok::identifier) || Previous->isSimpleTypeSpecifier()) {
+Previous = Previous->Previous;
+continue;

Consider using `Previous->getPreviousNonComment()` here and below to jump over 
comments.


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

https://reviews.llvm.org/D76850



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


[PATCH] D76850: clang-format: Fix pointer alignment for overloaded operators (PR45107)

2020-03-26 Thread Krasimir Georgiev via Phabricator via cfe-commits
krasimir added a comment.

@MyDeveloperDay : sorry, I must have not refreshed this page and didn't realize 
you already gave an LGTM until I posted the comment.


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

https://reviews.llvm.org/D76850



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


[PATCH] D74144: [OPENMP50]Add basic support for array-shaping operation.

2020-03-26 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev updated this revision to Diff 252859.
ABataev added a comment.

Rebase + fixes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74144

Files:
  clang/include/clang-c/Index.h
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/BuiltinTypes.def
  clang/include/clang/AST/ComputeDependence.h
  clang/include/clang/AST/ExprOpenMP.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/StmtNodes.td
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ComputeDependence.cpp
  clang/lib/AST/Expr.cpp
  clang/lib/AST/ExprClassification.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/NSAPI.cpp
  clang/lib/AST/StmtPrinter.cpp
  clang/lib/AST/StmtProfile.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypeLoc.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/SemaExceptionSpec.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTCommon.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/lib/StaticAnalyzer/Checkers/IdenticalExprChecker.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/test/OpenMP/depobj_messages.cpp
  clang/test/OpenMP/parallel_reduction_messages.c
  clang/test/OpenMP/task_ast_print.cpp
  clang/test/OpenMP/task_depend_messages.cpp
  clang/tools/libclang/CIndex.cpp
  clang/tools/libclang/CXCursor.cpp

Index: clang/tools/libclang/CXCursor.cpp
===
--- clang/tools/libclang/CXCursor.cpp
+++ clang/tools/libclang/CXCursor.cpp
@@ -423,6 +423,10 @@
 K = CXCursor_OMPArraySectionExpr;
 break;
 
+  case Stmt::OMPArrayShapingExprClass:
+K = CXCursor_OMPArrayShapingExpr;
+break;
+
   case Stmt::BinaryOperatorClass:
 K = CXCursor_BinaryOperator;
 break;
Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -5183,6 +5183,8 @@
 return cxstring::createRef("ArraySubscriptExpr");
   case CXCursor_OMPArraySectionExpr:
 return cxstring::createRef("OMPArraySectionExpr");
+  case CXCursor_OMPArrayShapingExpr:
+return cxstring::createRef("OMPArrayShapingExpr");
   case CXCursor_BinaryOperator:
 return cxstring::createRef("BinaryOperator");
   case CXCursor_CompoundAssignOperator:
Index: clang/test/OpenMP/task_depend_messages.cpp
===
--- clang/test/OpenMP/task_depend_messages.cpp
+++ clang/test/OpenMP/task_depend_messages.cpp
@@ -35,14 +35,14 @@
   #pragma omp task depend (source) // expected-error {{expected expression}} expected-warning {{missing ':' after dependency type - ignoring}} omp45-error {{expected 'in', 'out', 'inout' or 'mutexinoutset' in OpenMP clause 'depend'}} omp50-error {{expected 'in', 'out', 'inout', 'mutexinoutset' or 'depobj' in OpenMP clause 'depend'}}
   #pragma omp task depend (in : argc)) // expected-warning {{extra tokens at the end of '#pragma omp task' are ignored}}
   #pragma omp task depend (out: ) // expected-error {{expected expression}}
-  #pragma omp task depend (inout : foobool(argc)), depend (in, argc) // expected-error {{expected addressable lvalue expression, array element or array section}} expected-warning {{missing ':' after dependency type - ignoring}} expected-error {{expected expression}}
+  #pragma omp task depend (inout : foobool(argc)), depend (in, argc) // omp50-error {{expected addressable lvalue expression, array element, array section or array shaping expression}} omp45-error {{expected addressable lvalue expression, array element or array section}} expected-warning {{missing ':' after dependency type - ignoring}} expected-error {{expected expression}}
   #pragma omp task depend (out :S1) // expected-error {{'S1' does not refer to a value}}
   #pragma omp task depend(in : argv[1][1] = '2')
-  #pragma omp task depend (in : vec[1]) // expected-error {{expected addressable lvalue expression, array element or array section}}
+  #pragma omp task depend (in : vec[1]) // omp50-error {{expected addressable lvalue expression, array element, array section or array shaping expression}} omp45-error {{expected addressable lvalue expression, array element or array section}}
   #pragma omp task depend (in : argv[0])
   #pragma omp task depend (in : ) // expected-error {{expected expression}}
   #pragma omp task depend (in : main)
-  #pragma omp task depend(in : a[0]) // expected-error{{expected addressable lvalue expression, array element or array section}}
+  #pragma omp task depend(in : a[0]) // omp50-error 

[PATCH] D75360: [analyzer][NFC] Tie CheckerRegistry to CheckerManager, allow CheckerManager to be constructed for non-analysis purposes

2020-03-26 Thread Kristóf Umann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG2aac0c47aed8: Reland "[analyzer][NFC] Tie 
CheckerRegistry to CheckerManager, allow… (authored by Szelethus).

Changed prior to commit:
  https://reviews.llvm.org/D75360?vs=252073&id=252861#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75360

Files:
  clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
  clang/include/clang/StaticAnalyzer/Frontend/AnalysisConsumer.h
  clang/include/clang/StaticAnalyzer/Frontend/AnalyzerHelpFlags.h
  clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistration.h
  clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
  clang/include/clang/StaticAnalyzer/Frontend/FrontendActions.h
  clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
  clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  clang/lib/StaticAnalyzer/Frontend/AnalyzerHelpFlags.cpp
  clang/lib/StaticAnalyzer/Frontend/CMakeLists.txt
  clang/lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
  clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
  clang/lib/StaticAnalyzer/Frontend/CreateCheckerManager.cpp

Index: clang/lib/StaticAnalyzer/Frontend/CreateCheckerManager.cpp
===
--- /dev/null
+++ clang/lib/StaticAnalyzer/Frontend/CreateCheckerManager.cpp
@@ -0,0 +1,52 @@
+//===- CheckerManager.h - Static Analyzer Checker Manager ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// Defines the Static Analyzer Checker Manager.
+//
+//===--===//
+
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Frontend/CheckerRegistry.h"
+#include 
+
+namespace clang {
+namespace ento {
+
+CheckerManager::CheckerManager(
+ASTContext &Context, AnalyzerOptions &AOptions,
+ArrayRef plugins,
+ArrayRef> checkerRegistrationFns)
+: Context(&Context), LangOpts(Context.getLangOpts()), AOptions(AOptions),
+  Diags(Context.getDiagnostics()),
+  Registry(
+  std::make_unique(plugins, Context.getDiagnostics(),
+AOptions, checkerRegistrationFns)) {
+  Registry->initializeRegistry(*this);
+  Registry->initializeManager(*this);
+  finishedCheckerRegistration();
+}
+
+/// Constructs a CheckerManager without requiring an AST. No checker
+/// registration will take place. Only useful for retrieving the
+/// CheckerRegistry and print for help flags where the AST is unavalaible.
+CheckerManager::CheckerManager(AnalyzerOptions &AOptions,
+   const LangOptions &LangOpts,
+   DiagnosticsEngine &Diags,
+   ArrayRef plugins)
+: LangOpts(LangOpts), AOptions(AOptions), Diags(Diags),
+  Registry(std::make_unique(plugins, Diags, AOptions)) {
+  Registry->initializeRegistry(*this);
+}
+
+CheckerManager::~CheckerManager() {
+  for (const auto &CheckerDtor : CheckerDtors)
+CheckerDtor();
+}
+
+} // namespace ento
+} // namespace clang
Index: clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
===
--- clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
+++ clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
@@ -109,9 +109,9 @@
 
 CheckerRegistry::CheckerRegistry(
 ArrayRef Plugins, DiagnosticsEngine &Diags,
-AnalyzerOptions &AnOpts, const LangOptions &LangOpts,
+AnalyzerOptions &AnOpts,
 ArrayRef> CheckerRegistrationFns)
-: Diags(Diags), AnOpts(AnOpts), LangOpts(LangOpts) {
+: Diags(Diags), AnOpts(AnOpts) {
 
   // Register builtin checkers.
 #define GET_CHECKERS
@@ -179,12 +179,16 @@
   addDependency(FULLNAME, DEPENDENCY);
 
 #define GET_CHECKER_OPTIONS
-#define CHECKER_OPTION(TYPE, FULLNAME, CMDFLAG, DESC, DEFAULT_VAL, DEVELOPMENT_STATUS, IS_HIDDEN)  \
-  addCheckerOption(TYPE, FULLNAME, CMDFLAG, DEFAULT_VAL, DESC, DEVELOPMENT_STATUS, IS_HIDDEN);
+#define CHECKER_OPTION(TYPE, FULLNAME, CMDFLAG, DESC, DEFAULT_VAL, \
+   DEVELOPMENT_STATUS, IS_HIDDEN)  \
+  addCheckerOption(TYPE, FULLNAME, CMDFLAG, DEFAULT_VAL, DESC, \
+   DEVELOPMENT_STATUS, IS_HIDDEN);
 
 #define GET_PACKAGE_OPTIONS
-#define PACKAGE_OPTION(TYPE, FULLNAME, CMDFLAG, DESC, DEFAULT_VAL, DEVELOPMENT_STATUS, IS_HIDDEN)  \
-  addPackageOption(TYPE, FULLNAME, CMDFLAG, DEFAULT_VAL, DESC, DEVELOPMENT_STATUS, IS_HIDDEN);
+#define PACKAGE_OPTION(TYPE, FULLNAME, CMDFLAG, DESC, DEFAU

[clang] 0766d1d - Make a windows buildbot happy

2020-03-26 Thread Kirstóf Umann via cfe-commits

Author: Kirstóf Umann
Date: 2020-03-26T17:04:23+01:00
New Revision: 0766d1dca8685e77e8672b13f26797a5d4c8b4d7

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

LOG: Make a windows buildbot happy

Added: 


Modified: 
clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h

Removed: 




diff  --git a/clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h 
b/clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
index dde2409ed72c..73594bb8b380 100644
--- a/clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
+++ b/clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
@@ -241,8 +241,9 @@ class CheckerRegistry {
   bool IsHidden = false) {
 // Avoid MSVC's Compiler Error C2276:
 // http://msdn.microsoft.com/en-us/library/850cstw1(v=VS.80).aspx
-addChecker(&initializeManager, &returnTrue, FullName,
-   Desc, DocsUri, IsHidden);
+addChecker(&CheckerRegistry::initializeManager,
+   &CheckerRegistry::returnTrue, FullName, Desc, DocsUri,
+   IsHidden);
   }
 
   /// Makes the checker with the full name \p fullName depends on the checker



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


[PATCH] D75677: [Analyzer] Only add iterator note tags to the operations of the affected iterators

2020-03-26 Thread Balogh, Ádám via Phabricator via cfe-commits
baloghadamsoftware marked an inline comment as done.
baloghadamsoftware added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp:541-542
+BR.markInteresting(It1);
+if (const auto &LCV1 = It1.getAs()) {
+  BR.markInteresting(LCV1->getRegion());
+}

baloghadamsoftware wrote:
> baloghadamsoftware wrote:
> > Szelethus wrote:
> > > NoQ wrote:
> > > > baloghadamsoftware wrote:
> > > > > NoQ wrote:
> > > > > > I'm opposed to this code for the same reason that i'm opposed to it 
> > > > > > in the debug checker. Parent region is an undocumented 
> > > > > > implementation detail of `RegionStore`. It is supposed to be 
> > > > > > immaterial to the user. You should not rely on its exact value.
> > > > > > 
> > > > > > @baloghadamsoftware Can we eliminate all such code from iterator 
> > > > > > checkers and instead identify all iterators by regions in which 
> > > > > > they're stored? Does my improved C++ support help with this anyhow 
> > > > > > whenever it kicks in?
> > > > > How to find the region where it is stored? I am open to find better 
> > > > > solutions, but it was the only one I could find so far. If we ignore 
> > > > > `LazyCompoundVal` then everything falls apart, we can remove all the 
> > > > > iterator-related checkers.
> > > > It's the region from which you loaded it. If you obtained it as 
> > > > `Call.getArgSVal()` then it's the parameter region for the call. If you 
> > > > obtained it as `Call.getReturnValue()` then it's the target region can 
> > > > be obtained by looking at the //construction context// for the call.
> > > `LazyCompoundVal` and friends seem to be a constantly emerging headache 
> > > for almost everyone. For how long I've spent in the analyzer, and have 
> > > religiously studied conversations and your workbook about it, I still 
> > > feel anxious whenever it comes up.
> > > 
> > > It would be great to have this documented in the code sometime.
> > Do you mean `CallEvent::getParameterLocation()` for arguments? What is the 
> > //construction context// for the call? How can it be obtained?
> I do not know exactly how many place `LazyCompoundVal`  appears, but one 
> place for sure is in `MaterializeTemporaryExpr::getSubExpr()`. What to use 
> there instead?
I also get it in the `Val` parameter of `checkBind()`.


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

https://reviews.llvm.org/D75677



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


[PATCH] D74144: [OPENMP50]Add basic support for array-shaping operation.

2020-03-26 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.

Thanks, LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74144



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


[PATCH] D76830: [Analyzer][MallocChecker] No warning for kfree of ZERO_SIZE_PTR.

2020-03-26 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment.

In D76830#1943133 , @balazske wrote:

> FIXME: There is a test file "kmalloc-linux.c" but it seems to be 
> non-maintained and buggy (has no //-verify// option so it passes always but 
> produces multiple warnings).


Crap, even I did some changes on that file, and never noticed the lack of 
verify, or the lack of `-analyzer-checker` flags.




Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1687
+  if (ArgValKnown) {
+if (!KernelZeroSizePtrValue)
+  KernelZeroSizePtrValue =

martong wrote:
> balazske wrote:
> > martong wrote:
> > > martong wrote:
> > > > This is a bit confusing for me. Perhaps alternatively we could have a 
> > > > free function `isInitialized(KernelZero...)` instead. Or maybe having a 
> > > > separate bool variable to indicate whether it was initialized could be 
> > > > cleaner?
> > > Another idea: Adding a helper struct to contain the bool `initialized`? 
> > > E.g. (draft):
> > > ```
> > > struct LazyOptional {
> > >   bool initialized = false;
> > >   Opt value;
> > >   Opt& get();
> > >   void set(const Opt&);
> > > };
> > > ```
> > It may be OK to have a function `lazyInitKernelZeroSizePtrValue`?
> I don't insist, given we have a better described type for 
> `KernelZeroSizePtrValue` (e.g. having a `using` `template`)
`AnalysisManager` has access to the `Preprocessor`, and it is also responsible 
for the construction of the `CheckerManager` object. This would make moving 
such code to the checker registry function! I'll gladly take this issue off 
your hand and patch it in once rG2aac0c47aed8d1eff7ab6d858173c532b881d948 
settles :)



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1690-1700
+  // If there is a macro called ZERO_SIZE_PTR, it could be a kernel source code
+  // and this value indicates a special value used for a zero-sized memory
+  // block. It is a constant value that is allowed to be freed.
+  const llvm::APSInt *ArgValKnown =
+  C.getSValBuilder().getKnownValue(State, ArgVal);
+  if (ArgValKnown) {
+initKernelZeroSizePtrValue(C.getPreprocessor());

I found this article on the subject:

https://lwn.net/Articles/236920/

> That brings us to the third possibility: this patch from Christoph Lameter 
> which causes kmalloc(0) to return a special ZERO_SIZE_PTR value. It is a 
> non-NULL value which looks like a legitimate pointer, but which causes a 
> fault on any attempt at dereferencing it. Any attempt to call kfree() with 
> this special value will do the right thing, of course.

But I would argue that using `delete` on such a pointer might still be a sign 
of code smell. Granted, if the source code is hacking the kernel this is very 
unlikely, but still, I think this code should be placed...



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1707-1708
   if (!R) {
 ReportBadFree(C, ArgVal, ArgExpr->getSourceRange(), ParentExpr, Family);
 return nullptr;
   }

...here!

```lang=cpp
bool isArgZERO_SIZE_PTR(ArgVal) const {
  const llvm::APSInt *ArgValKnown =
  C.getSValBuilder().getKnownValue(State, ArgVal);
  if (ArgValKnown) {
initKernelZeroSizePtrValue(C.getPreprocessor());
if (*KernelZeroSizePtrValue &&
ArgValKnown->getSExtValue() == **KernelZeroSizePtrValue)
  return true;
  }
  return false;
}

// ...

if (ArgVal has no associated MemRegion)
  // If there is a macro called ZERO_SIZE_PTR, it could be a kernel source code
  // and this value indicates a special value used for a zero-sized memory
  // block. It is a constant value that is allowed to be freed.
  if (!isArgZERO_SIZE_PTR(ArgVal)
ReportBadFree(C, ArgVal, ArgExpr->getSourceRange(), ParentExpr, Family);
return nullptr;
  }
  // Still check for incorrect deallocator usage, etc.
}
```

Or something like this. WDYT?



Comment at: clang/test/Analysis/kmalloc-linux-1.c:15
+
+// FIXME: malloc checker expects kfree with 2 arguments, is this correct?
+// (recent kernel source code contains a `kfree(const void *)`)

martong wrote:
> Do you plan to sniff around a bit about the arguments (as part of another 
> patch)? Would be good to handle both old and new signature kernel allocation 
> functions.
I'll take a quick look as well, I am quite familiar with MallocChecker.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76830



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


[PATCH] D76856: Fix TBAA for unsigned fixed-point types

2020-03-26 Thread mattias.v.eriks...@ericsson.com via Phabricator via cfe-commits
materi created this revision.
materi added reviewers: ebevhan, leonardchan.
Herald added a subscriber: kosarev.
Herald added a project: clang.

Unsigned types can alias the corresponding signed types. I don't see
that this is explicitly mentioned in the Embedded-C specification, but
I think it should work the same as for the integer types.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76856

Files:
  clang/lib/CodeGen/CodeGenTBAA.cpp
  clang/test/CodeGen/fixed-point-tbaa.c

Index: clang/test/CodeGen/fixed-point-tbaa.c
===
--- /dev/null
+++ clang/test/CodeGen/fixed-point-tbaa.c
@@ -0,0 +1,109 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -O1 -ffixed-point %s -emit-llvm -o - | FileCheck %s  -check-prefixes=CHECK
+//
+// Check that we generate correct TBAA metadata for fixed-point types.
+
+void sfract(unsigned short _Fract *p, short _Fract *q,
+unsigned _Sat short _Fract *r, _Sat short _Fract *s) {
+  // CHECK-LABEL: define void @sfract
+  // CHECK: store i8 -128, i8* %p, align 1, !tbaa [[TAG_sf:!.*]]
+  // CHECK: store i8 -64, i8* %q, align 1, !tbaa [[TAG_sf]]
+  // CHECK: store i8 -128, i8* %r, align 1, !tbaa [[TAG_sat_sf:!.*]]
+  // CHECK: store i8 -64, i8* %s, align 1, !tbaa [[TAG_sat_sf]]
+  *p = 0.5hur;
+  *q = -0.5hr;
+  *r = 0.5hur;
+  *s = -0.5hr;
+}
+
+void fract(unsigned _Fract *p, _Fract *q,
+   unsigned _Sat _Fract *r, _Sat _Fract *s) {
+  // CHECK-LABEL: define void @fract
+  // CHECK: store i16 -32768, i16* %p, align 2, !tbaa [[TAG_f:!.*]]
+  // CHECK: store i16 -16384, i16* %q, align 2, !tbaa [[TAG_f]]
+  // CHECK: store i16 -32768, i16* %r, align 2, !tbaa [[TAG_sat_f:!.*]]
+  // CHECK: store i16 -16384, i16* %s, align 2, !tbaa [[TAG_sat_f]]
+  *p = 0.5ur;
+  *q = -0.5r;
+  *r = 0.5ur;
+  *s = -0.5r;
+}
+
+void lfract(unsigned long _Fract *p, long _Fract *q,
+unsigned _Sat long _Fract *r, _Sat long _Fract *s) {
+  // CHECK-LABEL: define void @lfract
+  // CHECK: store i32 -2147483648, i32* %p, align 4, !tbaa [[TAG_lf:!.*]]
+  // CHECK: store i32 -1073741824, i32* %q, align 4, !tbaa [[TAG_lf]]
+  // CHECK: store i32 -2147483648, i32* %r, align 4, !tbaa [[TAG_sat_lf:!.*]]
+  // CHECK: store i32 -1073741824, i32* %s, align 4, !tbaa [[TAG_sat_lf]]
+  *p = 0.5ulr;
+  *q = -0.5lr;
+  *r = 0.5ulr;
+  *s = -0.5lr;
+}
+
+void saccum(unsigned short _Accum *p, short _Accum *q,
+unsigned _Sat short _Accum *r, _Sat short _Accum *s) {
+  // CHECK-LABEL: define void @saccum
+  // CHECK: store i16 128, i16* %p, align 2, !tbaa [[TAG_sk:!.*]]
+  // CHECK: store i16 -64, i16* %q, align 2, !tbaa [[TAG_sk]]
+  // CHECK: store i16 128, i16* %r, align 2, !tbaa [[TAG_sat_sk:!.*]]
+  // CHECK: store i16 -64, i16* %s, align 2, !tbaa [[TAG_sat_sk]]
+  *p = 0.5huk;
+  *q = -0.5hk;
+  *r = 0.5huk;
+  *s = -0.5hk;
+}
+
+void accum(unsigned _Accum *p, _Accum *q,
+   unsigned _Sat _Accum *r, _Sat _Accum *s) {
+  // CHECK-LABEL: define void @accum
+  // CHECK: store i32 32768, i32* %p, align 4, !tbaa [[TAG_k:!.*]]
+  // CHECK: store i32 -16384, i32* %q, align 4, !tbaa [[TAG_k]]
+  // CHECK: store i32 32768, i32* %r, align 4, !tbaa [[TAG_sat_k:!.*]]
+  // CHECK: store i32 -16384, i32* %s, align 4, !tbaa [[TAG_sat_k]]
+  *p = 0.5uk;
+  *q = -0.5k;
+  *r = 0.5uk;
+  *s = -0.5k;
+}
+
+void laccum(unsigned long _Accum *p, long _Accum *q,
+unsigned _Sat long _Accum *r, _Sat long _Accum *s) {
+  // CHECK-LABEL: define void @laccum
+  // CHECK: store i64 2147483648, i64* %p, align 8, !tbaa [[TAG_lk:!.*]]
+  // CHECK: store i64 -1073741824, i64* %q, align 8, !tbaa [[TAG_lk]]
+  // CHECK: store i64 2147483648, i64* %r, align 8, !tbaa [[TAG_sat_lk:!.*]]
+  // CHECK: store i64 -1073741824, i64* %s, align 8, !tbaa [[TAG_sat_lk]]
+  *p = 0.5ulk;
+  *q = -0.5lk;
+  *r = 0.5ulk;
+  *s = -0.5lk;
+}
+
+// CHECK-DAG: [[TAG_sf]] = !{[[TYPE_sf:!.*]], [[TYPE_sf]], i64 0}
+// CHECK-DAG: [[TYPE_sf]] = !{!"short _Fract"
+// CHECK-DAG: [[TAG_f]] = !{[[TYPE_f:!.*]], [[TYPE_f]], i64 0}
+// CHECK-DAG: [[TYPE_f]] = !{!"_Fract"
+// CHECK-DAG: [[TAG_lf]] = !{[[TYPE_lf:!.*]], [[TYPE_lf]], i64 0}
+// CHECK-DAG: [[TYPE_lf]] = !{!"long _Fract"
+
+// CHECK-DAG: [[TAG_sat_sf]] = !{[[TYPE_sat_sf:!.*]], [[TYPE_sat_sf]], i64 0}
+// CHECK-DAG: [[TYPE_sat_sf]] = !{!"_Sat short _Fract"
+// CHECK-DAG: [[TAG_sat_f]] = !{[[TYPE_sat_f:!.*]], [[TYPE_sat_f]], i64 0}
+// CHECK-DAG: [[TYPE_sat_f]] = !{!"_Sat _Fract"
+// CHECK-DAG: [[TAG_sat_lf]] = !{[[TYPE_sat_lf:!.*]], [[TYPE_sat_lf]], i64 0}
+// CHECK-DAG: [[TYPE_sat_lf]] = !{!"_Sat long _Fract"
+
+// CHECK-DAG: [[TAG_sk]] = !{[[TYPE_sk:!.*]], [[TYPE_sk]], i64 0}
+// CHECK-DAG: [[TYPE_sk]] = !{!"short _Accum"
+// CHECK-DAG: [[TAG_k]] = !{[[TYPE_k:!.*]], [[TYPE_k]], i64 0}
+// CHECK-DAG: [[TYPE_k]] = !{!"_Accum"
+// CHECK-DAG: [[TAG_lk]] = !{[[TYPE_lk:!.*]], [[TYPE_lk]], i64 0}
+// CHECK-DAG: [[TYPE_lk]] = !{!"long _Accum"
+
+// CHECK-DAG: [[TAG_sat_sk]] = !{[[TYPE_sat_sk:!

[PATCH] D76140: [InlineFunction] update attributes during inlining

2020-03-26 Thread Anna Thomas via Phabricator via cfe-commits
anna updated this revision to Diff 252868.
anna added a comment.

NFC w.r.t prev diff. Use VMap.lookup instead of a lambda which does the same.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76140

Files:
  clang/test/CodeGen/builtins-systemz-zvector.c
  clang/test/CodeGen/builtins-systemz-zvector2.c
  clang/test/CodeGen/movbe-builtins.c
  clang/test/CodeGen/rot-intrinsics.c
  llvm/lib/Transforms/Utils/InlineFunction.cpp
  llvm/test/Transforms/Inline/ret_attr_update.ll

Index: llvm/test/Transforms/Inline/ret_attr_update.ll
===
--- /dev/null
+++ llvm/test/Transforms/Inline/ret_attr_update.ll
@@ -0,0 +1,112 @@
+; RUN: opt < %s -inline-threshold=0 -always-inline -S | FileCheck %s
+; RUN: opt < %s -passes=always-inline -S | FileCheck %s
+
+declare i8* @foo(i8*) argmemonly nounwind
+
+define i8* @callee(i8 *%p) alwaysinline {
+; CHECK: @callee(
+; CHECK: call i8* @foo(i8* noalias %p)
+  %r = call i8* @foo(i8* noalias %p)
+  ret i8* %r
+}
+
+define i8* @caller(i8* %ptr, i64 %x) {
+; CHECK-LABEL: @caller
+; CHECK: call nonnull i8* @foo(i8* noalias
+  %gep = getelementptr inbounds i8, i8* %ptr, i64 %x
+  %p = call nonnull i8* @callee(i8* %gep)
+  ret i8* %p
+}
+
+declare void @llvm.experimental.guard(i1,...)
+; Cannot add nonnull attribute to foo
+; because the guard is a throwing call
+define internal i8* @callee_with_throwable(i8* %p) alwaysinline {
+; CHECK-NOT: callee_with_throwable
+  %r = call i8* @foo(i8* %p)
+  %cond = icmp ne i8* %r, null
+  call void (i1, ...) @llvm.experimental.guard(i1 %cond) [ "deopt"() ]
+  ret i8* %r
+}
+
+declare i8* @bar(i8*) readonly nounwind
+; Here also we cannot add nonnull attribute to the call bar.
+define internal i8* @callee_with_explicit_control_flow(i8* %p) alwaysinline {
+; CHECK-NOT: callee_with_explicit_control_flow
+  %r = call i8* @bar(i8* %p)
+  %cond = icmp ne i8* %r, null
+  br i1 %cond, label %ret, label %orig
+
+ret:
+  ret i8* %r
+
+orig:
+  ret i8* %p
+}
+
+define i8* @caller2(i8* %ptr, i64 %x, i1 %cond) {
+; CHECK-LABEL: @caller2
+; CHECK: call i8* @foo
+; CHECK: call i8* @bar
+  %gep = getelementptr inbounds i8, i8* %ptr, i64 %x
+  %p = call nonnull i8* @callee_with_throwable(i8* %gep)
+  %q = call nonnull i8* @callee_with_explicit_control_flow(i8* %gep)
+  br i1 %cond, label %pret, label %qret
+
+pret:
+  ret i8* %p
+
+qret:
+  ret i8* %q
+}
+
+define internal i8* @callee3(i8 *%p) alwaysinline {
+; CHECK-NOT: callee3
+  %r = call noalias i8* @foo(i8* %p)
+  ret i8* %r
+}
+
+; add the deref attribute to the existing attributes on foo.
+define i8* @caller3(i8* %ptr, i64 %x) {
+; CHECK-LABEL: caller3
+; CHECK: call noalias dereferenceable_or_null(12) i8* @foo
+  %gep = getelementptr inbounds i8, i8* %ptr, i64 %x
+  %p = call dereferenceable_or_null(12) i8* @callee3(i8* %gep)
+  ret i8* %p
+}
+
+declare i8* @inf_loop_call(i8*) nounwind
+; We cannot propagate attributes to foo because we do not know whether inf_loop_call
+; will return execution.
+define internal i8* @callee_with_sideeffect_callsite(i8* %p) alwaysinline {
+; CHECK-NOT: callee_with_sideeffect_callsite
+  %r = call i8* @foo(i8* %p)
+  %v = call i8* @inf_loop_call(i8* %p)
+  ret i8* %r
+}
+
+; do not add deref attribute to foo
+define i8* @test4(i8* %ptr, i64 %x) {
+; CHECK-LABEL: test4
+; CHECK: call i8* @foo
+  %gep = getelementptr inbounds i8, i8* %ptr, i64 %x
+  %p = call dereferenceable_or_null(12) i8* @callee_with_sideeffect_callsite(i8* %gep)
+  ret i8* %p
+}
+
+declare i8* @baz(i8*) nounwind readonly
+define internal i8* @callee5(i8* %p) alwaysinline {
+; CHECK-NOT: callee5
+  %r = call i8* @foo(i8* %p)
+  %v = call i8* @baz(i8* %p)
+  ret i8* %r
+}
+
+; add the deref attribute to foo.
+define i8* @test5(i8* %ptr, i64 %x) {
+; CHECK-LABEL: test5
+; CHECK: call dereferenceable_or_null(12) i8* @foo
+  %gep = getelementptr inbounds i8, i8* %ptr, i64 %x
+  %s = call dereferenceable_or_null(12) i8* @callee5(i8* %gep)
+  ret i8* %s
+}
Index: llvm/lib/Transforms/Utils/InlineFunction.cpp
===
--- llvm/lib/Transforms/Utils/InlineFunction.cpp
+++ llvm/lib/Transforms/Utils/InlineFunction.cpp
@@ -80,11 +80,21 @@
   cl::Hidden,
   cl::desc("Convert noalias attributes to metadata during inlining."));
 
+static cl::opt UpdateReturnAttributes(
+"update-return-attrs", cl::init(true), cl::Hidden,
+cl::desc("Update return attributes on calls within inlined body"));
+
 static cl::opt
 PreserveAlignmentAssumptions("preserve-alignment-assumptions-during-inlining",
   cl::init(true), cl::Hidden,
   cl::desc("Convert align attributes to assumptions during inlining."));
 
+static cl::opt MaxInstCheckedForThrow(
+"max-inst-checked-for-throw-during-inlining", cl::Hidden,
+cl::desc("the maximum number of instructions analyzed for may throw during "
+ "attribute inference in inline

[PATCH] D76696: [AST] Build recovery expressions by default for C++.

2020-03-26 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added a comment.

We have also encountered crashes in downstream testing caused by this using 
just the vanilla source from trunk. When there is a proposed fix, please let us 
know so we can test. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76696



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


[PATCH] D76830: [Analyzer][MallocChecker] No warning for kfree of ZERO_SIZE_PTR.

2020-03-26 Thread Balázs Kéri via Phabricator via cfe-commits
balazske marked an inline comment as done.
balazske added inline comments.



Comment at: clang/test/Analysis/kmalloc-linux-1.c:15
+
+// FIXME: malloc checker expects kfree with 2 arguments, is this correct?
+// (recent kernel source code contains a `kfree(const void *)`)

Szelethus wrote:
> martong wrote:
> > Do you plan to sniff around a bit about the arguments (as part of another 
> > patch)? Would be good to handle both old and new signature kernel 
> > allocation functions.
> I'll take a quick look as well, I am quite familiar with MallocChecker.
I found that `kfree` has one argument, not two (even in the 2.6 kernel). 
Probably argument count was wrong in the last change when `CallDescription` was 
introduced.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76830



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


[clang] 40076c1 - CUDA: Fix broken test run lines

2020-03-26 Thread Matt Arsenault via cfe-commits

Author: Matt Arsenault
Date: 2020-03-26T12:19:34-04:00
New Revision: 40076c14fef509d0304cbdad49ba64113f4816fd

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

LOG: CUDA: Fix broken test run lines

There was a misisng space between the -march and --cuda-gpu-arch
arguments, so --cuda-gpu-arch wasn't actually being parsed. I'm not
sure what the intent of the sm_10 run lines were, but they error as an
unsupported architecture. Switch these to something else.

Added: 


Modified: 
clang/test/Driver/cuda-flush-denormals-to-zero.cu

Removed: 




diff  --git a/clang/test/Driver/cuda-flush-denormals-to-zero.cu 
b/clang/test/Driver/cuda-flush-denormals-to-zero.cu
index 84ef358758fd..74f4bbc1585e 100644
--- a/clang/test/Driver/cuda-flush-denormals-to-zero.cu
+++ b/clang/test/Driver/cuda-flush-denormals-to-zero.cu
@@ -2,10 +2,10 @@
 // -fcuda-flush-denormals-to-zero. This should be translated to
 // -fdenormal-fp-math-f32=preserve-sign
 
-// RUN: %clang -no-canonical-prefixes -### -target x86_64-linux-gnu -c 
-march=haswell--cuda-gpu-arch=sm_20 -fcuda-flush-denormals-to-zero -nocudainc 
-nocudalib %s 2>&1 | FileCheck -check-prefix=FTZ %s
-// RUN: %clang -no-canonical-prefixes -### -target x86_64-linux-gnu -c 
-march=haswell--cuda-gpu-arch=sm_20 -fno-cuda-flush-denormals-to-zero 
-nocudainc -nocudalib %s 2>&1 | FileCheck -check-prefix=NOFTZ %s
-// RUN: %clang -no-canonical-prefixes -### -target x86_64-linux-gnu -c 
-march=haswell--cuda-gpu-arch=sm_10 -fcuda-flush-denormals-to-zero -nocudainc 
-nocudalib %s 2>&1 | FileCheck -check-prefix=FTZ %s
-// RUN: %clang -no-canonical-prefixes -### -target x86_64-linux-gnu -c 
-march=haswell--cuda-gpu-arch=sm_10 -fno-cuda-flush-denormals-to-zero 
-nocudainc -nocudalib %s 2>&1 | FileCheck -check-prefix=NOFTZ %s
+// RUN: %clang -no-canonical-prefixes -### -target x86_64-linux-gnu -c 
-march=haswell --cuda-gpu-arch=sm_20 -fcuda-flush-denormals-to-zero -nocudainc 
-nocudalib %s 2>&1 | FileCheck -check-prefix=FTZ %s
+// RUN: %clang -no-canonical-prefixes -### -target x86_64-linux-gnu -c 
-march=haswell --cuda-gpu-arch=sm_20 -fno-cuda-flush-denormals-to-zero 
-nocudainc -nocudalib %s 2>&1 | FileCheck -check-prefix=NOFTZ %s
+// RUN: %clang -no-canonical-prefixes -### -target x86_64-linux-gnu -c 
-march=haswell --cuda-gpu-arch=sm_70 -fcuda-flush-denormals-to-zero -nocudainc 
-nocudalib %s 2>&1 | FileCheck -check-prefix=FTZ %s
+// RUN: %clang -no-canonical-prefixes -### -target x86_64-linux-gnu -c 
-march=haswell --cuda-gpu-arch=sm_70 -fno-cuda-flush-denormals-to-zero 
-nocudainc -nocudalib %s 2>&1 | FileCheck -check-prefix=NOFTZ %s
 
 // CPUFTZ-NOT: -fdenormal-fp-math
 



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


[clang] 4dc8472 - [analyzer] Add the Preprocessor to CheckerManager

2020-03-26 Thread Kirstóf Umann via cfe-commits

Author: Kirstóf Umann
Date: 2020-03-26T17:29:52+01:00
New Revision: 4dc8472942ce60f29de9587b9451cc3d49df7abf

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

LOG: [analyzer] Add the Preprocessor to CheckerManager

Added: 


Modified: 
clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp

clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
clang/lib/StaticAnalyzer/Frontend/CreateCheckerManager.cpp
clang/unittests/StaticAnalyzer/Reusables.h

Removed: 




diff  --git a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h 
b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
index 4635ca35736a..f34f5c239290 100644
--- a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
+++ b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
@@ -122,9 +122,10 @@ enum class ObjCMessageVisitKind {
 };
 
 class CheckerManager {
-  ASTContext *Context;
+  ASTContext *Context = nullptr;
   const LangOptions LangOpts;
-  AnalyzerOptions &AOptions;
+  const AnalyzerOptions &AOptions;
+  const Preprocessor *PP = nullptr;
   CheckerNameRef CurrentCheckerName;
   DiagnosticsEngine &Diags;
   std::unique_ptr Registry;
@@ -137,15 +138,16 @@ class CheckerManager {
   // dependencies look like this: Core -> Checkers -> Frontend.
 
   CheckerManager(
-  ASTContext &Context, AnalyzerOptions &AOptions,
+  ASTContext &Context, AnalyzerOptions &AOptions, const Preprocessor &PP,
   ArrayRef plugins,
   ArrayRef> checkerRegistrationFns);
 
   /// Constructs a CheckerManager that ignores all non TblGen-generated
   /// checkers. Useful for unit testing, unless the checker infrastructure
   /// itself is tested.
-  CheckerManager(ASTContext &Context, AnalyzerOptions &AOptions)
-  : CheckerManager(Context, AOptions, {}, {}) {}
+  CheckerManager(ASTContext &Context, AnalyzerOptions &AOptions,
+ const Preprocessor &PP)
+  : CheckerManager(Context, AOptions, PP, {}, {}) {}
 
   /// Constructs a CheckerManager without requiring an AST. No checker
   /// registration will take place. Only useful for retrieving the
@@ -163,7 +165,11 @@ class CheckerManager {
   void finishedCheckerRegistration();
 
   const LangOptions &getLangOpts() const { return LangOpts; }
-  AnalyzerOptions &getAnalyzerOptions() const { return AOptions; }
+  const AnalyzerOptions &getAnalyzerOptions() const { return AOptions; }
+  const Preprocessor &getPreprocessor() const {
+assert(PP);
+return *PP;
+  }
   const CheckerRegistry &getCheckerRegistry() const { return *Registry; }
   DiagnosticsEngine &getDiagnostics() const { return Diags; }
   ASTContext &getASTContext() const {

diff  --git 
a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
index 6f8cb1432bb1..1af93cc45363 100644
--- 
a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
+++ 
b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
@@ -1485,7 +1485,7 @@ bool ento::shouldRegisterRetainCountBase(const 
LangOptions &LO) {
 // it should be possible to enable the NS/CF retain count checker as
 // osx.cocoa.RetainCount, and it should be possible to disable
 // osx.OSObjectRetainCount using osx.cocoa.RetainCount:CheckOSObject=false.
-static bool getOption(AnalyzerOptions &Options,
+static bool getOption(const AnalyzerOptions &Options,
   StringRef Postfix,
   StringRef Value) {
   auto I = Options.Config.find(

diff  --git 
a/clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
 
b/clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
index 14f551403d98..562d553b8a84 100644
--- 
a/clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
+++ 
b/clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
@@ -608,7 +608,7 @@ std::string clang::ento::getVariableName(const FieldDecl 
*Field) {
 void ento::registerUninitializedObjectChecker(CheckerManager &Mgr) {
   auto Chk = Mgr.registerChecker();
 
-  AnalyzerOptions &AnOpts = Mgr.getAnalyzerOptions();
+  const AnalyzerOptions &AnOpts = Mgr.getAnalyzerOptions();
   UninitObjCheckerOptions &ChOpts = Chk->Opts;
 
   ChOpts.IsPedantic = AnOpts.getCheckerBooleanOption(Chk, "Pedantic");

diff  --git a/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp 
b/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
index 6c123521e5f8..74c689730e58 100644
--- a/clang/lib/StaticAnalyzer/Frontend/Ana

[PATCH] D76862: HIP: Ensure new denormal mode attributes are set

2020-03-26 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm created this revision.
arsenm added reviewers: yaxunl, gregrodgers.
Herald added subscribers: kerbowa, tpr, nhaehnle, wdng, jvesely.

Apparently HIPToolChain does not subclass from AMDGPUToolChain, so
this was not applying the new denormal attributes. I'm not sure why
this doesn't subclass. Just copy the implementation for now.


https://reviews.llvm.org/D76862

Files:
  clang/lib/Driver/ToolChains/AMDGPU.cpp
  clang/lib/Driver/ToolChains/AMDGPU.h
  clang/lib/Driver/ToolChains/HIP.cpp
  clang/lib/Driver/ToolChains/HIP.h
  clang/test/Driver/cuda-flush-denormals-to-zero.cu

Index: clang/test/Driver/cuda-flush-denormals-to-zero.cu
===
--- clang/test/Driver/cuda-flush-denormals-to-zero.cu
+++ clang/test/Driver/cuda-flush-denormals-to-zero.cu
@@ -7,6 +7,16 @@
 // RUN: %clang -no-canonical-prefixes -### -target x86_64-linux-gnu -c -march=haswell --cuda-gpu-arch=sm_70 -fcuda-flush-denormals-to-zero -nocudainc -nocudalib %s 2>&1 | FileCheck -check-prefix=FTZ %s
 // RUN: %clang -no-canonical-prefixes -### -target x86_64-linux-gnu -c -march=haswell --cuda-gpu-arch=sm_70 -fno-cuda-flush-denormals-to-zero -nocudainc -nocudalib %s 2>&1 | FileCheck -check-prefix=NOFTZ %s
 
+// Test explicit argument.
+// RUN: %clang -no-canonical-prefixes -### -target x86_64-linux-gnu -c -march=haswell --cuda-gpu-arch=gfx803 -fcuda-flush-denormals-to-zero -nocudainc -nogpulib %s 2>&1 | FileCheck -check-prefix=FTZ %s
+// RUN: %clang -no-canonical-prefixes -### -target x86_64-linux-gnu -c -march=haswell --cuda-gpu-arch=gfx803 -fno-cuda-flush-denormals-to-zero -nocudainc -nogpulib %s 2>&1 | FileCheck -check-prefix=NOFTZ %s
+// RUN: %clang -x hip -no-canonical-prefixes -### -target x86_64-linux-gnu -c -march=haswell --cuda-gpu-arch=gfx900 -fcuda-flush-denormals-to-zero -nocudainc -nogpulib %s 2>&1 | FileCheck -check-prefix=FTZ %s
+// RUN: %clang -x hip -no-canonical-prefixes -### -target x86_64-linux-gnu -c -march=haswell --cuda-gpu-arch=gfx900 -fno-cuda-flush-denormals-to-zero -nocudainc -nogpulib %s 2>&1 | FileCheck -check-prefix=NOFTZ %s
+
+// Test the default changing with no argument based on the subtarget.
+// RUN: %clang -x hip -no-canonical-prefixes -### -target x86_64-linux-gnu -c -march=haswell --cuda-gpu-arch=gfx803 -nocudainc -nogpulib %s 2>&1 | FileCheck -check-prefix=FTZ %s
+// RUN: %clang -x hip -no-canonical-prefixes -### -target x86_64-linux-gnu -c -march=haswell --cuda-gpu-arch=gfx900 -nocudainc -nogpulib %s 2>&1 | FileCheck -check-prefix=NOFTZ %s
+
 // CPUFTZ-NOT: -fdenormal-fp-math
 
 // FTZ-NOT: -fdenormal-fp-math-f32=
Index: clang/lib/Driver/ToolChains/HIP.h
===
--- clang/lib/Driver/ToolChains/HIP.h
+++ clang/lib/Driver/ToolChains/HIP.h
@@ -115,6 +115,11 @@
 
   unsigned GetDefaultDwarfVersion() const override { return 4; }
 
+  llvm::DenormalMode getDefaultDenormalModeForType(
+const llvm::opt::ArgList &DriverArgs,
+Action::OffloadKind DeviceOffloadKind,
+const llvm::fltSemantics *FPType = nullptr) const override;
+
   const ToolChain &HostTC;
 
 protected:
Index: clang/lib/Driver/ToolChains/HIP.cpp
===
--- clang/lib/Driver/ToolChains/HIP.cpp
+++ clang/lib/Driver/ToolChains/HIP.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "HIP.h"
+#include "AMDGPU.h"
 #include "CommonArgs.h"
 #include "InputInfo.h"
 #include "clang/Basic/Cuda.h"
@@ -16,6 +17,7 @@
 #include "clang/Driver/Options.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/TargetParser.h"
 
 using namespace clang::driver;
 using namespace clang::driver::toolchains;
@@ -272,6 +274,34 @@
   getProgramPaths().push_back(getDriver().Dir);
 }
 
+// FIXME: Duplicated in AMDGPUToolChain
+llvm::DenormalMode HIPToolChain::getDefaultDenormalModeForType(
+const llvm::opt::ArgList &DriverArgs, Action::OffloadKind DeviceOffloadKind,
+const llvm::fltSemantics *FPType) const {
+  // Denormals should always be enabled for f16 and f64.
+  if (!FPType || FPType != &llvm::APFloat::IEEEsingle())
+return llvm::DenormalMode::getIEEE();
+
+  if (DeviceOffloadKind == Action::OFK_Cuda) {
+if (FPType && FPType == &llvm::APFloat::IEEEsingle() &&
+DriverArgs.hasFlag(options::OPT_fcuda_flush_denormals_to_zero,
+   options::OPT_fno_cuda_flush_denormals_to_zero,
+   false))
+  return llvm::DenormalMode::getPreserveSign();
+  }
+
+  const StringRef GpuArch = DriverArgs.getLastArgValue(options::OPT_mcpu_EQ);
+  auto Kind = llvm::AMDGPU::parseArchAMDGCN(GpuArch);
+
+  // TODO: There are way too many flags that change this. Do we need to check
+  // them all?
+  bool DAZ = DriverArgs.hasArg(options::OPT_cl_denorms_are_zero) ||
+!AMDGPUToolChain::getDefaultDenormsAreZeroForTarget(Kind);
+  // Outputs 

[PATCH] D76830: [Analyzer][MallocChecker] No warning for kfree of ZERO_SIZE_PTR.

2020-03-26 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments.



Comment at: clang/test/Analysis/kmalloc-linux-1.c:15
+
+// FIXME: malloc checker expects kfree with 2 arguments, is this correct?
+// (recent kernel source code contains a `kfree(const void *)`)

balazske wrote:
> Szelethus wrote:
> > martong wrote:
> > > Do you plan to sniff around a bit about the arguments (as part of another 
> > > patch)? Would be good to handle both old and new signature kernel 
> > > allocation functions.
> > I'll take a quick look as well, I am quite familiar with MallocChecker.
> I found that `kfree` has one argument, not two (even in the 2.6 kernel). 
> Probably argument count was wrong in the last change when `CallDescription` 
> was introduced.
Yup, you're totally right, this was on me :) I'll get that fixed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76830



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


[PATCH] D76830: [Analyzer][MallocChecker] No warning for kfree of ZERO_SIZE_PTR.

2020-03-26 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1687
+  if (ArgValKnown) {
+if (!KernelZeroSizePtrValue)
+  KernelZeroSizePtrValue =

Szelethus wrote:
> martong wrote:
> > balazske wrote:
> > > martong wrote:
> > > > martong wrote:
> > > > > This is a bit confusing for me. Perhaps alternatively we could have a 
> > > > > free function `isInitialized(KernelZero...)` instead. Or maybe having 
> > > > > a separate bool variable to indicate whether it was initialized could 
> > > > > be cleaner?
> > > > Another idea: Adding a helper struct to contain the bool `initialized`? 
> > > > E.g. (draft):
> > > > ```
> > > > struct LazyOptional {
> > > >   bool initialized = false;
> > > >   Opt value;
> > > >   Opt& get();
> > > >   void set(const Opt&);
> > > > };
> > > > ```
> > > It may be OK to have a function `lazyInitKernelZeroSizePtrValue`?
> > I don't insist, given we have a better described type for 
> > `KernelZeroSizePtrValue` (e.g. having a `using` `template`)
> `AnalysisManager` has access to the `Preprocessor`, and it is also 
> responsible for the construction of the `CheckerManager` object. This would 
> make moving such code to the checker registry function! I'll gladly take this 
> issue off your hand and patch it in once 
> rG2aac0c47aed8d1eff7ab6d858173c532b881d948 settles :)
Just pushed rG4dc8472942ce60f29de9587b9451cc3d49df7abf. It it settles (no 
buildbots complain), you could rebase and the lazy initialization could be a 
thing of the past! :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76830



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


[PATCH] D76416: [WIP][ASan] Apply -ffile-prefix-map mappings to ASan instrumentation

2020-03-26 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment.

> Putting this on hold for now. Although this implementation works for ASan, it 
> would be have to be repeated for other tools like SourceBasedCoverage or 
> other sanitizers.

(You can click "Add Action..."->"Plan Changes" so that this Differential will 
disappear from others' review list for a while..)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76416



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


[PATCH] D76725: [clangd] Build ASTs only with fresh preambles or after building a new preamble

2020-03-26 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments.



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:8
 
//===--===//
 // For each file, managed by TUScheduler, we create a single ASTWorker that
+// manages an AST and a preamble for that file. All operations that modify or

This is explaining a pretty complicated thing, so I think it's particularly 
important to clearly organize the explanation, use consistent terminology, and 
avoid including unneccesary details.

I'd suggest introducing with paragraphs in this order:

- TUScheduler and the idea of a worker per TU.
- ASTWorker thread, the queue of interleaved updates and reads, elision of dead 
updates.
- Compatibility of preambles and updates, picky reads. 
- PreambleWorker thread, elision of preambles, golden ASTs.
- Published ASTs, epochs/monotonicity.



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:11
+// read the AST are run on a separate dedicated thread asynchronously in FIFO
+// order. There is a second thread for preamble builds, ASTWorker thread issues
+// updates on that preamble thread as preamble becomes stale.

nit: name the other thread (there is a second PreambleWorker thread ...)



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:12
+// order. There is a second thread for preamble builds, ASTWorker thread issues
+// updates on that preamble thread as preamble becomes stale.
+//

"issues updates" is vague. What about "... enqueues rebuilds on the 
PreambleWorker thread as preamble becomes stale. Currently, it then immediately 
blocks on that request."



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:19
+// FIXME: Get rid of the synchronization between ASTWorker::update and
+// PreambleThread builds by using patched ASTs instead of compatible preambles
+// for reads. Only keep the blocking behaviour for picky read requests, e.g.

You haven't defined "compatible" anywhere, and it's not obvious what it means.



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:23
+//
+// Updates are processed in two phases, by issuing a preamble and an ast build.
+// The preamble build gets issued into a different worker and might be

The wording here (particularly the word "phases") implies sequencing: an update 
results in a preamble build followed by an ast build in sequence, when in fact 
it usually results in an ast build and preamble build in parallel (the former 
usually finishing first) with a second ast build sequenced after the preamble 
build.



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:25
+// The preamble build gets issued into a different worker and might be
+// overwritten by subsequent updates. An AST will be built for an update if
+// latest built preamble is compatible for it or a new preamble gets built for

This isn't true, an AST is built for an update if it is needed (a read is based 
on it, wantdiagnostics=yes, it changed the preamble, or wantdiagnostics=maybe 
and the debounce expired)



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:29
+// AST builds are always executed on ASTWorker thread, either immediately
+// following an update request with a compatible preamble, or through a new
+// request, inserted at front of the queue, from PreambleThread after building 
a

I'm not sure what compatible means here, but we will certainly build ASTs for 
incompatible preambles.
(Or is this describing the intermediate state as of this patch, with the plan 
to rewrite it all next patch? I think we should rather describe the new model, 
and then document the current deviations from it)



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:29
+// AST builds are always executed on ASTWorker thread, either immediately
+// following an update request with a compatible preamble, or through a new
+// request, inserted at front of the queue, from PreambleThread after building 
a

sammccall wrote:
> I'm not sure what compatible means here, but we will certainly build ASTs for 
> incompatible preambles.
> (Or is this describing the intermediate state as of this patch, with the plan 
> to rewrite it all next patch? I think we should rather describe the new 
> model, and then document the current deviations from it)
I'm not sure what "immediately following" is relative to: if it's after the 
update() call, it's not immediate - it has to wait in the queue.



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:33
+//
+// Diagnostics and index is updated as a result of building AST for write
+// requests. Progress on those updates are ensured with golden ASTs, as they'll

"and *the* index *are* updated"... "building *the* AST"



Comment at: clang-tools-extra/cla

[PATCH] D74934: [Clang interpreter] Rename Block.{h,cpp} to InterpBlock.{h,cpp}

2020-03-26 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment.

@gribozavr2 - hey, an annoying quirk of Phabricator is that it doesn't send 
mail to the mailing list on state changes with no text, which means reviews 
like these look like tehy were committed without approval (on the mailing list 
there's no record of the approval). In the future, can you add some comment to 
your approval ("Looks good", "thanks", etc) to ensure mail is sent to the 
mailing list to document that approval there?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74934



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


[PATCH] D76365: [cuda][hip] Add CUDA builtin surface/texture reference support.

2020-03-26 Thread Artem Belevich via Phabricator via cfe-commits
tra accepted this revision.
tra added a comment.
This revision is now accepted and ready to land.

LGTM. Next step is to figure out what various 
__nv_tex_surf_handler(...) maps to for various strings (there are ~110 
of them in CUDA-10.2) and implement its replacement. I think we should be able 
to do it in the wrapper file.




Comment at: clang/lib/Headers/__clang_cuda_runtime_wrapper.h:82-94
 #undef __CUDACC__
 #if CUDA_VERSION < 9000
 #define __CUDABE__
 #else
+#define __CUDACC__
 #define __CUDA_LIBDEVICE__
 #endif

hliao wrote:
> tra wrote:
> > hliao wrote:
> > > hliao wrote:
> > > > tra wrote:
> > > > > hliao wrote:
> > > > > > tra wrote:
> > > > > > > Please add comments on why __CUDACC__ is needed for 
> > > > > > > driver_types.h here? AFAICT, driver_types.h does not have any 
> > > > > > > conditionals that depend on __CUDACC__. What happens if it's not 
> > > > > > > defined.
> > > > > > > 
> > > > > > > 
> > > > > > `driver_types.h` includes `host_defines.h`, where macros 
> > > > > > `__device_builtin_surface_type__` and 
> > > > > > `__device_builtin_texture_type__` are conditional defined if 
> > > > > > `__CUDACC__`.
> > > > > > 
> > > > > > The following is extracted from `cuda/crt/host_defines.h`
> > > > > > 
> > > > > > ```
> > > > > > #if !defined(__CUDACC__)
> > > > > > #define __device_builtin__
> > > > > > #define __device_builtin_texture_type__
> > > > > > #define __device_builtin_surface_type__
> > > > > > #define __cudart_builtin__
> > > > > > #else /* defined(__CUDACC__) */
> > > > > > #define __device_builtin__ \
> > > > > > __location__(device_builtin)
> > > > > > #define __device_builtin_texture_type__ \
> > > > > > __location__(device_builtin_texture_type)
> > > > > > #define __device_builtin_surface_type__ \
> > > > > > __location__(device_builtin_surface_type)
> > > > > > #define __cudart_builtin__ \
> > > > > > __location__(cudart_builtin)
> > > > > > #endif /* !defined(__CUDACC__) */
> > > > > > ```
> > > > > My concern is -- what else is going to get defined? There are ~60 
> > > > > references to __CUDACC__ in CUDA-10.1 headers. The wrappers are 
> > > > > fragile enough that there's a good chance something may break. It 
> > > > > does not help that my CUDA build bot decided to die just after we 
> > > > > switched to work-from-home, so there will be no early warning if 
> > > > > something goes wrong.
> > > > > 
> > > > > If all we need are the macros above, we may just define them. 
> > > > Let me check all CUDA SDK through their dockers. Redefining sounds good 
> > > > me as wll.
> > > I checked headers from 7.0 to 10.0, `__device_builtin_texture_type__` and 
> > > `__builtin_builtin_surface_type__` are only defined with that attributes 
> > > if `__CUDACC__` is defined. As we only pre-define `__CUDA_ARCH__` in 
> > > clang but flip `__CUDACC__` on and off in the wrapper headers to 
> > > selectively reuse CUDA's headers. I would hear your suggestion on that.
> > > BTW, macros like `__device__` are defined regardless of `__CUDACC__` from 
> > > 7.0 to 10.0 as `__location(device)`. `__location__` is defined if 
> > > `__CUDACC__` is present. But, different from `__device__`, 
> > > `__device_builtin_texture_type__` is defined only `__CUDACC__` is defined.
> > `__device_builtin_texture_type__` is defined in `host_defines.h`, which 
> > does not seem to include any other files or does anything suspicious with 
> > `__CUDACC__`
> > 
> > It may be OK to move inclusion of `host_defines.h` to the point before 
> > `driver_types.h`, which happens to include the host_defines.h first, and 
> > define __CUDACC__ only around `host_defines.h`.
> > 
> > An alternative is to add the macros just after inclusion of `host_defines.h`
> > 
> > In either case please verify that these attributes are the only things 
> > that's changed by diffing output of `clang++ -x cuda /dev/null 
> > --cuda-host-only -dD -E -o -` before and after the change.
> With `__CUDACC__`, the only difference is the additional attributes added, 
> such as `device_builtin_texture_type`. Attributes like `cudart_builtin` are 
> also defined correctly. That should be used to start the support CUDART 
> features.
> I revised the change to include `host_defines.h` first and found there's no 
> changes from the one using `driver_types.h`. We should be OK for that change.
SGTM. Thank you for verifying this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76365



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


[PATCH] D74324: Tools emit the bug report URL on crash

2020-03-26 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments.



Comment at: llvm/lib/Support/PrettyStackTrace.cpp:145
 
+static const char* customBugReportMsg;
+

jhenderson wrote:
> jhenderson wrote:
> > `static const char *BugReportMsg;`
> > 
> > Why not just have this set to the default in the first place, and overwrite 
> > it if somebody needs to? That'll remove the need for the new `if` in the 
> > `CrashHandler` function.
> Again, this hasn't been properly addresssed. This should use an upper-case 
> letter for the first letter of variable names. Please see [[ 
> https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly
>  | the coding standards ]].
`static const char BugReportMsg[] = ...`

`static const char *BugReportMsg` defines a writable variable.

In IR, the former does not have the unnamed_addr attribute while the latter 
has. The latter can lower to .rodata.str1.1 (SHF_MERGE | SHF_STRINGS) which can 
be optimized by the linker. This is nuance and the optimization probably weighs 
nothing (it is hardly to think there will be another thing having the same byte 
sequence.) The former just seems more correct.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74324



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


[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

2020-03-26 Thread Jason Liu via Phabricator via cfe-commits
jasonliu added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:4205
 
-CharUnits PPC32_SVR4_ABIInfo::getParamTypeAlignment(QualType Ty) const {
+CharUnits PowerPC32ABIInfo::getParamTypeAlignment(QualType Ty) const {
   // Complex types are passed just like their elements

sfertile wrote:
> jasonliu wrote:
> > So for AIX we are taking PowerPC32ABIInfo right now. And we know EmitVAArg 
> > of this target does the right thing on AIX after the change. 
> > But for other functions, for example, getParamTypeAlignment, 
> > initDwarfEHRegSizeTable... Are we sure it's doing the right thing on AIX?
> > If we are not sure, is there anything we want to do (etc, put a comment on 
> > where it gets used or at the function definition)? Or are we fine to just 
> > leave it as it is and have a TODO in our head?
> Looking at the values in `initDwarfEHRegSizeTable` it has the right sizes for 
> all the registers. Even though the OS is different the underlying hardware is 
> the same. I'm not sure it's something that makes sense to support for AIX 
> though, in which case I think its valid to return `true` to indicate its not 
> supported. 
> 
> `getParamTypeAlignment` is used only in the context of the alignment for 
> vaarg, we should make sure its correct for AIX since supporting vaarg is the 
> scope of this patch.
In that case, is it better if we have lit test to actually exercise the path in 
getParamTypeAlignment to show that we actually confirmed the behavior is 
correct for AIX? And if it is some path we do not care for now(ComplexType? 
VectorType?), then we would want TODOs on them to show we need further 
investigation later. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76360



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


[clang] 47e7bdb - Test that would have caught recovery-expr crashes in 0788acbccbec. NFC

2020-03-26 Thread Sam McCall via cfe-commits

Author: Sam McCall
Date: 2020-03-26T18:43:29+01:00
New Revision: 47e7bdb10732e6f140adce39a1bc34e3ee2a6ea6

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

LOG: Test that would have caught recovery-expr crashes in 0788acbccbec. NFC

Added: 
clang/test/Sema/invalid-member.cpp

Modified: 


Removed: 




diff  --git a/clang/test/Sema/invalid-member.cpp 
b/clang/test/Sema/invalid-member.cpp
new file mode 100644
index ..5475157e936e
--- /dev/null
+++ b/clang/test/Sema/invalid-member.cpp
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -verify -fsyntax-only %s
+void foo(); // expected-note {{requires 0 arguments}}
+class X {
+  decltype(foo(42)) invalid; // expected-error {{no matching function}}
+};
+// Should be able to evaluate sizeof without crashing.
+static_assert(sizeof(X) == 1, "No valid members");



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


[PATCH] D76772: [AMDGPU] Add __builtin_amdgcn_workgroup_size_x/y/z

2020-03-26 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl marked 2 inline comments as done.
yaxunl added inline comments.



Comment at: clang/test/CodeGenCUDA/amdgpu-workgroup-size.cu:2
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa \
+// RUN: -fcuda-is-device -emit-llvm -o - -x hip %s \
+// RUN: | FileCheck %s

arsenm wrote:
> I assume the addrspacecast got optimized out? Should this disable llvm passes?
We did not emit addrspacecast here since we only need return the loaded value.

HIP by default uses -O0, therefore no need to disable llvm passes.


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

https://reviews.llvm.org/D76772



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


[PATCH] D74324: Tools emit the bug report URL on crash

2020-03-26 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments.



Comment at: llvm/lib/Support/PrettyStackTrace.cpp:145
 
+static const char* customBugReportMsg;
+

MaskRay wrote:
> jhenderson wrote:
> > jhenderson wrote:
> > > `static const char *BugReportMsg;`
> > > 
> > > Why not just have this set to the default in the first place, and 
> > > overwrite it if somebody needs to? That'll remove the need for the new 
> > > `if` in the `CrashHandler` function.
> > Again, this hasn't been properly addresssed. This should use an upper-case 
> > letter for the first letter of variable names. Please see [[ 
> > https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly
> >  | the coding standards ]].
> `static const char BugReportMsg[] = ...`
> 
> `static const char *BugReportMsg` defines a writable variable.
> 
> In IR, the former does not have the unnamed_addr attribute while the latter 
> has. The latter can lower to .rodata.str1.1 (SHF_MERGE | SHF_STRINGS) which 
> can be optimized by the linker. This is nuance and the optimization probably 
> weighs nothing (it is hardly to think there will be another thing having the 
> same byte sequence.) The former just seems more correct.
Nevermind. I did not see `setBugReportMsg`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74324



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


  1   2   >