[PATCH] D66713: ContentCache: Drop getBuffer's dependency on SourceManager

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith updated this revision to Diff 217036.
dexonsmith added a comment.

(Updating the diff with full context.)


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

https://reviews.llvm.org/D66713

Files:
  clang/include/clang/Basic/SourceManager.h
  clang/lib/Basic/SourceManager.cpp
  clang/lib/Serialization/ASTWriter.cpp

Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1804,12 +1804,12 @@
 
 InputFileEntry Entry;
 Entry.File = Cache->OrigEntry;
-Entry.IsSystemFile = Cache->IsSystemFile;
+Entry.IsSystemFile = isSystem(File.getFileCharacteristic());
 Entry.IsTransient = Cache->IsTransient;
 Entry.BufferOverridden = Cache->BufferOverridden;
 Entry.IsTopLevelModuleMap = isModuleMap(File.getFileCharacteristic()) &&
 File.getIncludeLoc().isInvalid();
-if (Cache->IsSystemFile)
+if (Entry.IsSystemFile)
   SortedFiles.push_back(Entry);
 else
   SortedFiles.push_front(Entry);
@@ -2315,8 +2315,8 @@
 // We add one to the size so that we capture the trailing NULL
 // that is required by llvm::MemoryBuffer::getMemBuffer (on
 // the reader side).
-const llvm::MemoryBuffer *Buffer
-  = Content->getBuffer(PP.getDiagnostics(), PP.getSourceManager());
+const llvm::MemoryBuffer *Buffer =
+Content->getBuffer(PP.getDiagnostics(), PP.getFileManager());
 StringRef Name = Buffer->getBufferIdentifier();
 Stream.EmitRecordWithBlob(SLocBufferAbbrv, Record,
   StringRef(Name.data(), Name.size() + 1));
@@ -2330,7 +2330,7 @@
 // Include the implicit terminating null character in the on-disk buffer
 // if we're writing it uncompressed.
 const llvm::MemoryBuffer *Buffer =
-Content->getBuffer(PP.getDiagnostics(), PP.getSourceManager());
+Content->getBuffer(PP.getDiagnostics(), PP.getFileManager());
 StringRef Blob(Buffer->getBufferStart(), Buffer->getBufferSize() + 1);
 emitBlob(Stream, Blob, SLocBufferBlobCompressedAbbrv,
  SLocBufferBlobAbbrv);
Index: clang/lib/Basic/SourceManager.cpp
===
--- clang/lib/Basic/SourceManager.cpp
+++ clang/lib/Basic/SourceManager.cpp
@@ -96,7 +96,7 @@
 }
 
 const llvm::MemoryBuffer *ContentCache::getBuffer(DiagnosticsEngine &Diag,
-  const SourceManager &SM,
+  FileManager &FM,
   SourceLocation Loc,
   bool *Invalid) const {
   // Lazily create the Buffer for ContentCaches that wrap files.  If we already
@@ -134,9 +134,7 @@
 return Buffer.getPointer();
   }
 
-  bool isVolatile = SM.userFilesAreVolatile() && !IsSystemFile;
-  auto BufferOrError =
-  SM.getFileManager().getBufferForFile(ContentsEntry, isVolatile);
+  auto BufferOrError = FM.getBufferForFile(ContentsEntry, IsFileVolatile);
 
   // If we were unable to open the file, then we are in an inconsistent
   // situation where the content cache referenced a file which no longer
@@ -389,7 +387,7 @@
 Clone->OrigEntry = Cache->OrigEntry;
 Clone->ContentsEntry = Cache->ContentsEntry;
 Clone->BufferOverridden = Cache->BufferOverridden;
-Clone->IsSystemFile = Cache->IsSystemFile;
+Clone->IsFileVolatile = Cache->IsFileVolatile;
 Clone->IsTransient = Cache->IsTransient;
 Clone->replaceBuffer(Cache->getRawBuffer(), /*DoNotFree*/true);
 return Clone;
@@ -438,7 +436,7 @@
 new (Entry) ContentCache(FileEnt);
   }
 
-  Entry->IsSystemFile = isSystemFile;
+  Entry->IsFileVolatile = UserFilesAreVolatile && !isSystemFile;
   Entry->IsTransient = FilesAreTransient;
 
   return Entry;
@@ -645,7 +643,7 @@
 SourceManager::getMemoryBufferForFile(const FileEntry *File, bool *Invalid) {
   const SrcMgr::ContentCache *IR = getOrCreateContentCache(File);
   assert(IR && "getOrCreateContentCache() cannot return NULL");
-  return IR->getBuffer(Diag, *this, SourceLocation(), Invalid);
+  return IR->getBuffer(Diag, getFileManager(), SourceLocation(), Invalid);
 }
 
 void SourceManager::overrideFileContents(const FileEntry *SourceFile,
@@ -699,7 +697,7 @@
   }
 
   const llvm::MemoryBuffer *Buf = SLoc.getFile().getContentCache()->getBuffer(
-  Diag, *this, SourceLocation(), &MyInvalid);
+  Diag, getFileManager(), SourceLocation(), &MyInvalid);
   if (Invalid)
 *Invalid = MyInvalid;
 
@@ -1130,7 +1128,7 @@
   }
   const llvm::MemoryBuffer *Buffer =
   Entry.getFile().getContentCache()->getBuffer(
-  Diag, *this, SourceLocation(), &CharDataInvalid);
+  Diag, getFileManager(), SourceLocation(), &CharDataInvalid);
   if (Inva

[PATCH] D66713: ContentCache: Drop getBuffer's dependency on SourceManager

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith created this revision.
dexonsmith added reviewers: arphaman, bruno.
Herald added a subscriber: ributzka.

Refactor ContentCache::IsSystemFile to IsFileVolatile, checking
SourceManager::userFilesAreVolatile at construction time.  This is a
step toward lowering ContentCache down from SourceManager to
FileManager.

No functionality change intended.


https://reviews.llvm.org/D66713

Files:
  clang/include/clang/Basic/SourceManager.h
  clang/lib/Basic/SourceManager.cpp
  clang/lib/Serialization/ASTWriter.cpp

Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1804,12 +1804,12 @@
 
 InputFileEntry Entry;
 Entry.File = Cache->OrigEntry;
-Entry.IsSystemFile = Cache->IsSystemFile;
+Entry.IsSystemFile = isSystem(File.getFileCharacteristic());
 Entry.IsTransient = Cache->IsTransient;
 Entry.BufferOverridden = Cache->BufferOverridden;
 Entry.IsTopLevelModuleMap = isModuleMap(File.getFileCharacteristic()) &&
 File.getIncludeLoc().isInvalid();
-if (Cache->IsSystemFile)
+if (Entry.IsSystemFile)
   SortedFiles.push_back(Entry);
 else
   SortedFiles.push_front(Entry);
@@ -2315,8 +2315,8 @@
 // We add one to the size so that we capture the trailing NULL
 // that is required by llvm::MemoryBuffer::getMemBuffer (on
 // the reader side).
-const llvm::MemoryBuffer *Buffer
-  = Content->getBuffer(PP.getDiagnostics(), PP.getSourceManager());
+const llvm::MemoryBuffer *Buffer =
+Content->getBuffer(PP.getDiagnostics(), PP.getFileManager());
 StringRef Name = Buffer->getBufferIdentifier();
 Stream.EmitRecordWithBlob(SLocBufferAbbrv, Record,
   StringRef(Name.data(), Name.size() + 1));
@@ -2330,7 +2330,7 @@
 // Include the implicit terminating null character in the on-disk buffer
 // if we're writing it uncompressed.
 const llvm::MemoryBuffer *Buffer =
-Content->getBuffer(PP.getDiagnostics(), PP.getSourceManager());
+Content->getBuffer(PP.getDiagnostics(), PP.getFileManager());
 StringRef Blob(Buffer->getBufferStart(), Buffer->getBufferSize() + 1);
 emitBlob(Stream, Blob, SLocBufferBlobCompressedAbbrv,
  SLocBufferBlobAbbrv);
Index: clang/lib/Basic/SourceManager.cpp
===
--- clang/lib/Basic/SourceManager.cpp
+++ clang/lib/Basic/SourceManager.cpp
@@ -96,7 +96,7 @@
 }
 
 const llvm::MemoryBuffer *ContentCache::getBuffer(DiagnosticsEngine &Diag,
-  const SourceManager &SM,
+  FileManager &FM,
   SourceLocation Loc,
   bool *Invalid) const {
   // Lazily create the Buffer for ContentCaches that wrap files.  If we already
@@ -134,9 +134,7 @@
 return Buffer.getPointer();
   }
 
-  bool isVolatile = SM.userFilesAreVolatile() && !IsSystemFile;
-  auto BufferOrError =
-  SM.getFileManager().getBufferForFile(ContentsEntry, isVolatile);
+  auto BufferOrError = FM.getBufferForFile(ContentsEntry, IsFileVolatile);
 
   // If we were unable to open the file, then we are in an inconsistent
   // situation where the content cache referenced a file which no longer
@@ -389,7 +387,7 @@
 Clone->OrigEntry = Cache->OrigEntry;
 Clone->ContentsEntry = Cache->ContentsEntry;
 Clone->BufferOverridden = Cache->BufferOverridden;
-Clone->IsSystemFile = Cache->IsSystemFile;
+Clone->IsFileVolatile = Cache->IsFileVolatile;
 Clone->IsTransient = Cache->IsTransient;
 Clone->replaceBuffer(Cache->getRawBuffer(), /*DoNotFree*/true);
 return Clone;
@@ -438,7 +436,7 @@
 new (Entry) ContentCache(FileEnt);
   }
 
-  Entry->IsSystemFile = isSystemFile;
+  Entry->IsFileVolatile = UserFilesAreVolatile && !isSystemFile;
   Entry->IsTransient = FilesAreTransient;
 
   return Entry;
@@ -645,7 +643,7 @@
 SourceManager::getMemoryBufferForFile(const FileEntry *File, bool *Invalid) {
   const SrcMgr::ContentCache *IR = getOrCreateContentCache(File);
   assert(IR && "getOrCreateContentCache() cannot return NULL");
-  return IR->getBuffer(Diag, *this, SourceLocation(), Invalid);
+  return IR->getBuffer(Diag, getFileManager(), SourceLocation(), Invalid);
 }
 
 void SourceManager::overrideFileContents(const FileEntry *SourceFile,
@@ -699,7 +697,7 @@
   }
 
   const llvm::MemoryBuffer *Buf = SLoc.getFile().getContentCache()->getBuffer(
-  Diag, *this, SourceLocation(), &MyInvalid);
+  Diag, getFileManager(), SourceLocation(), &MyInvalid);
   if (Invalid)
 *Invalid = MyInvalid;
 
@@ -1130,7 +1128,7 @@
   }
   const llvm::MemoryBuffer *Buffer =
   Entry.getFile(

[PATCH] D66712: [Driver] Enable ShadowCallStack, not SafeStack, by default on AArch64 Fuchsia

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added reviewers: phosek, leonardchan, jakehehrlich.
mcgrathr added a comment.

This should land only after we've done our first stages of downstream roll-out 
and burn-in testing.  But setting it up now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66712



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

[PATCH] D66712: [Driver] Enable ShadowCallStack, not SafeStack, by default on AArch64 Fuchsia

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr created this revision.
Herald added subscribers: cfe-commits, cryptoad, kristof.beyls, javed.absar.
Herald added a project: clang.

On AArch64, Fuchsia fully supports both SafeStack and ShadowCallStack ABIs.
The latter is now preferred and will be the default.  It's possible to
enable both simultaneously, but ShadowCallStack is believed to have most
of the practical benefit of SafeStack with less cost.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66712

Files:
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  clang/test/Driver/fuchsia.c


Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -13,7 +13,8 @@
 // CHECK: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
 // CHECK: "-isysroot" "[[SYSROOT:[^"]+]]"
 // CHECK: "-internal-externc-isystem" "[[SYSROOT]]{{/|}}include"
-// CHECK: "-fsanitize=safe-stack"
+// CHECK-AARCH64: "-fsanitize=shadow-call-stack"
+// CHECK-X86_64: "-fsanitize=safe-stack"
 // CHECK: "-stack-protector" "2"
 // CHECK: "-fno-common"
 // CHECK: {{.*}}ld.lld{{.*}}" "-z" "rodynamic" "-z" "separate-code"
@@ -102,7 +103,7 @@
 // RUN: -fuse-ld=lld \
 // RUN: | FileCheck %s -check-prefix=CHECK-ASAN-AARCH64
 // CHECK-ASAN-AARCH64: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
-// CHECK-ASAN-AARCH64: "-fsanitize=address"
+// CHECK-ASAN-AARCH64: "-fsanitize=address,shadow-call-stack"
 // CHECK-ASAN-AARCH64: "-fsanitize-address-globals-dead-stripping"
 // CHECK-ASAN-AARCH64: "-dynamic-linker" "asan/ld.so.1"
 // CHECK-ASAN-AARCH64: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}aarch64-fuchsia{{/|}}libclang_rt.asan.so"
@@ -134,7 +135,7 @@
 // RUN: -fuse-ld=lld \
 // RUN: | FileCheck %s -check-prefix=CHECK-FUZZER-AARCH64
 // CHECK-FUZZER-AARCH64: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
-// CHECK-FUZZER-AARCH64: "-fsanitize=fuzzer,fuzzer-no-link,safe-stack"
+// CHECK-FUZZER-AARCH64: "-fsanitize=fuzzer,fuzzer-no-link,shadow-call-stack"
 // CHECK-FUZZER-AARCH64: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}aarch64-fuchsia{{/|}}libclang_rt.fuzzer.a"
 
 // RUN: %clang %s -### --target=x86_64-fuchsia \
@@ -153,7 +154,7 @@
 // RUN: -fuse-ld=lld \
 // RUN: | FileCheck %s -check-prefix=CHECK-SCUDO-AARCH64
 // CHECK-SCUDO-AARCH64: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
-// CHECK-SCUDO-AARCH64: "-fsanitize=safe-stack,scudo"
+// CHECK-SCUDO-AARCH64: "-fsanitize=shadow-call-stack,scudo"
 // CHECK-SCUDO-AARCH64: "-pie"
 // CHECK-SCUDO-AARCH64: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}aarch64-fuchsia{{/|}}libclang_rt.scudo.so"
 
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -343,5 +343,10 @@
 }
 
 SanitizerMask Fuchsia::getDefaultSanitizers() const {
-  return SanitizerKind::SafeStack;
+  SanitizerMask Res;
+  if (getTriple().getArch() == llvm::Triple::aarch64)
+Res |= SanitizerKind::ShadowCallStack;
+  else
+Res |= SanitizerKind::SafeStack;
+  return Res;
 }


Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -13,7 +13,8 @@
 // CHECK: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
 // CHECK: "-isysroot" "[[SYSROOT:[^"]+]]"
 // CHECK: "-internal-externc-isystem" "[[SYSROOT]]{{/|}}include"
-// CHECK: "-fsanitize=safe-stack"
+// CHECK-AARCH64: "-fsanitize=shadow-call-stack"
+// CHECK-X86_64: "-fsanitize=safe-stack"
 // CHECK: "-stack-protector" "2"
 // CHECK: "-fno-common"
 // CHECK: {{.*}}ld.lld{{.*}}" "-z" "rodynamic" "-z" "separate-code"
@@ -102,7 +103,7 @@
 // RUN: -fuse-ld=lld \
 // RUN: | FileCheck %s -check-prefix=CHECK-ASAN-AARCH64
 // CHECK-ASAN-AARCH64: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
-// CHECK-ASAN-AARCH64: "-fsanitize=address"
+// CHECK-ASAN-AARCH64: "-fsanitize=address,shadow-call-stack"
 // CHECK-ASAN-AARCH64: "-fsanitize-address-globals-dead-stripping"
 // CHECK-ASAN-AARCH64: "-dynamic-linker" "asan/ld.so.1"
 // CHECK-ASAN-AARCH64: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aarch64-fuchsia{{/|}}libclang_rt.asan.so"
@@ -134,7 +135,7 @@
 // RUN: -fuse-ld=lld \
 // RUN: | FileCheck %s -check-prefix=CHECK-FUZZER-AARCH64
 // CHECK-FUZZER-AARCH64: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
-// CHECK-FUZZER-AARCH64: "-fsanitize=fuzzer,fuzzer-no-link,safe-stack"
+// CHECK-FUZZER-AARCH64: "-fsanitize=fuzzer,fuzzer-no-link,shadow-call-stack"
 // CHECK-FUZZER-AARCH64: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aarch64-fuchsia{{/|}}libclang_rt.fuzzer.a"
 
 // RUN: %clang %s -### --target=x86_64-fuchsia \
@@ -153,7 +154,7 @@
 // RUN: -fuse-ld=lld \
 // RUN: | FileCheck %s -check-prefix=CHECK-SCUDO-AARCH64
 // CHECK-SCUDO-AARCH64: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
-// CHECK-SCUDO-AARCH64: "-fsanitize=safe-stack,scudo"
+// CHECK-SCUDO-A

[PATCH] D66662: [clang-format] [PR43100] clang-format C# support does not add a space between "using" and paren

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

The test case fails after the missing `)` is added, so it seems that the patch 
has no effect.


Repository:
  rC Clang

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

https://reviews.llvm.org/D2



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

[PATCH] D66711: [clang] Warning for non-final classes with final destructors

[Logan Smith via Phabricator via cfe-commits]

logan-5 created this revision.
logan-5 added reviewers: rsmith, yamauchi.
logan-5 added a project: clang.
Herald added a subscriber: cfe-commits.

Marking a class' destructor `final` prevents the class from being inherited 
from. However, it is a subtle and awkward way to express that at best, and 
unintended at worst. It may also generate worse code (in other compilers) than 
marking the class itself `final`. For these reasons, this revision adds a 
warning for nonfinal classes with final destructors, with a note to suggest 
marking the class final to silence the warning.

See https://reviews.llvm.org/D66621 for more background.


Repository:
  rC Clang

https://reviews.llvm.org/D66711

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/SemaCXX/warn-final-dtor-nonfinal-class.cpp


Index: clang/test/SemaCXX/warn-final-dtor-nonfinal-class.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/warn-final-dtor-nonfinal-class.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -fsyntax-only -std=c++11 -verify %s 
-Wfinal-dtor-non-final-class
+
+class A {
+~A();
+};
+
+class B { // expected-note {{mark 'B' as 'final' to silence this warning}}
+virtual ~B() final; // expected-warning {{class with destructor marked 
'final' cannot be inherited from}}
+};
+
+class C final {
+virtual ~C() final;
+};
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -6235,6 +6235,19 @@
 }
   }
 
+  // Warn if the class has a final destructor but is not itself marked final.
+  if (!Record->hasAttr()) {
+if (CXXDestructorDecl *dtor = Record->getDestructor()) {
+  if (FinalAttr *FA = dtor->getAttr()) {
+Diag(FA->getLocation(), diag::warn_final_destructor_nonfinal_class)
+  << FA->isSpelledAsSealed();
+Diag(Record->getLocation(), 
diag::note_final_destructor_nonfinal_class_silence)
+  << Context.getRecordType(Record)
+  << FA->isSpelledAsSealed();
+  }
+}
+  }
+
   // See if trivial_abi has to be dropped.
   if (Record->hasAttr())
 checkIllFormedTrivialABIStruct(*Record);
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2201,6 +2201,11 @@
   "base %0 is marked '%select{final|sealed}1'">;
 def warn_abstract_final_class : Warning<
   "abstract class is marked '%select{final|sealed}0'">, 
InGroup;
+def warn_final_destructor_nonfinal_class : Warning<
+  "class with destructor marked '%select{final|sealed}0' cannot be inherited 
from">,
+  InGroup;
+def note_final_destructor_nonfinal_class_silence : Note<
+  "mark %0 as '%select{final|sealed}1' to silence this warning">;
 
 // C++11 attributes
 def err_repeat_attribute : Error<"%0 attribute cannot be repeated">;
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -114,6 +114,7 @@
  [DeleteNonAbstractNonVirtualDtor,
   DeleteAbstractNonVirtualDtor]>;
 def AbstractFinalClass : DiagGroup<"abstract-final-class">;
+def FinalDtorNonFinalClass : DiagGroup<"final-dtor-non-final-class">;
 
 def CXX11CompatDeprecatedWritableStr :
   DiagGroup<"c++11-compat-deprecated-writable-strings">;


Index: clang/test/SemaCXX/warn-final-dtor-nonfinal-class.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/warn-final-dtor-nonfinal-class.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -fsyntax-only -std=c++11 -verify %s -Wfinal-dtor-non-final-class
+
+class A {
+~A();
+};
+
+class B { // expected-note {{mark 'B' as 'final' to silence this warning}}
+virtual ~B() final; // expected-warning {{class with destructor marked 'final' cannot be inherited from}}
+};
+
+class C final {
+virtual ~C() final;
+};
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -6235,6 +6235,19 @@
 }
   }
 
+  // Warn if the class has a final destructor but is not itself marked final.
+  if (!Record->hasAttr()) {
+if (CXXDestructorDecl *dtor = Record->getDestructor()) {
+  if (FinalAttr *FA = dtor->getAttr()) {
+Diag(FA->getLocation(), diag::warn_final_destructor_nonfinal_class)
+  << FA->isSpelledAsSealed();
+Diag(Record->getLocation(), diag::note_final_destructor_nonfinal_class_silence)
+  << Context.getRecordType(Record)
+  << FA->isSpel

[PATCH] D66710: ASTReader: Bypass overridden files when reading PCHs

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith created this revision.
dexonsmith added reviewers: rsmith, arphaman, akyrtzi, bruno.
Herald added a subscriber: ributzka.

If contents of a file that is part of a PCM are overridden when reading
it, but weren't overridden when the PCM was being built, the ASTReader
will emit an error.  Now it creates a separate FileEntry for recovery,
bypassing the overridden content instead of discarding it.  The
pre-existing testcase clang/test/PCH/remap-file-from-pch.cpp confirms
that the new recovery method works correctly.

This resolves a long-standing FIXME to avoid hypothetically invalidating
another precompiled module that's already using the overridden contents.

This also removes ContentCache-related API that would be unsafe to use
across `CompilerInstance`s in an implicit modules build.  This helps to
unblock us sinking it from SourceManager into FileManager in the future,
which would allow us to delete `InMemoryModuleCache`.


https://reviews.llvm.org/D66710

Files:
  clang/include/clang/Basic/FileManager.h
  clang/include/clang/Basic/SourceManager.h
  clang/lib/Basic/FileManager.cpp
  clang/lib/Basic/SourceManager.cpp
  clang/lib/Serialization/ASTReader.cpp

Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -2315,19 +2315,14 @@
   if ((!Overridden && !Transient) && SM.isFileOverridden(File)) {
 if (Complain)
   Error(diag::err_fe_pch_file_overridden, Filename);
-// After emitting the diagnostic, recover by disabling the override so
-// that the original file will be used.
-//
-// FIXME: This recovery is just as broken as the original state; there may
-// be another precompiled module that's using the overridden contents, or
-// we might be half way through parsing it. Instead, we should treat the
-// overridden contents as belonging to a separate FileEntry.
-SM.disableFileContentsOverride(File);
-// The FileEntry is a virtual file entry with the size of the contents
-// that would override the original contents. Set it to the original's
-// size/time.
-FileMgr.modifyFileEntry(const_cast(File),
-StoredSize, StoredTime);
+
+// After emitting the diagnostic, bypass the overriding file to recover
+// (this creates a separate FileEntry).
+File = SM.bypassFileContentsOverride(*File);
+if (!File) {
+  F.InputFilesLoaded[ID - 1] = InputFile::getNotFound();
+  return InputFile();
+}
   }
 
   bool IsOutOfDate = false;
Index: clang/lib/Basic/SourceManager.cpp
===
--- clang/lib/Basic/SourceManager.cpp
+++ clang/lib/Basic/SourceManager.cpp
@@ -671,17 +671,17 @@
   getOverriddenFilesInfo().OverriddenFiles[SourceFile] = NewFile;
 }
 
-void SourceManager::disableFileContentsOverride(const FileEntry *File) {
-  if (!isFileOverridden(File))
-return;
+const FileEntry *
+SourceManager::bypassFileContentsOverride(const FileEntry &File) {
+  assert(isFileOverridden(&File));
+  auto *BypassFile = FileMgr.getBypassFile(File);
 
-  const SrcMgr::ContentCache *IR = getOrCreateContentCache(File);
-  const_cast(IR)->replaceBuffer(nullptr);
-  const_cast(IR)->ContentsEntry = IR->OrigEntry;
+  // If the file can't be found in the FS, give up.
+  if (!BypassFile)
+return nullptr;
 
-  assert(OverriddenFilesInfo);
-  OverriddenFilesInfo->OverriddenFiles.erase(File);
-  OverriddenFilesInfo->OverriddenFilesWithBuffer.erase(File);
+  (void)getOrCreateContentCache(BypassFile);
+  return BypassFile;
 }
 
 void SourceManager::setFileIsTransient(const FileEntry *File) {
Index: clang/lib/Basic/FileManager.cpp
===
--- clang/lib/Basic/FileManager.cpp
+++ clang/lib/Basic/FileManager.cpp
@@ -389,6 +389,24 @@
   return UFE;
 }
 
+const FileEntry *FileManager::getBypassFile(const FileEntry &VFE) {
+  // Stat of the file and return nullptr if it doesn't exist.
+  llvm::vfs::Status Status;
+  if (getStatValue(VFE.Name, Status, /*isFile=*/true, /*F=*/nullptr))
+return nullptr;
+
+  // Fill it in from the stat.
+  BypassFileEntries.push_back(std::make_unique());
+  auto &BFE = *BypassFileEntries.back();
+  BFE.Name = VFE.Name;
+  BFE.Size = Status.getSize();
+  BFE.Dir = VFE.Dir;
+  BFE.ModTime = llvm::sys::toTimeT(Status.getLastModificationTime());
+  BFE.UID = NextFileUID++;
+  BFE.IsValid = true;
+  return &BFE;
+}
+
 bool FileManager::FixupRelativePath(SmallVectorImpl &path) const {
   StringRef pathRef(path.data(), path.size());
 
@@ -532,12 +550,6 @@
 UIDToFiles[VFE->getUID()] = VFE.get();
 }
 
-void FileManager::modifyFileEntry(FileEntry *File,
-  off_t Size, time_t ModificationTime) {
-  File->Size = Size;
-  File->ModTime = ModificationTime;
-}
-
 StringRef FileManager::getCanonicalName(const DirectoryEntry 

r369861 - FileManager: Factor duplicated code in getBufferForFile, NFC

[Duncan P. N. Exon Smith via cfe-commits]

Author: dexonsmith
Date: Sat Aug 24 18:18:35 2019
New Revision: 369861

URL: http://llvm.org/viewvc/llvm-project?rev=369861&view=rev
Log:
FileManager: Factor duplicated code in getBufferForFile, NFC

Incidentally, this also unifies the two versions (removing an
unnecessary call to `SmallString::c_str`).

Modified:
cfe/trunk/include/clang/Basic/FileManager.h
cfe/trunk/lib/Basic/FileManager.cpp

Modified: cfe/trunk/include/clang/Basic/FileManager.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/FileManager.h?rev=369861&r1=369860&r2=369861&view=diff
==
--- cfe/trunk/include/clang/Basic/FileManager.h (original)
+++ cfe/trunk/include/clang/Basic/FileManager.h Sat Aug 24 18:18:35 2019
@@ -311,8 +311,15 @@ public:
   getBufferForFile(const FileEntry *Entry, bool isVolatile = false,
bool ShouldCloseOpenFile = true);
   llvm::ErrorOr>
-  getBufferForFile(StringRef Filename, bool isVolatile = false);
+  getBufferForFile(StringRef Filename, bool isVolatile = false) {
+return getBufferForFileImpl(Filename, /*FileSize=*/-1, isVolatile);
+  }
 
+private:
+  llvm::ErrorOr>
+  getBufferForFileImpl(StringRef Filename, int64_t FileSize, bool isVolatile);
+
+public:
   /// Get the 'stat' information for the given \p Path.
   ///
   /// If the path is relative, it will be resolved against the WorkingDir of 
the

Modified: cfe/trunk/lib/Basic/FileManager.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/FileManager.cpp?rev=369861&r1=369860&r2=369861&view=diff
==
--- cfe/trunk/lib/Basic/FileManager.cpp (original)
+++ cfe/trunk/lib/Basic/FileManager.cpp Sat Aug 24 18:18:35 2019
@@ -447,27 +447,22 @@ FileManager::getBufferForFile(const File
   }
 
   // Otherwise, open the file.
+  return getBufferForFileImpl(Filename, FileSize, isVolatile);
+}
 
+llvm::ErrorOr>
+FileManager::getBufferForFileImpl(StringRef Filename, int64_t FileSize,
+  bool isVolatile) {
   if (FileSystemOpts.WorkingDir.empty())
 return FS->getBufferForFile(Filename, FileSize,
 /*RequiresNullTerminator=*/true, isVolatile);
 
-  SmallString<128> FilePath(Entry->getName());
+  SmallString<128> FilePath(Filename);
   FixupRelativePath(FilePath);
   return FS->getBufferForFile(FilePath, FileSize,
   /*RequiresNullTerminator=*/true, isVolatile);
 }
 
-llvm::ErrorOr>
-FileManager::getBufferForFile(StringRef Filename, bool isVolatile) {
-  if (FileSystemOpts.WorkingDir.empty())
-return FS->getBufferForFile(Filename, -1, true, isVolatile);
-
-  SmallString<128> FilePath(Filename);
-  FixupRelativePath(FilePath);
-  return FS->getBufferForFile(FilePath.c_str(), -1, true, isVolatile);
-}
-
 /// getStatValue - Get the 'stat' information for the specified path,
 /// using the cache to accelerate it if possible.  This returns true
 /// if the path points to a virtual file or does not exist, or returns


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

[PATCH] D66705: FileManager: Use llvm::Expected in new getFileRef API

[Lang Hames via Phabricator via cfe-commits]

lhames added a comment.

In D66705#1644166 , @dexonsmith wrote:

> Updated patch after running `check-clang` and learning more about `Expected`. 
>  I've added a parallel API using `Optional` for clients that 
> don't want to do anything with the error.
>
> @lhames, is it reasonable to add `llvm::expectedToOptional`?


Absolutely: We've already got an errorToBool, so expectedToOptional makes sense.


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

https://reviews.llvm.org/D66705



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

[PATCH] D66705: FileManager: Use llvm::Expected in new getFileRef API

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith updated this revision to Diff 217026.
dexonsmith edited the summary of this revision.
dexonsmith added a comment.

Fix the change to CompilerInstance.cpp.


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

https://reviews.llvm.org/D66705

Files:
  clang/include/clang/Basic/FileManager.h
  clang/lib/Basic/FileManager.cpp
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Lex/HeaderMap.cpp
  clang/lib/Lex/HeaderSearch.cpp
  llvm/include/llvm/Support/Error.h

Index: llvm/include/llvm/Support/Error.h
===
--- llvm/include/llvm/Support/Error.h
+++ llvm/include/llvm/Support/Error.h
@@ -982,6 +982,20 @@
   handleAllErrors(std::move(Err), [](const ErrorInfoBase &) {});
 }
 
+/// Convert an Expected to an Optional without doing anything. This method
+/// should be used only where an error can be considered a reasonable and
+/// expected return value.
+///
+/// Uses of this method are potentially indicative of problems: perhaps the
+/// error should be propagated further, or the error-producer should just
+/// return an Optional in the first place.
+template  Optional expectedToOptional(Expected &&E) {
+  if (E)
+return std::move(*E);
+  consumeError(E.takeError());
+  return None;
+}
+
 /// Helper for converting an Error to a bool.
 ///
 /// This method returns true if Err is in an error state, or false if it is
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -314,7 +314,7 @@
   if (!File) {
 // For rare, surprising errors (e.g. "out of file handles"), diag the EC
 // message.
-std::error_code EC = File.getError();
+std::error_code EC = llvm::errorToErrorCode(File.takeError());
 if (EC != llvm::errc::no_such_file_or_directory &&
 EC != llvm::errc::invalid_argument &&
 EC != llvm::errc::is_a_directory && EC != llvm::errc::not_a_directory) {
@@ -401,7 +401,7 @@
   FixupSearchPath();
   return *Result;
 }
-  } else if (auto Res = HS.getFileMgr().getFileRef(Dest)) {
+  } else if (auto Res = HS.getFileMgr().getOptionalFileRef(Dest)) {
 FixupSearchPath();
 return *Res;
   }
@@ -553,9 +553,8 @@
 
   FrameworkName.append(Filename.begin()+SlashPos+1, Filename.end());
 
-  llvm::ErrorOr File =
-  FileMgr.getFileRef(FrameworkName, /*OpenFile=*/!SuggestedModule);
-
+  auto File =
+  FileMgr.getOptionalFileRef(FrameworkName, /*OpenFile=*/!SuggestedModule);
   if (!File) {
 // Check "/System/Library/Frameworks/Cocoa.framework/PrivateHeaders/file.h"
 const char *Private = "Private";
@@ -565,7 +564,8 @@
   SearchPath->insert(SearchPath->begin()+OrigSize, Private,
  Private+strlen(Private));
 
-File = FileMgr.getFileRef(FrameworkName, /*OpenFile=*/!SuggestedModule);
+File = FileMgr.getOptionalFileRef(FrameworkName,
+  /*OpenFile=*/!SuggestedModule);
   }
 
   // If we found the header and are allowed to suggest a module, do so now.
@@ -1076,9 +1076,7 @@
   }
 
   HeadersFilename.append(Filename.begin()+SlashPos+1, Filename.end());
-  llvm::ErrorOr File =
-  FileMgr.getFileRef(HeadersFilename, /*OpenFile=*/true);
-
+  auto File = FileMgr.getOptionalFileRef(HeadersFilename, /*OpenFile=*/true);
   if (!File) {
 // Check ".../Frameworks/HIToolbox.framework/PrivateHeaders/HIToolbox.h"
 HeadersFilename = FrameworkName;
@@ -1090,7 +1088,7 @@
 }
 
 HeadersFilename.append(Filename.begin()+SlashPos+1, Filename.end());
-File = FileMgr.getFileRef(HeadersFilename, /*OpenFile=*/true);
+File = FileMgr.getOptionalFileRef(HeadersFilename, /*OpenFile=*/true);
 
 if (!File)
   return None;
Index: clang/lib/Lex/HeaderMap.cpp
===
--- clang/lib/Lex/HeaderMap.cpp
+++ clang/lib/Lex/HeaderMap.cpp
@@ -204,9 +204,7 @@
   if (Dest.empty())
 return None;
 
-  if (auto File = FM.getFileRef(Dest))
-return *File;
-  return None;
+  return FM.getOptionalFileRef(Dest);
 }
 
 StringRef HeaderMapImpl::lookupFilename(StringRef Filename,
Index: clang/lib/Frontend/CompilerInstance.cpp
===
--- clang/lib/Frontend/CompilerInstance.cpp
+++ clang/lib/Frontend/CompilerInstance.cpp
@@ -833,6 +833,8 @@
   if (InputFile != "-") {
 auto FileOrErr = FileMgr.getFileRef(InputFile, /*OpenFile=*/true);
 if (!FileOrErr) {
+  // FIXME: include the error in the diagnostic.
+  consumeError(FileOrErr.takeError());
   Diags.Report(diag::err_fe_error_reading) << InputFile;
   return false;
 }
Index: clang/lib/Basic/FileManager.cpp
===
--- clang/lib/Basic/FileManager.cpp
+++ clang/lib/Basic/FileManager.cpp
@@ -187,10 +187,10 @@
   auto Result = getFileRef(Filename, op

[PATCH] D66705: FileManager: Use llvm::Expected in new getFileRef API

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith updated this revision to Diff 217025.
dexonsmith marked an inline comment as done.
dexonsmith edited the summary of this revision.
dexonsmith added a comment.

Also update `FileEntryRef` to be copy- and move-assignable so that 
`Optional` can be assigned to.


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

https://reviews.llvm.org/D66705

Files:
  clang/include/clang/Basic/FileManager.h
  clang/lib/Basic/FileManager.cpp
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Lex/HeaderMap.cpp
  clang/lib/Lex/HeaderSearch.cpp
  llvm/include/llvm/Support/Error.h

Index: llvm/include/llvm/Support/Error.h
===
--- llvm/include/llvm/Support/Error.h
+++ llvm/include/llvm/Support/Error.h
@@ -982,6 +982,20 @@
   handleAllErrors(std::move(Err), [](const ErrorInfoBase &) {});
 }
 
+/// Convert an Expected to an Optional without doing anything. This method
+/// should be used only where an error can be considered a reasonable and
+/// expected return value.
+///
+/// Uses of this method are potentially indicative of problems: perhaps the
+/// error should be propagated further, or the error-producer should just
+/// return an Optional in the first place.
+template  Optional expectedToOptional(Expected &&E) {
+  if (E)
+return std::move(*E);
+  consumeError(E.takeError());
+  return None;
+}
+
 /// Helper for converting an Error to a bool.
 ///
 /// This method returns true if Err is in an error state, or false if it is
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -314,7 +314,7 @@
   if (!File) {
 // For rare, surprising errors (e.g. "out of file handles"), diag the EC
 // message.
-std::error_code EC = File.getError();
+std::error_code EC = llvm::errorToErrorCode(File.takeError());
 if (EC != llvm::errc::no_such_file_or_directory &&
 EC != llvm::errc::invalid_argument &&
 EC != llvm::errc::is_a_directory && EC != llvm::errc::not_a_directory) {
@@ -401,7 +401,7 @@
   FixupSearchPath();
   return *Result;
 }
-  } else if (auto Res = HS.getFileMgr().getFileRef(Dest)) {
+  } else if (auto Res = HS.getFileMgr().getOptionalFileRef(Dest)) {
 FixupSearchPath();
 return *Res;
   }
@@ -553,9 +553,8 @@
 
   FrameworkName.append(Filename.begin()+SlashPos+1, Filename.end());
 
-  llvm::ErrorOr File =
-  FileMgr.getFileRef(FrameworkName, /*OpenFile=*/!SuggestedModule);
-
+  auto File =
+  FileMgr.getOptionalFileRef(FrameworkName, /*OpenFile=*/!SuggestedModule);
   if (!File) {
 // Check "/System/Library/Frameworks/Cocoa.framework/PrivateHeaders/file.h"
 const char *Private = "Private";
@@ -565,7 +564,8 @@
   SearchPath->insert(SearchPath->begin()+OrigSize, Private,
  Private+strlen(Private));
 
-File = FileMgr.getFileRef(FrameworkName, /*OpenFile=*/!SuggestedModule);
+File = FileMgr.getOptionalFileRef(FrameworkName,
+  /*OpenFile=*/!SuggestedModule);
   }
 
   // If we found the header and are allowed to suggest a module, do so now.
@@ -1076,9 +1076,7 @@
   }
 
   HeadersFilename.append(Filename.begin()+SlashPos+1, Filename.end());
-  llvm::ErrorOr File =
-  FileMgr.getFileRef(HeadersFilename, /*OpenFile=*/true);
-
+  auto File = FileMgr.getOptionalFileRef(HeadersFilename, /*OpenFile=*/true);
   if (!File) {
 // Check ".../Frameworks/HIToolbox.framework/PrivateHeaders/HIToolbox.h"
 HeadersFilename = FrameworkName;
@@ -1090,7 +1088,7 @@
 }
 
 HeadersFilename.append(Filename.begin()+SlashPos+1, Filename.end());
-File = FileMgr.getFileRef(HeadersFilename, /*OpenFile=*/true);
+File = FileMgr.getOptionalFileRef(HeadersFilename, /*OpenFile=*/true);
 
 if (!File)
   return None;
Index: clang/lib/Lex/HeaderMap.cpp
===
--- clang/lib/Lex/HeaderMap.cpp
+++ clang/lib/Lex/HeaderMap.cpp
@@ -204,9 +204,7 @@
   if (Dest.empty())
 return None;
 
-  if (auto File = FM.getFileRef(Dest))
-return *File;
-  return None;
+  return FM.getOptionalFileRef(Dest);
 }
 
 StringRef HeaderMapImpl::lookupFilename(StringRef Filename,
Index: clang/lib/Frontend/CompilerInstance.cpp
===
--- clang/lib/Frontend/CompilerInstance.cpp
+++ clang/lib/Frontend/CompilerInstance.cpp
@@ -831,8 +831,10 @@
 
   // Figure out where to get and map in the main file.
   if (InputFile != "-") {
-auto FileOrErr = FileMgr.getFileRef(InputFile, /*OpenFile=*/true);
+auto FileOrErr = FileMgr.getOptionalFileRef(InputFile, /*OpenFile=*/true);
 if (!FileOrErr) {
+  // FIXME: include the error in the diagnostic.
+  consumeError(FileOrErr.takeError());
   Diags.Report(diag::err_fe_error_reading) << InputFile;
   return false;
 

[PATCH] D66700: [Wdocumentation] improve wording of a warning message

[Mark de Wever via Phabricator via cfe-commits]

Mordante added a comment.

Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66700



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

[PATCH] D66705: FileManager: Use llvm::Expected in new getFileRef API

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

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



Comment at: clang/lib/Lex/HeaderSearch.cpp:1079
   if (!File) {
 // Check ".../Frameworks/HIToolbox.framework/PrivateHeaders/HIToolbox.h"
 HeadersFilename = FrameworkName;

arphaman wrote:
> It should be fine to update the return type, but I believe that `Expected` 
> will cause an assertion as the error is unhandled in cases like this one. Can 
> you verify that all errors are consumed somehow?
Yup, when the tests finished running I saw that I'd misunderstood how 
`Expected::operator bool` works.

For the users that don't care about the errors, consuming them is harmful 
boilerplate.  I've switched to a split API that either returns `Expected` or 
`Optional`.  This way clients that do care get the assertions if they mishandle 
it, and the clients that don't annotate that at the call site.  WDYT?


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

https://reviews.llvm.org/D66705



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

[PATCH] D66705: FileManager: Use llvm::Expected in new getFileRef API

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith updated this revision to Diff 217024.
dexonsmith edited the summary of this revision.
dexonsmith added a subscriber: lhames.
dexonsmith added a comment.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Updated patch after running `check-clang` and learning more about `Expected`.  
I've added a parallel API using `Optional` for clients that don't 
want to do anything with the error.

@lhames, is it reasonable to add `llvm::expectedToOptional`?


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

https://reviews.llvm.org/D66705

Files:
  clang/include/clang/Basic/FileManager.h
  clang/lib/Basic/FileManager.cpp
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Lex/HeaderMap.cpp
  clang/lib/Lex/HeaderSearch.cpp
  llvm/include/llvm/Support/Error.h

Index: llvm/include/llvm/Support/Error.h
===
--- llvm/include/llvm/Support/Error.h
+++ llvm/include/llvm/Support/Error.h
@@ -982,6 +982,20 @@
   handleAllErrors(std::move(Err), [](const ErrorInfoBase &) {});
 }
 
+/// Convert an Expected to an Optional without doing anything. This method
+/// should be used only where an error can be considered a reasonable and
+/// expected return value.
+///
+/// Uses of this method are potentially indicative of problems: perhaps the
+/// error should be propagated further, or the error-producer should just
+/// return an Optional in the first place.
+template  Optional expectedToOptional(Expected &&E) {
+  if (E)
+return std::move(*E);
+  consumeError(E.takeError());
+  return None;
+}
+
 /// Helper for converting an Error to a bool.
 ///
 /// This method returns true if Err is in an error state, or false if it is
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -314,7 +314,7 @@
   if (!File) {
 // For rare, surprising errors (e.g. "out of file handles"), diag the EC
 // message.
-std::error_code EC = File.getError();
+std::error_code EC = llvm::errorToErrorCode(File.takeError());
 if (EC != llvm::errc::no_such_file_or_directory &&
 EC != llvm::errc::invalid_argument &&
 EC != llvm::errc::is_a_directory && EC != llvm::errc::not_a_directory) {
@@ -401,7 +401,7 @@
   FixupSearchPath();
   return *Result;
 }
-  } else if (auto Res = HS.getFileMgr().getFileRef(Dest)) {
+  } else if (auto Res = HS.getFileMgr().getOptionalFileRef(Dest)) {
 FixupSearchPath();
 return *Res;
   }
@@ -553,9 +553,8 @@
 
   FrameworkName.append(Filename.begin()+SlashPos+1, Filename.end());
 
-  llvm::ErrorOr File =
-  FileMgr.getFileRef(FrameworkName, /*OpenFile=*/!SuggestedModule);
-
+  auto File =
+  FileMgr.getOptionalFileRef(FrameworkName, /*OpenFile=*/!SuggestedModule);
   if (!File) {
 // Check "/System/Library/Frameworks/Cocoa.framework/PrivateHeaders/file.h"
 const char *Private = "Private";
@@ -565,7 +564,8 @@
   SearchPath->insert(SearchPath->begin()+OrigSize, Private,
  Private+strlen(Private));
 
-File = FileMgr.getFileRef(FrameworkName, /*OpenFile=*/!SuggestedModule);
+File = FileMgr.getOptionalFileRef(FrameworkName,
+  /*OpenFile=*/!SuggestedModule);
   }
 
   // If we found the header and are allowed to suggest a module, do so now.
@@ -1076,9 +1076,7 @@
   }
 
   HeadersFilename.append(Filename.begin()+SlashPos+1, Filename.end());
-  llvm::ErrorOr File =
-  FileMgr.getFileRef(HeadersFilename, /*OpenFile=*/true);
-
+  auto File = FileMgr.getOptionalFileRef(HeadersFilename, /*OpenFile=*/true);
   if (!File) {
 // Check ".../Frameworks/HIToolbox.framework/PrivateHeaders/HIToolbox.h"
 HeadersFilename = FrameworkName;
@@ -1090,7 +1088,7 @@
 }
 
 HeadersFilename.append(Filename.begin()+SlashPos+1, Filename.end());
-File = FileMgr.getFileRef(HeadersFilename, /*OpenFile=*/true);
+File = FileMgr.getOptionalFileRef(HeadersFilename, /*OpenFile=*/true);
 
 if (!File)
   return None;
Index: clang/lib/Lex/HeaderMap.cpp
===
--- clang/lib/Lex/HeaderMap.cpp
+++ clang/lib/Lex/HeaderMap.cpp
@@ -204,9 +204,7 @@
   if (Dest.empty())
 return None;
 
-  if (auto File = FM.getFileRef(Dest))
-return *File;
-  return None;
+  return FM.getOptionalFileRef(Dest);
 }
 
 StringRef HeaderMapImpl::lookupFilename(StringRef Filename,
Index: clang/lib/Frontend/CompilerInstance.cpp
===
--- clang/lib/Frontend/CompilerInstance.cpp
+++ clang/lib/Frontend/CompilerInstance.cpp
@@ -831,8 +831,10 @@
 
   // Figure out where to get and map in the main file.
   if (InputFile != "-") {
-auto FileOrErr = FileMgr.getFileRef(InputFile, /*OpenFile=*/true);
+auto FileOrErr = FileMgr.getOptionalFileRef(InputFile, /*OpenFile=*/true)

[PATCH] D66700: [Wdocumentation] improve wording of a warning message

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added a comment.

I will commit on Monday.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66700



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

[PATCH] D66511: [clang-scan-deps] Skip UTF-8 BOM in source minimizer

[Alexandre Ganea via Phabricator via cfe-commits]

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



Comment at: lib/Lex/DependencyDirectivesSourceMinimizer.cpp:822
 bool Minimizer::minimizeImpl(const char *First, const char *const End) {
+  skipUTF8ByteOrderMark(First, End);
   while (First != End)

dexonsmith wrote:
> Is skipping this the right thing, or should it also be copied to the output?
The code in `Lexer::InitLexer()` assumes the files are always encoded as UTF-8, 
it simply skips over the BOM like we do here.


Repository:
  rC Clang

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

https://reviews.llvm.org/D66511



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

[PATCH] D66700: [Wdocumentation] improve wording of a warning message

[Mark de Wever via Phabricator via cfe-commits]

Mordante added a comment.

You're welcome @davezarzycki. Can one of you commit the patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66700



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

[PATCH] D66706: [Wdocumentation] fixes an assertion failure with typedefed function and block pointer

[Mark de Wever via Phabricator via cfe-commits]

Mordante created this revision.
Mordante added a reviewer: gribozavr.
Mordante added a project: clang.

The assertion happens when compiling with -Wdocumentation with variable 
declaration to a typedefed function pointer. I not too familiar with the ObjC 
syntax but first two tests assert without this patch.

Fixes https://bugs.llvm.org/show_bug.cgi?id=42844


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66706

Files:
  clang/lib/AST/Comment.cpp
  clang/lib/AST/CommentSema.cpp
  clang/test/Sema/warn-documentation.cpp
  clang/test/Sema/warn-documentation.m

Index: clang/test/Sema/warn-documentation.m
===
--- clang/test/Sema/warn-documentation.m
+++ clang/test/Sema/warn-documentation.m
@@ -310,3 +310,10 @@
  * now should work too.
  */
 typedef void (^VariadicBlockType)(int a, ...);
+
+// PR42844 - Assertion failures when using typedefed block pointers
+typedef void(^VoidBlockType)();
+typedef VoidBlockType VoidBlockTypeCall();
+VoidBlockTypeCall *d; ///< \return none
+VoidBlockTypeCall ^e; ///< \return none
+VoidBlockTypeCall f;  ///< \return none
Index: clang/test/Sema/warn-documentation.cpp
===
--- clang/test/Sema/warn-documentation.cpp
+++ clang/test/Sema/warn-documentation.cpp
@@ -1362,3 +1362,30 @@
  */
 class EmptyNoteNoCrash {
 };
+
+namespace PR42844 { // Assertion failures when using typedefed function pointers
+typedef void (*AA)();
+typedef AA A();
+A *a1; ///< \return none
+A a2;  ///< \return none
+
+typedef int B();
+B *b; ///< \return none
+
+typedef void C();
+C *c; ///< \return none
+// expected-warning@-1 {{'\return' command used in a comment that is attached to a function returning void}}
+
+using DD = void(*)();
+using D = DD();
+D *d1; ///< \return none
+D d2;  ///< \return none
+
+using E = int();
+E *e; ///< \return none
+
+using F = void();
+F *f; ///< \return none
+// expected-warning@-1 {{'\return' command used in a comment that is attached to a function returning void}}
+
+} // namespace PR42844
Index: clang/lib/AST/CommentSema.cpp
===
--- clang/lib/AST/CommentSema.cpp
+++ clang/lib/AST/CommentSema.cpp
@@ -588,6 +588,8 @@
   if (isObjCPropertyDecl())
 return;
   if (isFunctionDecl() || isFunctionOrBlockPointerVarLikeDecl()) {
+assert(!ThisDeclInfo->ReturnType.isNull() &&
+   "should have a valid return type");
 if (ThisDeclInfo->ReturnType->isVoidType()) {
   unsigned DiagKind;
   switch (ThisDeclInfo->CommentDecl->getKind()) {
Index: clang/lib/AST/Comment.cpp
===
--- clang/lib/AST/Comment.cpp
+++ clang/lib/AST/Comment.cpp
@@ -112,7 +112,8 @@
   return true;
 }
 
-static TypeLoc lookThroughTypedefOrTypeAliasLocs(TypeLoc &SrcTL) {
+static TypeLoc lookThroughTypedefOrTypeAliasLocs(TypeLoc &SrcTL,
+ bool testTypedefTypeLoc) {
   TypeLoc TL = SrcTL.IgnoreParens();
 
   // Look through attribute types.
@@ -136,15 +137,22 @@
 return MemberPointerTL.getPointeeLoc().getUnqualifiedLoc();
   if (ElaboratedTypeLoc ETL = TL.getAs())
 return ETL.getNamedTypeLoc();
+  if (testTypedefTypeLoc)
+if (TypedefTypeLoc TTL = TL.getAs()) {
+  TypedefNameDecl *TND = TTL.getTypedefNameDecl();
+  if (const TypeSourceInfo *TSI = TND->getTypeSourceInfo())
+return TSI->getTypeLoc().getUnqualifiedLoc();
+}
 
   return TL;
 }
 
-static bool getFunctionTypeLoc(TypeLoc TL, FunctionTypeLoc &ResFTL) {
+static bool getFunctionTypeLoc(TypeLoc TL, FunctionTypeLoc &ResFTL,
+   bool testTypedefTypeLoc = false) {
   TypeLoc PrevTL;
   while (PrevTL != TL) {
 PrevTL = TL;
-TL = lookThroughTypedefOrTypeAliasLocs(TL);
+TL = lookThroughTypedefOrTypeAliasLocs(TL, testTypedefTypeLoc);
   }
 
   if (FunctionTypeLoc FTL = TL.getAs()) {
@@ -293,7 +301,10 @@
 if (TSI) {
   TypeLoc TL = TSI->getTypeLoc().getUnqualifiedLoc();
   FunctionTypeLoc FTL;
-  if (getFunctionTypeLoc(TL, FTL)) {
+  // Need to look through the typedef when
+  // Sema::isFunctionOrBlockPointerVarLikeDecl found a FunctionPointerType
+  // or a BlockPointerType else the ReturnType will not be set.
+  if (getFunctionTypeLoc(TL, FTL, true)) {
 ParamVars = FTL.getParams();
 ReturnType = FTL.getReturnLoc().getType();
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r369853 - [clang-tidy] Manually enable exceptions in tesst that uses them

[Benjamin Kramer via cfe-commits]

Author: d0k
Date: Sat Aug 24 10:19:06 2019
New Revision: 369853

URL: http://llvm.org/viewvc/llvm-project?rev=369853&view=rev
Log:
[clang-tidy] Manually enable exceptions in tesst that uses them

Modified:
clang-tools-extra/trunk/test/clang-tidy/duplicate-reports.cpp

Modified: clang-tools-extra/trunk/test/clang-tidy/duplicate-reports.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/duplicate-reports.cpp?rev=369853&r1=369852&r2=369853&view=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/duplicate-reports.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/duplicate-reports.cpp Sat Aug 24 
10:19:06 2019
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s cert-err09-cpp,cert-err61-cpp %t
+// RUN: %check_clang_tidy %s cert-err09-cpp,cert-err61-cpp %t -- -- 
-fexceptions
 
 void alwaysThrows() {
   int ex = 42;


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

[PATCH] D66336: [ASTImporter] Add development internals docs

[Aleksei Sidorin via Phabricator via cfe-commits]

a_sidorin added inline comments.



Comment at: clang/docs/InternalsManual.rst:1470
+*templated* class (the ``CXXRecordDecl``) of a ``ClassTemplateDecl`` with
+``ClassTemplateDecl::getTemplatedDec()``. And we can get back a pointer of the
+"described" class template from the *templated* class:

martong wrote:
> a_sidorin wrote:
> > TemplatedDec_l_?
> Could you please elaborate, I don't get the meaning of the comment.
Sorry, I mean, you probably meant `ClassTemplateDecl::getTemplatedDecl` but 
typed `getTemplatedDec()` (missing 'l' at the end). 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66336



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

[PATCH] D66705: FileManager: Use llvm::Expected in new getFileRef API

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.



Comment at: clang/lib/Lex/HeaderSearch.cpp:1079
   if (!File) {
 // Check ".../Frameworks/HIToolbox.framework/PrivateHeaders/HIToolbox.h"
 HeadersFilename = FrameworkName;

It should be fine to update the return type, but I believe that `Expected` will 
cause an assertion as the error is unhandled in cases like this one. Can you 
verify that all errors are consumed somehow?


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

https://reviews.llvm.org/D66705



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

[PATCH] D66705: FileManager: Use llvm::Expected in new getFileRef API

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added a comment.

@arphaman, is there a reason you think `ErrorOr` is more appropriate long-term 
here?


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

https://reviews.llvm.org/D66705



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

[PATCH] D66705: FileManager: Use llvm::Expected in new getFileRef API

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith created this revision.
dexonsmith added a reviewer: arphaman.
Herald added a subscriber: ributzka.
dexonsmith added a comment.

@arphaman, is there a reason you think `ErrorOr` is more appropriate long-term 
here?


`FileManager::getFileRef` is a modern API which we expect to convert to
over time.  We should modernize the error handling as well, using
`llvm::Expected` instead of `llvm::ErrorOr`.

There's no functionality change intended here; it looks like all the
callers were already checking the error case.


https://reviews.llvm.org/D66705

Files:
  clang/include/clang/Basic/FileManager.h
  clang/lib/Basic/FileManager.cpp
  clang/lib/Lex/HeaderSearch.cpp


Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -314,7 +314,7 @@
   if (!File) {
 // For rare, surprising errors (e.g. "out of file handles"), diag the EC
 // message.
-std::error_code EC = File.getError();
+std::error_code EC = llvm::errorToErrorCode(File.takeError());
 if (EC != llvm::errc::no_such_file_or_directory &&
 EC != llvm::errc::invalid_argument &&
 EC != llvm::errc::is_a_directory && EC != llvm::errc::not_a_directory) 
{
@@ -553,9 +553,7 @@
 
   FrameworkName.append(Filename.begin()+SlashPos+1, Filename.end());
 
-  llvm::ErrorOr File =
-  FileMgr.getFileRef(FrameworkName, /*OpenFile=*/!SuggestedModule);
-
+  auto File = FileMgr.getFileRef(FrameworkName, /*OpenFile=*/!SuggestedModule);
   if (!File) {
 // Check "/System/Library/Frameworks/Cocoa.framework/PrivateHeaders/file.h"
 const char *Private = "Private";
@@ -1076,9 +1074,7 @@
   }
 
   HeadersFilename.append(Filename.begin()+SlashPos+1, Filename.end());
-  llvm::ErrorOr File =
-  FileMgr.getFileRef(HeadersFilename, /*OpenFile=*/true);
-
+  auto File = FileMgr.getFileRef(HeadersFilename, /*OpenFile=*/true);
   if (!File) {
 // Check ".../Frameworks/HIToolbox.framework/PrivateHeaders/HIToolbox.h"
 HeadersFilename = FrameworkName;
Index: clang/lib/Basic/FileManager.cpp
===
--- clang/lib/Basic/FileManager.cpp
+++ clang/lib/Basic/FileManager.cpp
@@ -187,10 +187,10 @@
   auto Result = getFileRef(Filename, openFile, CacheFailure);
   if (Result)
 return &Result->getFileEntry();
-  return Result.getError();
+  return llvm::errorToErrorCode(Result.takeError());
 }
 
-llvm::ErrorOr
+llvm::Expected
 FileManager::getFileRef(StringRef Filename, bool openFile, bool CacheFailure) {
   ++NumFileLookups;
 
@@ -199,7 +199,8 @@
   SeenFileEntries.insert({Filename, std::errc::no_such_file_or_directory});
   if (!SeenFileInsertResult.second) {
 if (!SeenFileInsertResult.first->second)
-  return SeenFileInsertResult.first->second.getError();
+  return llvm::errorCodeToError(
+  SeenFileInsertResult.first->second.getError());
 // Construct and return and FileEntryRef, unless it's a redirect to another
 // filename.
 SeenFileEntryOrRedirect Value = *SeenFileInsertResult.first->second;
@@ -230,7 +231,7 @@
 else
   SeenFileEntries.erase(Filename);
 
-return DirInfoOrErr.getError();
+return llvm::errorCodeToError(DirInfoOrErr.getError());
   }
   const DirectoryEntry *DirInfo = *DirInfoOrErr;
 
@@ -249,7 +250,7 @@
 else
   SeenFileEntries.erase(Filename);
 
-return statError;
+return llvm::errorCodeToError(statError);
   }
 
   assert((openFile || !F) && "undesired open file");
Index: clang/include/clang/Basic/FileManager.h
===
--- clang/include/clang/Basic/FileManager.h
+++ clang/include/clang/Basic/FileManager.h
@@ -284,9 +284,9 @@
   ///
   /// \param CacheFailure If true and the file does not exist, we'll cache
   /// the failure to find this file.
-  llvm::ErrorOr getFileRef(StringRef Filename,
- bool OpenFile = false,
- bool CacheFailure = true);
+  llvm::Expected getFileRef(StringRef Filename,
+  bool OpenFile = false,
+  bool CacheFailure = true);
 
   /// Returns the current file system options
   FileSystemOptions &getFileSystemOpts() { return FileSystemOpts; }


Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -314,7 +314,7 @@
   if (!File) {
 // For rare, surprising errors (e.g. "out of file handles"), diag the EC
 // message.
-std::error_code EC = File.getError();
+std::error_code EC = llvm::errorToErrorCode(File.takeError());
 if (EC != llvm::errc::no_such_file_or_directory &&
 EC != llvm::errc::invalid_argument &&
 EC != llvm::errc::is_a_directory && EC != llvm::errc::not_a_directo

[PATCH] D64671: [clang-tidy] New check: misc-init-local-variables

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/cppcoreguidelines-init-variables.cpp:3
+
+#include 
+

aaron.ballman wrote:
> Eugene.Zelenko wrote:
> > lebedev.ri wrote:
> > > Eugene.Zelenko wrote:
> > > > It'll be better to include cstdint.
> > > I'll correct that comment: tests normally shouldn't depend on system 
> > > headers; can you just mock whatever you need from there?
> > If test is C++, why not to include C++ equivalent of system headers? See 
> > modernize-deprecated-headers.
> Because we want our tests to be hermetic and rely on the system running the 
> test as little as possible.
I'm not commenting on c-vs-c++ header, i'm specifically commenting on an `<>` 
include; unless i'm misremembering this is advised against.


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

https://reviews.llvm.org/D64671



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

[PATCH] D64671: [clang-tidy] New check: misc-init-local-variables

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/cppcoreguidelines-init-variables.cpp:3
+
+#include 
+

Eugene.Zelenko wrote:
> lebedev.ri wrote:
> > Eugene.Zelenko wrote:
> > > It'll be better to include cstdint.
> > I'll correct that comment: tests normally shouldn't depend on system 
> > headers; can you just mock whatever you need from there?
> If test is C++, why not to include C++ equivalent of system headers? See 
> modernize-deprecated-headers.
Because we want our tests to be hermetic and rely on the system running the 
test as little as possible.


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

https://reviews.llvm.org/D64671



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

[PATCH] D64671: [clang-tidy] New check: misc-init-local-variables

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/cppcoreguidelines-init-variables.cpp:3
+
+#include 
+

lebedev.ri wrote:
> Eugene.Zelenko wrote:
> > It'll be better to include cstdint.
> I'll correct that comment: tests normally shouldn't depend on system headers; 
> can you just mock whatever you need from there?
If test is C++, why not to include C++ equivalent of system headers? See 
modernize-deprecated-headers.


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

https://reviews.llvm.org/D64671



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

[PATCH] D64671: [clang-tidy] New check: misc-init-local-variables

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/cppcoreguidelines-init-variables.cpp:3
+
+#include 
+

Eugene.Zelenko wrote:
> It'll be better to include cstdint.
I'll correct that comment: tests normally shouldn't depend on system headers; 
can you just mock whatever you need from there?


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

https://reviews.llvm.org/D64671



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

[PATCH] D64671: [clang-tidy] New check: misc-init-local-variables

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/cppcoreguidelines-init-variables.cpp:3
+
+#include 
+

It'll be better to include cstdint.


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

https://reviews.llvm.org/D64671



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

r369846 - [OpenCL] Microoptimize OCL2Qual a bit

[Benjamin Kramer via cfe-commits]

Author: d0k
Date: Sat Aug 24 06:04:34 2019
New Revision: 369846

URL: http://llvm.org/viewvc/llvm-project?rev=369846&view=rev
Log:
[OpenCL] Microoptimize OCL2Qual a bit

Still not optimal, but makes clang 25k smaller.

Modified:
cfe/trunk/lib/Sema/SemaLookup.cpp
cfe/trunk/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp

Modified: cfe/trunk/lib/Sema/SemaLookup.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLookup.cpp?rev=369846&r1=369845&r2=369846&view=diff
==
--- cfe/trunk/lib/Sema/SemaLookup.cpp (original)
+++ cfe/trunk/lib/Sema/SemaLookup.cpp Sat Aug 24 06:04:34 2019
@@ -686,8 +686,8 @@ LLVM_DUMP_METHOD void LookupResult::dump
 ///of (vector sizes) x (types) .
 static void GetQualTypesForOpenCLBuiltin(
 ASTContext &Context, const OpenCLBuiltinStruct &OpenCLBuiltin,
-unsigned &GenTypeMaxCnt, std::vector &RetTypes,
-SmallVector, 5> &ArgTypes) {
+unsigned &GenTypeMaxCnt, SmallVector &RetTypes,
+SmallVector, 5> &ArgTypes) {
   // Get the QualType instances of the return types.
   unsigned Sig = SignatureTable[OpenCLBuiltin.SigTableIndex];
   OCL2Qual(Context, TypeTable[Sig], RetTypes);
@@ -696,11 +696,11 @@ static void GetQualTypesForOpenCLBuiltin
   // Get the QualType instances of the arguments.
   // First type is the return type, skip it.
   for (unsigned Index = 1; Index < OpenCLBuiltin.NumTypes; Index++) {
-std::vector Ty;
+SmallVector Ty;
 OCL2Qual(Context,
 TypeTable[SignatureTable[OpenCLBuiltin.SigTableIndex + Index]], Ty);
-ArgTypes.push_back(Ty);
 GenTypeMaxCnt = (Ty.size() > GenTypeMaxCnt) ? Ty.size() : GenTypeMaxCnt;
+ArgTypes.push_back(std::move(Ty));
   }
 }
 
@@ -713,11 +713,10 @@ static void GetQualTypesForOpenCLBuiltin
 /// \param FunctionList (out) List of FunctionTypes.
 /// \param RetTypes (in) List of the possible return types.
 /// \param ArgTypes (in) List of the possible types for the arguments.
-static void
-GetOpenCLBuiltinFctOverloads(ASTContext &Context, unsigned GenTypeMaxCnt,
- std::vector &FunctionList,
- std::vector &RetTypes,
- SmallVector, 5> &ArgTypes) {
+static void GetOpenCLBuiltinFctOverloads(
+ASTContext &Context, unsigned GenTypeMaxCnt,
+std::vector &FunctionList, SmallVector &RetTypes,
+SmallVector, 5> &ArgTypes) {
   FunctionProtoType::ExtProtoInfo PI;
   PI.Variadic = false;
 
@@ -765,8 +764,8 @@ static void InsertOCLBuiltinDeclarations
 BuiltinTable[FctIndex + SignatureIndex];
 ASTContext &Context = S.Context;
 
-std::vector RetTypes;
-SmallVector, 5> ArgTypes;
+SmallVector RetTypes;
+SmallVector, 5> ArgTypes;
 
 // Obtain QualType lists for the function signature.
 GetQualTypesForOpenCLBuiltin(Context, OpenCLBuiltin, GenTypeMaxCnt,

Modified: cfe/trunk/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp?rev=369846&r1=369845&r2=369846&view=diff
==
--- cfe/trunk/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp (original)
+++ cfe/trunk/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp Sat Aug 24 06:04:34 
2019
@@ -434,17 +434,17 @@ void BuiltinNameEmitter::EmitQualTypeFin
 // Step 2: Qualifiers and other type properties such as vector size are
 // applied.
 static void OCL2Qual(ASTContext &Context, const OpenCLTypeStruct &Ty,
- std::vector &QT) {
+ llvm::SmallVectorImpl &QT) {
   // Number of scalar types in the GenType.
   unsigned GenTypeNumTypes;
   // Pointer to the list of vector sizes for the GenType.
-  llvm::SmallVector *GenVectorSizes;
+  llvm::ArrayRef GenVectorSizes;
 )";
 
   // Generate list of vector sizes for each generic type.
   for (const auto *VectList : Records.getAllDerivedDefinitions("IntList")) {
-OS << "  llvm::SmallVector List"
-   << VectList->getValueAsString("Name") << "{";
+OS << "  constexpr unsigned List"
+   << VectList->getValueAsString("Name") << "[] = {";
 for (const auto V : VectList->getValueAsListOfInts("List")) {
   OS << V << ", ";
 }
@@ -458,6 +458,7 @@ static void OCL2Qual(ASTContext &Context
   // Switch cases for generic types.
   for (const auto *GenType : Records.getAllDerivedDefinitions("GenericType")) {
 OS << "case OCLT_" << GenType->getValueAsString("Name") << ":\n";
+OS << "  QT.append({";
 
 // Build the Cartesian product of (vector sizes) x (types).  Only insert
 // the plain scalar types for now; other type information such as vector
@@ -468,10 +469,11 @@ static void OCL2Qual(ASTContext &Context
  I++) {
   for (const auto *T :
GenType->getValueAsDef("TypeList")->getValueAsListOfDefs("List")) {
-OS << "  QT.push_back(Context."
-

r369845 - [analyzer] Analysis: Fix checker silencing

[Csaba Dabis via cfe-commits]

Author: charusso
Date: Sat Aug 24 05:17:49 2019
New Revision: 369845

URL: http://llvm.org/viewvc/llvm-project?rev=369845&view=rev
Log:
[analyzer] Analysis: Fix checker silencing

Modified:
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/tools/scan-build/bin/scan-build

Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=369845&r1=369844&r2=369845&view=diff
==
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Sat Aug 24 05:17:49 2019
@@ -464,22 +464,6 @@ static void parseAnalyzerConfigs(Analyze
 
   // At this point, AnalyzerOptions is configured. Let's validate some options.
 
-  if (!Diags)
-return;
-
-  if (AnOpts.ShouldTrackConditionsDebug && !AnOpts.ShouldTrackConditions)
-Diags->Report(diag::err_analyzer_config_invalid_input)
-<< "track-conditions-debug" << "'track-conditions' to also be enabled";
-
-  if (!AnOpts.CTUDir.empty() && !llvm::sys::fs::is_directory(AnOpts.CTUDir))
-Diags->Report(diag::err_analyzer_config_invalid_input) << "ctu-dir"
-   << "a filename";
-
-  if (!AnOpts.ModelPath.empty() &&
-  !llvm::sys::fs::is_directory(AnOpts.ModelPath))
-Diags->Report(diag::err_analyzer_config_invalid_input) << "model-path"
-   << "a filename";
-
   // FIXME: Here we try to validate the silenced checkers or packages are 
valid.
   // The current approach only validates the registered checkers which does not
   // contain the runtime enabled checkers and optimally we would validate both.
@@ -493,18 +477,37 @@ static void parseAnalyzerConfigs(Analyze
 AnOpts.RawSilencedCheckersAndPackages.split(CheckersAndPackages, ";");
 
 for (const StringRef CheckerOrPackage : CheckersAndPackages) {
-  bool IsChecker = CheckerOrPackage.contains('.');
-  bool IsValidName =
-  IsChecker ? llvm::find(Checkers, CheckerOrPackage) != Checkers.end()
-: llvm::find(Packages, CheckerOrPackage) != Packages.end();
-
-  if (!IsValidName)
-Diags->Report(diag::err_unknown_analyzer_checker_or_package)
-<< CheckerOrPackage;
+  if (Diags) {
+bool IsChecker = CheckerOrPackage.contains('.');
+bool IsValidName =
+IsChecker
+? llvm::find(Checkers, CheckerOrPackage) != Checkers.end()
+: llvm::find(Packages, CheckerOrPackage) != Packages.end();
+
+if (!IsValidName)
+  Diags->Report(diag::err_unknown_analyzer_checker_or_package)
+  << CheckerOrPackage;
+  }
 
   AnOpts.SilencedCheckersAndPackages.emplace_back(CheckerOrPackage);
 }
   }
+
+  if (!Diags)
+return;
+
+  if (AnOpts.ShouldTrackConditionsDebug && !AnOpts.ShouldTrackConditions)
+Diags->Report(diag::err_analyzer_config_invalid_input)
+<< "track-conditions-debug" << "'track-conditions' to also be enabled";
+
+  if (!AnOpts.CTUDir.empty() && !llvm::sys::fs::is_directory(AnOpts.CTUDir))
+Diags->Report(diag::err_analyzer_config_invalid_input) << "ctu-dir"
+   << "a filename";
+
+  if (!AnOpts.ModelPath.empty() &&
+  !llvm::sys::fs::is_directory(AnOpts.ModelPath))
+Diags->Report(diag::err_analyzer_config_invalid_input) << "model-path"
+   << "a filename";
 }
 
 static bool ParseMigratorArgs(MigratorOptions &Opts, ArgList &Args) {

Modified: cfe/trunk/tools/scan-build/bin/scan-build
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/scan-build/bin/scan-build?rev=369845&r1=369844&r2=369845&view=diff
==
--- cfe/trunk/tools/scan-build/bin/scan-build (original)
+++ cfe/trunk/tools/scan-build/bin/scan-build Sat Aug 24 05:17:49 2019
@@ -1889,11 +1889,6 @@ foreach (sort { $Options{DisableCheckers
   # Push checkers in order they were disabled.
   push @AnalysesToRun, "-analyzer-disable-checker", $_;
 }
-foreach (sort { $Options{SilenceCheckers}{$a} <=> 
$Options{SilenceCheckers}{$b} }
- keys %{$Options{SilenceCheckers}}) {
-  # Push checkers in order they were silenced.
-  push @AnalysesToRun, "-analyzer-config silence-checker", $_;
-}
 if ($Options{AnalyzeHeaders}) { push @AnalysesToRun, 
"-analyzer-opt-analyze-headers"; }
 if ($Options{AnalyzerStats}) { push @AnalysesToRun, 
'-analyzer-checker=debug.Stats'; }
 if ($Options{MaxLoop} > 0) { push @AnalysesToRun, "-analyzer-max-loop 
$Options{MaxLoop}"; }
@@ -1903,6 +1898,14 @@ if ($Options{MaxLoop} > 0) { push @Analy
 my $CCC_ANALYZER_ANALYSIS = join ' ', @AnalysesToRun;
 my $CCC_ANALYZER_PLUGINS = join ' ', map { "-load ".$_ } 
@{$Options{PluginsToLoad}};
 my $CCC_ANALYZ

[PATCH] D66700: [Wdocumentation] improve wording of a warning message

[David Zarzycki via Phabricator via cfe-commits]

davezarzycki added a comment.

Works for me. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66700



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

[PATCH] D64671: [clang-tidy] New check: misc-init-local-variables

[Jussi Pakkanen via Phabricator via cfe-commits]

jpakkane added a comment.
Herald added a subscriber: wuzish.

Does this still need work? FWICT all issues raised have been fixed.


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

https://reviews.llvm.org/D64671



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

[PATCH] D64696: Adds a warning when an inline Doxygen comment has no argument

[Mark de Wever via Phabricator via cfe-commits]

Mordante added a comment.

I created D66700  with an improved warning 
message.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64696



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

[PATCH] D66700: [Wdocumentation] improve wording of a warning message

[Mark de Wever via Phabricator via cfe-commits]

Mordante created this revision.
Mordante added reviewers: gribozavr, davezarzycki.
Mordante added a project: clang.

Based on @davezarzycki remarks in D64696  
improved the wording of the warning message.

(Note I don't have commit access)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66700

Files:
  clang/include/clang/Basic/DiagnosticCommentKinds.td
  clang/test/Sema/warn-documentation.cpp


Index: clang/test/Sema/warn-documentation.cpp
===
--- clang/test/Sema/warn-documentation.cpp
+++ clang/test/Sema/warn-documentation.cpp
@@ -1045,49 +1045,49 @@
 void test_attach38::test_attach39(int, B);
 
 // The inline comments expect a string after the command.
-// expected-warning@+1 {{'\a' command does not have an argument}}
+// expected-warning@+1 {{'\a' command does not have a valid word argument}}
 /// \a
 int test_inline_no_argument_a_bad(int);
 
 /// \a A
 int test_inline_no_argument_a_good(int);
 
-// expected-warning@+1 {{'\anchor' command does not have an argument}}
+// expected-warning@+1 {{'\anchor' command does not have a valid word 
argument}}
 /// \anchor
 int test_inline_no_argument_anchor_bad(int);
 
 /// \anchor A
 int test_inline_no_argument_anchor_good(int);
 
-// expected-warning@+1 {{'@b' command does not have an argument}}
+// expected-warning@+1 {{'@b' command does not have a valid word argument}}
 /// @b
 int test_inline_no_argument_b_bad(int);
 
 /// @b A
 int test_inline_no_argument_b_good(int);
 
-// expected-warning@+1 {{'\c' command does not have an argument}}
+// expected-warning@+1 {{'\c' command does not have a valid word argument}}
 /// \c
 int test_inline_no_argument_c_bad(int);
 
 /// \c A
 int test_inline_no_argument_c_good(int);
 
-// expected-warning@+1 {{'\e' command does not have an argument}}
+// expected-warning@+1 {{'\e' command does not have a valid word argument}}
 /// \e
 int test_inline_no_argument_e_bad(int);
 
 /// \e A
 int test_inline_no_argument_e_good(int);
 
-// expected-warning@+1 {{'\em' command does not have an argument}}
+// expected-warning@+1 {{'\em' command does not have a valid word argument}}
 /// \em
 int test_inline_no_argument_em_bad(int);
 
 /// \em A
 int test_inline_no_argument_em_good(int);
 
-// expected-warning@+1 {{'\p' command does not have an argument}}
+// expected-warning@+1 {{'\p' command does not have a valid word argument}}
 /// \p
 int test_inline_no_argument_p_bad(int);
 
Index: clang/include/clang/Basic/DiagnosticCommentKinds.td
===
--- clang/include/clang/Basic/DiagnosticCommentKinds.td
+++ clang/include/clang/Basic/DiagnosticCommentKinds.td
@@ -156,7 +156,7 @@
 // inline contents commands
 
 def warn_doc_inline_contents_no_argument : Warning<
-  "'%select{\\|@}0%1' command does not have an argument">,
+  "'%select{\\|@}0%1' command does not have a valid word argument">,
   InGroup, DefaultIgnore;
 
 // verbatim block commands


Index: clang/test/Sema/warn-documentation.cpp
===
--- clang/test/Sema/warn-documentation.cpp
+++ clang/test/Sema/warn-documentation.cpp
@@ -1045,49 +1045,49 @@
 void test_attach38::test_attach39(int, B);
 
 // The inline comments expect a string after the command.
-// expected-warning@+1 {{'\a' command does not have an argument}}
+// expected-warning@+1 {{'\a' command does not have a valid word argument}}
 /// \a
 int test_inline_no_argument_a_bad(int);
 
 /// \a A
 int test_inline_no_argument_a_good(int);
 
-// expected-warning@+1 {{'\anchor' command does not have an argument}}
+// expected-warning@+1 {{'\anchor' command does not have a valid word argument}}
 /// \anchor
 int test_inline_no_argument_anchor_bad(int);
 
 /// \anchor A
 int test_inline_no_argument_anchor_good(int);
 
-// expected-warning@+1 {{'@b' command does not have an argument}}
+// expected-warning@+1 {{'@b' command does not have a valid word argument}}
 /// @b
 int test_inline_no_argument_b_bad(int);
 
 /// @b A
 int test_inline_no_argument_b_good(int);
 
-// expected-warning@+1 {{'\c' command does not have an argument}}
+// expected-warning@+1 {{'\c' command does not have a valid word argument}}
 /// \c
 int test_inline_no_argument_c_bad(int);
 
 /// \c A
 int test_inline_no_argument_c_good(int);
 
-// expected-warning@+1 {{'\e' command does not have an argument}}
+// expected-warning@+1 {{'\e' command does not have a valid word argument}}
 /// \e
 int test_inline_no_argument_e_bad(int);
 
 /// \e A
 int test_inline_no_argument_e_good(int);
 
-// expected-warning@+1 {{'\em' command does not have an argument}}
+// expected-warning@+1 {{'\em' command does not have a valid word argument}}
 /// \em
 int test_inline_no_argument_em_bad(int);
 
 /// \em A
 int test_inline_no_argument_em_good(int);
 
-// expected-warning@+1 {{'\p' command does not have an argument}}
+// expected-warning@+1 {{'\p' comm

[PATCH] D66067: Push lambda scope earlier when transforming lambda expression

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a reviewer: aaron.ballman.
lebedev.ri added inline comments.



Comment at: clang/test/SemaTemplate/default-arguments-cxx0x.cpp:1
-// RUN: %clang_cc1 -fsyntax-only -std=c++11 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++14 -verify %s
 // expected-no-diagnostics

Mordante wrote:
> comex wrote:
> > Mordante wrote:
> > > Wouldn't it be better to keep the original C++11 test and add a second 
> > > `RUN:` for the C++14 test?
> > > Then guard the new test with `#if __cplusplus >= 201402L`
> > Perhaps, but most tests aren't run multiple times with different -std 
> > options, and I didn't see any reason this test in particular needed that.  
> > Maybe it should just be renamed to default-arguments-cxx14.cpp.
> I'm rather new here so I don't know what the renaming policy is, so I leave 
> it to the more experienced reviewers.
The description does not call it out specifically i think - why does this need 
`-std=c++14`?
That being said i see absolutely no reason not to just have two run lines here.



Comment at: clang/test/SemaTemplate/default-arguments-cxx0x.cpp:118
+
+namespace lambda {
+template 

This is only simply checking that clang doesn't crash on this, right?
Add a comment about that?


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

https://reviews.llvm.org/D66067



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

[PATCH] D66067: Push lambda scope earlier when transforming lambda expression

[Mark de Wever via Phabricator via cfe-commits]

Mordante added inline comments.



Comment at: clang/test/SemaTemplate/default-arguments-cxx0x.cpp:1
-// RUN: %clang_cc1 -fsyntax-only -std=c++11 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++14 -verify %s
 // expected-no-diagnostics

comex wrote:
> Mordante wrote:
> > Wouldn't it be better to keep the original C++11 test and add a second 
> > `RUN:` for the C++14 test?
> > Then guard the new test with `#if __cplusplus >= 201402L`
> Perhaps, but most tests aren't run multiple times with different -std 
> options, and I didn't see any reason this test in particular needed that.  
> Maybe it should just be renamed to default-arguments-cxx14.cpp.
I'm rather new here so I don't know what the renaming policy is, so I leave it 
to the more experienced reviewers.


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

https://reviews.llvm.org/D66067



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

Re: r369830 - NFC: Rename some sanitizer related lifetime checks

[David Zarzycki via cfe-commits]

Fix: r369843

Please confirm. Thanks!

> On Aug 24, 2019, at 8:18 AM, David Zarzycki  wrote:
> 
> This seems to have broken building on Red Hat Fedora Linux 30 (x86_64). Was 
> this expected?
> 
> FAIL: Clang :: CodeGenCXX/lifetime-sanitizer.cpp (7750 of 50751)
>  TEST 'Clang :: CodeGenCXX/lifetime-sanitizer.cpp' FAILED 
> 
> Script:
> --
> : 'RUN: at line 1';   /tmp/_update_lc/r/bin/clang -w -target x86_64-linux-gnu 
> -S -emit-llvm -o - -fno-exceptions -O0 
> /home/dave/s/lp/clang/test/CodeGenCXX/lifetime-sanitizer.cpp |   
> /tmp/_update_lc/r/bin/FileCheck 
> /home/dave/s/lp/clang/test/CodeGenCXX/lifetime-sanitizer.cpp 
> -check-prefixes=CHECK,CHECK-O0 --implicit-check-not=llvm.lifetime
> : 'RUN: at line 3';   /tmp/_update_lc/r/bin/clang -w -target x86_64-linux-gnu 
> -S -emit-llvm -o - -fno-exceptions -O0  -fsanitize=address 
> -fsanitize-address-use-after-scope 
> /home/dave/s/lp/clang/test/CodeGenCXX/lifetime-sanitizer.cpp |  
> /tmp/_update_lc/r/bin/FileCheck 
> /home/dave/s/lp/clang/test/CodeGenCXX/lifetime-sanitizer.cpp 
> -check-prefixes=CHECK,LIFETIME
> --
> Exit Code: 1
> 
> Command Output (stderr):
> --
> /home/dave/s/lp/clang/test/CodeGenCXX/lifetime-sanitizer.cpp:30:18: error: 
> CHECK-LABEL: expected string not found in input
> // CHECK-LABEL: cond.true:
> ^
> :21:8: note: scanning from here
> br i1 %7, label %8, label %10
>   ^
> :25:6: note: possible intended match here
> store i1 true, i1* %4, align 1
> ^
> 
> 
> 
>> On Aug 24, 2019, at 2:31 AM, Vitaly Buka via cfe-commits 
>>  wrote:
>> 
>> Author: vitalybuka
>> Date: Fri Aug 23 18:31:38 2019
>> New Revision: 369830
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=369830&view=rev
>> Log:
>> NFC: Rename some sanitizer related lifetime checks
>> 
>> Added:
>>   cfe/trunk/test/CodeGen/lifetime-sanitizer.c
>>   cfe/trunk/test/CodeGenCXX/lifetime-sanitizer.cpp
>> Removed:
>>   cfe/trunk/test/CodeGen/lifetime-asan.c
>>   cfe/trunk/test/CodeGenCXX/lifetime-asan.cpp
>> 
>> Removed: cfe/trunk/test/CodeGen/lifetime-asan.c
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/lifetime-asan.c?rev=369829&view=auto
>> ==
>> --- cfe/trunk/test/CodeGen/lifetime-asan.c (original)
>> +++ cfe/trunk/test/CodeGen/lifetime-asan.c (removed)
>> @@ -1,21 +0,0 @@
>> -// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - -O0 %s | 
>> FileCheck %s -check-prefix=CHECK-O0
>> -// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - -O0 \
>> -// RUN: -fsanitize=address -fsanitize-address-use-after-scope %s | \
>> -// RUN: FileCheck %s -check-prefix=CHECK-ASAN-USE-AFTER-SCOPE
>> -
>> -extern int bar(char *A, int n);
>> -
>> -// CHECK-O0-NOT: @llvm.lifetime.start
>> -int foo(int n) {
>> -  if (n) {
>> -// CHECK-ASAN-USE-AFTER-SCOPE: @llvm.lifetime.start.p0i8(i64 10, i8* 
>> {{.*}})
>> -char A[10];
>> -return bar(A, 1);
>> -// CHECK-ASAN-USE-AFTER-SCOPE: @llvm.lifetime.end.p0i8(i64 10, i8* 
>> {{.*}})
>> -  } else {
>> -// CHECK-ASAN-USE-AFTER-SCOPE: @llvm.lifetime.start.p0i8(i64 20, i8* 
>> {{.*}})
>> -char A[20];
>> -return bar(A, 2);
>> -// CHECK-ASAN-USE-AFTER-SCOPE: @llvm.lifetime.end.p0i8(i64 20, i8* 
>> {{.*}})
>> -  }
>> -}
>> 
>> Added: cfe/trunk/test/CodeGen/lifetime-sanitizer.c
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/lifetime-sanitizer.c?rev=369830&view=auto
>> ==
>> --- cfe/trunk/test/CodeGen/lifetime-sanitizer.c (added)
>> +++ cfe/trunk/test/CodeGen/lifetime-sanitizer.c Fri Aug 23 18:31:38 2019
>> @@ -0,0 +1,21 @@
>> +// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - -O0 %s | 
>> FileCheck %s -check-prefix=CHECK-O0
>> +// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - -O0 \
>> +// RUN: -fsanitize=address -fsanitize-address-use-after-scope %s | \
>> +// RUN: FileCheck %s -check-prefix=LIFETIME
>> +
>> +extern int bar(char *A, int n);
>> +
>> +// CHECK-O0-NOT: @llvm.lifetime.start
>> +int foo(int n) {
>> +  if (n) {
>> +// LIFETIME: @llvm.lifetime.start.p0i8(i64 10, i8* {{.*}})
>> +char A[10];
>> +return bar(A, 1);
>> +// LIFETIME: @llvm.lifetime.end.p0i8(i64 10, i8* {{.*}})
>> +  } else {
>> +// LIFETIME: @llvm.lifetime.start.p0i8(i64 20, i8* {{.*}})
>> +char A[20];
>> +return bar(A, 2);
>> +// LIFETIME: @llvm.lifetime.end.p0i8(i64 20, i8* {{.*}})
>> +  }
>> +}
>> 
>> Removed: cfe/trunk/test/CodeGenCXX/lifetime-asan.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/lifetime-asan.cpp?rev=369829&view=auto
>> ==
>> --- cfe/trunk/test/CodeGenCXX/lifetime-asan.cpp (original)
>> +++ cfe/trunk/test/CodeGenCXX/lifetime-asan.cpp (removed)
>> @@ -1,42 +0,0 @@
>> -// RUN: %clang -target x86

r369843 - [Testing] Unbreak r369830

[David Zarzycki via cfe-commits]

Author: davezarzycki
Date: Sat Aug 24 01:12:51 2019
New Revision: 369843

URL: http://llvm.org/viewvc/llvm-project?rev=369843&view=rev
Log:
[Testing] Unbreak r369830

Modified:
cfe/trunk/test/CodeGenCXX/lifetime-sanitizer.cpp

Modified: cfe/trunk/test/CodeGenCXX/lifetime-sanitizer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/lifetime-sanitizer.cpp?rev=369843&r1=369842&r2=369843&view=diff
==
--- cfe/trunk/test/CodeGenCXX/lifetime-sanitizer.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/lifetime-sanitizer.cpp Sat Aug 24 01:12:51 2019
@@ -3,6 +3,7 @@
 // RUN: %clang -w -target x86_64-linux-gnu -S -emit-llvm -o - -fno-exceptions 
-O0 \
 // RUN: -fsanitize=address -fsanitize-address-use-after-scope %s | \
 // RUN: FileCheck %s -check-prefixes=CHECK,LIFETIME
+// REQUIRES: assertions
 
 extern int bar(char *A, int n);
 


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

[PATCH] D66686: [LifetimeAnalysis] Make it possible to disable the new warnings

[Matthias Gehre via Phabricator via cfe-commits]

mgehre added inline comments.



Comment at: clang/lib/Sema/SemaInit.cpp:6651
   visitLocalsRetainedByReferenceBinding(Path, Arg, RK_ReferenceBinding,
-Visit);
+Visit, true);
 else

Could we have argument comments here `/*EnableLifetimeWarnings=*/true,
and especially below where multiple bool literals are passed?


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

https://reviews.llvm.org/D66686



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

[PATCH] D66662: [clang-format] [PR43100] clang-format C# support does not add a space between "using" and paren

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/unittests/Format/FormatTestCSharp.cpp:169
+TEST_F(FormatTestCSharp, CSharpUsing) {
+  verifyFormat("using (StreamWriter sw = new StreamWriter(filename) { }");
+}

owenpan wrote:
> owenpan wrote:
> > Maybe set `SpaceBeforeParens` to `Always` first in order to really test the 
> > new behavior?
> I meant setting `SpaceBeforeParens` to ~~`Always`~~ `Never`. 
Also, the right parenthesis for the `using` statement is missing.


Repository:
  rC Clang

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

https://reviews.llvm.org/D2



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

Re: r369830 - NFC: Rename some sanitizer related lifetime checks

[David Zarzycki via cfe-commits]

This seems to have broken building on Red Hat Fedora Linux 30 (x86_64). Was 
this expected?

FAIL: Clang :: CodeGenCXX/lifetime-sanitizer.cpp (7750 of 50751)
 TEST 'Clang :: CodeGenCXX/lifetime-sanitizer.cpp' FAILED 

Script:
--
: 'RUN: at line 1';   /tmp/_update_lc/r/bin/clang -w -target x86_64-linux-gnu 
-S -emit-llvm -o - -fno-exceptions -O0 
/home/dave/s/lp/clang/test/CodeGenCXX/lifetime-sanitizer.cpp |   
/tmp/_update_lc/r/bin/FileCheck 
/home/dave/s/lp/clang/test/CodeGenCXX/lifetime-sanitizer.cpp 
-check-prefixes=CHECK,CHECK-O0 --implicit-check-not=llvm.lifetime
: 'RUN: at line 3';   /tmp/_update_lc/r/bin/clang -w -target x86_64-linux-gnu 
-S -emit-llvm -o - -fno-exceptions -O0  -fsanitize=address 
-fsanitize-address-use-after-scope 
/home/dave/s/lp/clang/test/CodeGenCXX/lifetime-sanitizer.cpp |  
/tmp/_update_lc/r/bin/FileCheck 
/home/dave/s/lp/clang/test/CodeGenCXX/lifetime-sanitizer.cpp 
-check-prefixes=CHECK,LIFETIME
--
Exit Code: 1

Command Output (stderr):
--
/home/dave/s/lp/clang/test/CodeGenCXX/lifetime-sanitizer.cpp:30:18: error: 
CHECK-LABEL: expected string not found in input
 // CHECK-LABEL: cond.true:
 ^
:21:8: note: scanning from here
 br i1 %7, label %8, label %10
   ^
:25:6: note: possible intended match here
 store i1 true, i1* %4, align 1
 ^



> On Aug 24, 2019, at 2:31 AM, Vitaly Buka via cfe-commits 
>  wrote:
> 
> Author: vitalybuka
> Date: Fri Aug 23 18:31:38 2019
> New Revision: 369830
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=369830&view=rev
> Log:
> NFC: Rename some sanitizer related lifetime checks
> 
> Added:
>cfe/trunk/test/CodeGen/lifetime-sanitizer.c
>cfe/trunk/test/CodeGenCXX/lifetime-sanitizer.cpp
> Removed:
>cfe/trunk/test/CodeGen/lifetime-asan.c
>cfe/trunk/test/CodeGenCXX/lifetime-asan.cpp
> 
> Removed: cfe/trunk/test/CodeGen/lifetime-asan.c
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/lifetime-asan.c?rev=369829&view=auto
> ==
> --- cfe/trunk/test/CodeGen/lifetime-asan.c (original)
> +++ cfe/trunk/test/CodeGen/lifetime-asan.c (removed)
> @@ -1,21 +0,0 @@
> -// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - -O0 %s | 
> FileCheck %s -check-prefix=CHECK-O0
> -// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - -O0 \
> -// RUN: -fsanitize=address -fsanitize-address-use-after-scope %s | \
> -// RUN: FileCheck %s -check-prefix=CHECK-ASAN-USE-AFTER-SCOPE
> -
> -extern int bar(char *A, int n);
> -
> -// CHECK-O0-NOT: @llvm.lifetime.start
> -int foo(int n) {
> -  if (n) {
> -// CHECK-ASAN-USE-AFTER-SCOPE: @llvm.lifetime.start.p0i8(i64 10, i8* 
> {{.*}})
> -char A[10];
> -return bar(A, 1);
> -// CHECK-ASAN-USE-AFTER-SCOPE: @llvm.lifetime.end.p0i8(i64 10, i8* 
> {{.*}})
> -  } else {
> -// CHECK-ASAN-USE-AFTER-SCOPE: @llvm.lifetime.start.p0i8(i64 20, i8* 
> {{.*}})
> -char A[20];
> -return bar(A, 2);
> -// CHECK-ASAN-USE-AFTER-SCOPE: @llvm.lifetime.end.p0i8(i64 20, i8* 
> {{.*}})
> -  }
> -}
> 
> Added: cfe/trunk/test/CodeGen/lifetime-sanitizer.c
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/lifetime-sanitizer.c?rev=369830&view=auto
> ==
> --- cfe/trunk/test/CodeGen/lifetime-sanitizer.c (added)
> +++ cfe/trunk/test/CodeGen/lifetime-sanitizer.c Fri Aug 23 18:31:38 2019
> @@ -0,0 +1,21 @@
> +// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - -O0 %s | 
> FileCheck %s -check-prefix=CHECK-O0
> +// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - -O0 \
> +// RUN: -fsanitize=address -fsanitize-address-use-after-scope %s | \
> +// RUN: FileCheck %s -check-prefix=LIFETIME
> +
> +extern int bar(char *A, int n);
> +
> +// CHECK-O0-NOT: @llvm.lifetime.start
> +int foo(int n) {
> +  if (n) {
> +// LIFETIME: @llvm.lifetime.start.p0i8(i64 10, i8* {{.*}})
> +char A[10];
> +return bar(A, 1);
> +// LIFETIME: @llvm.lifetime.end.p0i8(i64 10, i8* {{.*}})
> +  } else {
> +// LIFETIME: @llvm.lifetime.start.p0i8(i64 20, i8* {{.*}})
> +char A[20];
> +return bar(A, 2);
> +// LIFETIME: @llvm.lifetime.end.p0i8(i64 20, i8* {{.*}})
> +  }
> +}
> 
> Removed: cfe/trunk/test/CodeGenCXX/lifetime-asan.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/lifetime-asan.cpp?rev=369829&view=auto
> ==
> --- cfe/trunk/test/CodeGenCXX/lifetime-asan.cpp (original)
> +++ cfe/trunk/test/CodeGenCXX/lifetime-asan.cpp (removed)
> @@ -1,42 +0,0 @@
> -// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - -fno-exceptions 
> -O0 %s | FileCheck %s -check-prefixes=CHECK,CHECK-O0 
> --implicit-check-not=llvm.lifetime
> -// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - -fno-exceptions 
> -O0 \
> -// RUN: -fsanitiz