[PATCH] D54529: [clangd] Add USR to textDocument/definition response

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

OK, sounds like we have something to move forward with. I'd suggest we start 
with an operation returning {SymbolID, scope qualifiers, unqualified name, USR} 
and ignoring location for now, unless you have an immediate need. Reason being 
this sidesteps the index question, what the semantics of the location are, 
which redecl to use etc. Hmm, this looks a lot like `SymbolInformation` as used 
by `textDocument/documentSymbol` with a USR extension. We could consider 
reusing that type...

> A declaration in this case is perfectly fine. You can leverage existing code 
> like clang::tooling::getNamedDeclAt that will return the declaration for a 
> location, and that's used by things like clang-rename.

I'd suggest putting this operation in XRefs.cpp and reusing 
`getSymbolAtPosition`, because that's consistent with how we handle other 
"symbol at cursor" operations, and is aware of our preamble handling. If the 
implementation can be improved or reuse other libraries that's a good idea, but 
it should be uniform.
(getNamedDeclAt looks simpler/cleaner, but I assume you want this to work when 
the cursor is on a reference rather than a decl? If I'm reading getNamedDeclAt 
correctly, it doesn't seem to handle that).


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54529



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

[PATCH] D53818: [ASTImporter] Changed use of Import to Import_New in ASTImporter.

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

balazske marked 3 inline comments as done.
balazske added a comment.

If we change signature of `Import` now, other parts of the code (in clang and 
LLDB) would not compile (without changing to use the new kind of Import). If 
there is a `Import_New` the old code can still compile and can be changed later.




Comment at: lib/AST/ASTImporter.cpp:7889
+return NestedNameSpecifier::Create(ToContext, Prefix,
+   Import(FromNNS->getAsIdentifier()));
 

a_sidorin wrote:
> We can use Import_New if we change this code, like it is done below.
For `IdentifierInfo` there is no Import_New because this is the one kind of 
import that can not fail, it remains the same Import as before (does not return 
`Expected`).


Repository:
  rC Clang

https://reviews.llvm.org/D53818



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

[PATCH] D54539: [CodeGen] Replace '@' characters in block descriptors' symbol names with '\1' on ELF targets.

[David Chisnall via Phabricator via cfe-commits]

theraven added inline comments.



Comment at: lib/CodeGen/CGObjCRuntime.h:288
+  /// descriptor's symbol name.
+  virtual std::string getObjCEncodingForBlock(const BlockExpr *BE) const;
+

I'm not sure that this actually belongs in CGObjCRuntime.  The runtime doesn't 
care about the symbol namings, and the need for this mangling applies to any 
ELF platform (it would also apply to Mach-O if Mach-O adopted GNU-style symbol 
versioning).  It probably isn't relevant, for example, with WebAssembly, 
irrespective of the runtime, and it would be relevant for the Apple family of 
runtimes if anyone wanted to use them on ELF.  

If it does go in CGObjCRuntime, I think I'd prefer to have the implementation 
there as well and make it conditional on the object format, not on the runtime. 
 Having a function that maps Objective-C type encodings to valid symbol names 
in CGObjCRuntime would let us do a little bit of cleanup in CGObjCGNU* and 
could also be used in the blocks code.


Repository:
  rC Clang

https://reviews.llvm.org/D54539



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

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan added a comment.

I'd be interested in seeing tests for two saturating unsigned _Fract with and 
without padding.

If the padding case emits a uadd_sat, that seems wrong; uadd_sat doesn't 
saturate on the padding bit, but will saturate the whole number, which can 
result in invalid representation both with or without saturation taking effect.

Common semantics for unsigned types with padding might need a bit of trickery 
to get it to do the right thing.




Comment at: clang/lib/CodeGen/CGExprScalar.cpp:3383
+
+  // onvert the operands to the full precision type.
+  Value *FullLHS = EmitFixedPointConversion(LHS, LHSFixedSema, CommonFixedSema,

Missing a C there.



Comment at: clang/lib/Sema/SemaExpr.cpp:1306
+OtherTy = ResultTy;
+  }
+

leonardchan wrote:
> ebevhan wrote:
> > Does this handle compound assignment? The other functions handle this 
> > specially.
> Currently no. I was initially intending to add addition compound assignment 
> in a different patch, but do you think it would be appropriate to include it 
> here? I imagine the changes to Sema wouldn't be difficult, but would probably 
> require adding a lot more tests. 
That's fine by me, then. Take it in the next patch.



Comment at: clang/test/Frontend/fixed_point_add.c:36
+
+  // To smaller scale and same width
+  // CHECK:  [[SA:%[0-9]+]] = load i16, i16* %sa, align 2

What do these comments refer to? The scale is the same for both operands here.


Repository:
  rC Clang

https://reviews.llvm.org/D53738



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

[PATCH] D53818: [ASTImporter] Changed use of Import to Import_New in ASTImporter.

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

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

- Small style corrections.


Repository:
  rC Clang

https://reviews.llvm.org/D53818

Files:
  include/clang/AST/ASTImporter.h
  lib/AST/ASTImporter.cpp

Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -154,25 +154,6 @@
 return None;
   }
 
-  // FIXME: Temporary until every import returns Expected.
-  template <>
-  LLVM_NODISCARD Error
-  ASTImporter::importInto(SourceLocation &To, const SourceLocation &From) {
-To = Import(From);
-if (From.isValid() && To.isInvalid())
-return llvm::make_error();
-return Error::success();
-  }
-  // FIXME: Temporary until every import returns Expected.
-  template <>
-  LLVM_NODISCARD Error
-  ASTImporter::importInto(QualType &To, const QualType &From) {
-To = Import(From);
-if (!From.isNull() && To.isNull())
-return llvm::make_error();
-return Error::success();
-  }
-
   class ASTNodeImporter : public TypeVisitor,
   public DeclVisitor,
   public StmtVisitor {
@@ -7656,9 +7637,9 @@
 
 ASTImporter::~ASTImporter() = default;
 
-QualType ASTImporter::Import(QualType FromT) {
+Expected ASTImporter::Import_New(QualType FromT) {
   if (FromT.isNull())
-return {};
+return QualType{};
 
   const Type *FromTy = FromT.getTypePtr();
 
@@ -7671,36 +7652,64 @@
   // Import the type
   ASTNodeImporter Importer(*this);
   ExpectedType ToTOrErr = Importer.Visit(FromTy);
-  if (!ToTOrErr) {
-llvm::consumeError(ToTOrErr.takeError());
-return {};
-  }
+  if (!ToTOrErr)
+return ToTOrErr.takeError();
 
   // Record the imported type.
   ImportedTypes[FromTy] = (*ToTOrErr).getTypePtr();
 
   return ToContext.getQualifiedType(*ToTOrErr, FromT.getLocalQualifiers());
 }
+QualType ASTImporter::Import(QualType From) {
+  llvm::Expected To = Import_New(From);
+  if (To)
+return *To;
+  else
+llvm::consumeError(To.takeError());
+  return {};
+}
 
-TypeSourceInfo *ASTImporter::Import(TypeSourceInfo *FromTSI) {
+Expected ASTImporter::Import_New(TypeSourceInfo *FromTSI) {
   if (!FromTSI)
 return FromTSI;
 
   // FIXME: For now we just create a "trivial" type source info based
   // on the type and a single location. Implement a real version of this.
-  QualType T = Import(FromTSI->getType());
-  if (T.isNull())
-return nullptr;
+  ExpectedType TOrErr = Import_New(FromTSI->getType());
+  if (!TOrErr)
+return TOrErr.takeError();
+  ExpectedSLoc BeginLocOrErr = Import_New(FromTSI->getTypeLoc().getBeginLoc());
+  if (!BeginLocOrErr)
+return BeginLocOrErr.takeError();
 
-  return ToContext.getTrivialTypeSourceInfo(
-  T, Import(FromTSI->getTypeLoc().getBeginLoc()));
+  return ToContext.getTrivialTypeSourceInfo(*TOrErr, *BeginLocOrErr);
+}
+TypeSourceInfo *ASTImporter::Import(TypeSourceInfo *From) {
+  llvm::Expected To = Import_New(From);
+  if (To)
+return *To;
+  else
+llvm::consumeError(To.takeError());
+  return nullptr;
 }
 
-Attr *ASTImporter::Import(const Attr *FromAttr) {
+Expected ASTImporter::Import_New(const Attr *FromAttr) {
   Attr *ToAttr = FromAttr->clone(ToContext);
-  ToAttr->setRange(Import(FromAttr->getRange()));
+  if (auto ToRangeOrErr = Import_New(FromAttr->getRange()))
+ToAttr->setRange(*ToRangeOrErr);
+  else
+return ToRangeOrErr.takeError();
+
   return ToAttr;
 }
+Attr *ASTImporter::Import(const Attr *From) {
+  llvm::Expected To = Import_New(From);
+  if (To)
+return *To;
+  else
+llvm::consumeError(To.takeError());
+  return nullptr;
+}
 
 Decl *ASTImporter::GetAlreadyImportedOrNull(Decl *FromD) {
   llvm::DenseMap::iterator Pos = ImportedDecls.find(FromD);
@@ -7715,7 +7724,7 @@
   }
 }
 
-Decl *ASTImporter::Import(Decl *FromD) {
+Expected ASTImporter::Import_New(Decl *FromD) {
   if (!FromD)
 return nullptr;
 
@@ -7729,19 +7738,25 @@
 return ToD;
   }
 
-  // Import the type.
+  // Import the declaration.
   ExpectedDecl ToDOrErr = Importer.Visit(FromD);
-  if (!ToDOrErr) {
-llvm::consumeError(ToDOrErr.takeError());
-return nullptr;
-  }
+  if (!ToDOrErr)
+return ToDOrErr;
   ToD = *ToDOrErr;
 
   // Notify subclasses.
   Imported(FromD, ToD);
 
   updateFlags(FromD, ToD);
-  return ToD;
+  return ToDOrErr;
+}
+Decl *ASTImporter::Import(Decl *From) {
+  llvm::Expected To = Import_New(From);
+  if (To)
+return *To;
+  else
+llvm::consumeError(To.takeError());
+  return nullptr;
 }
 
 Expected ASTImporter::ImportContext(DeclContext *FromDC) {
@@ -7803,29 +7818,35 @@
   return ToDC;
 }
 
-Expr *ASTImporter::Import(Expr *FromE) {
-  if (!FromE)
-return nullptr;
-
-  return cast_or_null(Import(cast(FromE)));
+Expected ASTImporter::Import_New(Expr *FromE) {
+  if (ExpectedStmt ToSOrErr = Import_New(cast_or_null(FromE)))
+return cast_or_null(*ToSOrErr);
+  else
+return ToSOrErr.takeError();
+}
+Expr *AS

[PATCH] D54519: [clangd] Fix no results returned for global symbols in dexp

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: clangd/index/dex/dexp/Dexp.cpp:64
+// add the global scope to the request.
+Request.Scopes = {""};
 

I think the old behavior used `AnyScope` for a unqualified name. Do we want to 
keep that?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54519



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

[PATCH] D53818: [ASTImporter] Changed use of Import to Import_New in ASTImporter.

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

balazske added inline comments.



Comment at: include/clang/AST/ASTImporter.h:192
 ///
-/// \returns the equivalent declaration in the "to" context, or a NULL type
-/// if an error occurred.
+/// \returns The equivalent declaration in the "to" context, or the import
+/// error.

a_sidorin wrote:
> an import error?
I do not know what is the correct, but a specific import error is returned, 
that occurred during the import.



Comment at: include/clang/AST/ASTImporter.h:198
+}
+// FIXME: Remove this version.
 Decl *Import(Decl *FromD);

a_sidorin wrote:
> these versions
If the other comment is changed I can change this too, but alone this is not 
worth it. This is anyway a temporary comment, and if the first `Import` there 
is removed the second can not compile anyway (and the `Import_New` will be the 
`Import` later).


Repository:
  rC Clang

https://reviews.llvm.org/D53818



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

[PATCH] D54519: [clangd] Fix no results returned for global symbols in dexp

[Eric Liu via Phabricator via cfe-commits]

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



Comment at: clangd/index/dex/dexp/Dexp.cpp:64
+// add the global scope to the request.
+Request.Scopes = {""};
 

ioeric wrote:
> I think the old behavior used `AnyScope` for a unqualified name. Do we want 
> to keep that?
nvm, this is `getSymbolIDsFromIndex` not `fuzzyfind`. Looks good.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54519



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

[PATCH] D54572: [WebAssembly] Change type of wake count to unsigned int

[Heejin Ahn via Phabricator via cfe-commits]

aheejin created this revision.
aheejin added a reviewer: dschuff.
Herald added subscribers: cfe-commits, jfb, sunfish, jgravelle-google, sbc100.

We discussed this at the Nov 12th CG meeting, and decided to use the
unsigned semantics for the wake count.
Corresponding spec change:
https://github.com/WebAssembly/threads/pull/110


Repository:
  rC Clang

https://reviews.llvm.org/D54572

Files:
  include/clang/Basic/BuiltinsWebAssembly.def
  test/CodeGen/builtins-wasm.c


Index: test/CodeGen/builtins-wasm.c
===
--- test/CodeGen/builtins-wasm.c
+++ test/CodeGen/builtins-wasm.c
@@ -77,7 +77,7 @@
   // WEBASSEMBLY64: call i32 @llvm.wasm.atomic.wait.i64(i64* %{{.*}}, i64 
%{{.*}}, i64 %{{.*}})
 }
 
-unsigned int atomic_notify(int *addr, int count) {
+unsigned int atomic_notify(int *addr, unsigned int count) {
   return __builtin_wasm_atomic_notify(addr, count);
   // WEBASSEMBLY32: call i32 @llvm.wasm.atomic.notify(i32* %{{.*}}, i32 
%{{.*}})
   // WEBASSEMBLY64: call i32 @llvm.wasm.atomic.notify(i32* %{{.*}}, i32 
%{{.*}})
Index: include/clang/Basic/BuiltinsWebAssembly.def
===
--- include/clang/Basic/BuiltinsWebAssembly.def
+++ include/clang/Basic/BuiltinsWebAssembly.def
@@ -37,7 +37,7 @@
 // Atomic wait and notify.
 BUILTIN(__builtin_wasm_atomic_wait_i32, "ii*iLLi", "n")
 BUILTIN(__builtin_wasm_atomic_wait_i64, "iLLi*LLiLLi", "n")
-BUILTIN(__builtin_wasm_atomic_notify, "Uii*i", "n")
+BUILTIN(__builtin_wasm_atomic_notify, "Uii*Ui", "n")
 
 // Saturating fp-to-int conversions
 BUILTIN(__builtin_wasm_trunc_saturate_s_i32_f32, "if", "nc")


Index: test/CodeGen/builtins-wasm.c
===
--- test/CodeGen/builtins-wasm.c
+++ test/CodeGen/builtins-wasm.c
@@ -77,7 +77,7 @@
   // WEBASSEMBLY64: call i32 @llvm.wasm.atomic.wait.i64(i64* %{{.*}}, i64 %{{.*}}, i64 %{{.*}})
 }
 
-unsigned int atomic_notify(int *addr, int count) {
+unsigned int atomic_notify(int *addr, unsigned int count) {
   return __builtin_wasm_atomic_notify(addr, count);
   // WEBASSEMBLY32: call i32 @llvm.wasm.atomic.notify(i32* %{{.*}}, i32 %{{.*}})
   // WEBASSEMBLY64: call i32 @llvm.wasm.atomic.notify(i32* %{{.*}}, i32 %{{.*}})
Index: include/clang/Basic/BuiltinsWebAssembly.def
===
--- include/clang/Basic/BuiltinsWebAssembly.def
+++ include/clang/Basic/BuiltinsWebAssembly.def
@@ -37,7 +37,7 @@
 // Atomic wait and notify.
 BUILTIN(__builtin_wasm_atomic_wait_i32, "ii*iLLi", "n")
 BUILTIN(__builtin_wasm_atomic_wait_i64, "iLLi*LLiLLi", "n")
-BUILTIN(__builtin_wasm_atomic_notify, "Uii*i", "n")
+BUILTIN(__builtin_wasm_atomic_notify, "Uii*Ui", "n")
 
 // Saturating fp-to-int conversions
 BUILTIN(__builtin_wasm_trunc_saturate_s_i32_f32, "if", "nc")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r346938 - Introduce shard storage to auto-index.

[Kadir Cetinkaya via cfe-commits]

Author: kadircet
Date: Thu Nov 15 02:31:10 2018
New Revision: 346938

URL: http://llvm.org/viewvc/llvm-project?rev=346938&view=rev
Log:
Introduce shard storage to auto-index.

Reviewers: sammccall, ioeric

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

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

Modified:
clang-tools-extra/trunk/clangd/index/Background.cpp
clang-tools-extra/trunk/clangd/index/Background.h
clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp

Modified: clang-tools-extra/trunk/clangd/index/Background.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Background.cpp?rev=346938&r1=346937&r2=346938&view=diff
==
--- clang-tools-extra/trunk/clangd/index/Background.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Background.cpp Thu Nov 15 02:31:10 2018
@@ -26,26 +26,52 @@
 #include "llvm/Support/SHA1.h"
 #include 
 #include 
+#include 
+#include 
 
 using namespace llvm;
 namespace clang {
 namespace clangd {
 
-BackgroundIndex::BackgroundIndex(Context BackgroundContext,
- StringRef ResourceDir,
- const FileSystemProvider &FSProvider,
- ArrayRef URISchemes,
- size_t ThreadPoolSize)
+namespace {
+
+static BackgroundIndex::FileDigest digest(StringRef Content) {
+  return SHA1::hash({(const uint8_t *)Content.data(), Content.size()});
+}
+
+static Optional digestFile(const SourceManager 
&SM,
+FileID FID) {
+  bool Invalid = false;
+  StringRef Content = SM.getBufferData(FID, &Invalid);
+  if (Invalid)
+return None;
+  return digest(Content);
+}
+
+llvm::SmallString<128>
+getShardPathFromFilePath(llvm::SmallString<128> ShardRoot,
+ llvm::StringRef FilePath) {
+  sys::path::append(ShardRoot, sys::path::filename(FilePath) +
+   toHex(digest(FilePath)) + ".idx");
+  return ShardRoot;
+}
+
+} // namespace
+
+BackgroundIndex::BackgroundIndex(
+Context BackgroundContext, StringRef ResourceDir,
+const FileSystemProvider &FSProvider, ArrayRef URISchemes,
+std::unique_ptr IndexShardStorage, size_t ThreadPoolSize)
 : SwapIndex(make_unique()), ResourceDir(ResourceDir),
   FSProvider(FSProvider), BackgroundContext(std::move(BackgroundContext)),
-  URISchemes(URISchemes) {
+  URISchemes(URISchemes), IndexShardStorage(std::move(IndexShardStorage)) {
   assert(ThreadPoolSize > 0 && "Thread pool size can't be zero.");
   while (ThreadPoolSize--) {
 ThreadPool.emplace_back([this] { run(); });
 // Set priority to low, since background indexing is a long running task we
 // do not want to eat up cpu when there are any other high priority 
threads.
 // FIXME: In the future we might want a more general way of handling this 
to
-// support a tasks with various priorities.
+// support tasks with various priorities.
 setThreadPriority(ThreadPool.back(), ThreadPriority::Low);
   }
 }
@@ -123,6 +149,12 @@ void BackgroundIndex::enqueueAll(StringR
 }
 
 void BackgroundIndex::enqueueLocked(tooling::CompileCommand Cmd) {
+  // Initialize storage to project root. Since Initialize is no-op for multiple
+  // calls we can simply call it for each file.
+  if (IndexShardStorage && !IndexShardStorage->initialize(Cmd.Directory)) {
+elog("Failed to initialize shard storage");
+IndexShardStorage.reset();
+  }
   Queue.push_back(Bind(
   [this](tooling::CompileCommand Cmd) {
 std::string Filename = Cmd.Filename;
@@ -133,19 +165,6 @@ void BackgroundIndex::enqueueLocked(tool
   std::move(Cmd)));
 }
 
-static BackgroundIndex::FileDigest digest(StringRef Content) {
-  return SHA1::hash({(const uint8_t *)Content.data(), Content.size()});
-}
-
-static Optional digestFile(const SourceManager 
&SM,
-FileID FID) {
-  bool Invalid = false;
-  StringRef Content = SM.getBufferData(FID, &Invalid);
-  if (Invalid)
-return None;
-  return digest(Content);
-}
-
 // Resolves URI to file paths with cache.
 class URIToFileCache {
 public:
@@ -227,14 +246,25 @@ void BackgroundIndex::update(StringRef M
 for (const auto *R : F.second.Refs)
   Refs.insert(RefToIDs[R], *R);
 
+auto SS = llvm::make_unique(std::move(Syms).build());
+auto RS = llvm::make_unique(std::move(Refs).build());
+
+auto Hash = FilesToUpdate.lookup(Path);
+// Put shards into storage for subsequent use.
+// FIXME: Store Hash in the Shard.
+if (IndexShardStorage) {
+  IndexFileOut Shard;
+  Shard.Symbols = SS.get();
+  Shard.Refs = RS.get();
+  IndexShardStorage->storeShard(Path, Shard);
+}
+
 std::lock_guard Lock(DigestsMu);
 // This can override a newer version that is added in another thread,
  

[clang-tools-extra] r346940 - Address comments

[Kadir Cetinkaya via cfe-commits]

Author: kadircet
Date: Thu Nov 15 02:31:19 2018
New Revision: 346940

URL: http://llvm.org/viewvc/llvm-project?rev=346940&view=rev
Log:
Address comments

Modified:
clang-tools-extra/trunk/clangd/index/Background.cpp
clang-tools-extra/trunk/clangd/index/Background.h
clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp

Modified: clang-tools-extra/trunk/clangd/index/Background.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Background.cpp?rev=346940&r1=346939&r2=346940&view=diff
==
--- clang-tools-extra/trunk/clangd/index/Background.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Background.cpp Thu Nov 15 02:31:19 2018
@@ -56,15 +56,77 @@ getShardPathFromFilePath(llvm::SmallStri
   return ShardRoot;
 }
 
+// Uses disk as a storage for index shards. Creates a directory called
+// ".clangd-index/" under the path provided during initialize.
+// Note: Requires initialize to be called before storing or retrieval.
+// This class is thread-safe.
+class DiskBackedIndexStorage : public BackgroundIndexStorage {
+  llvm::SmallString<128> DiskShardRoot;
+
+public:
+  llvm::Expected
+  retrieveShard(llvm::StringRef ShardIdentifier) const override {
+auto ShardPath = getShardPathFromFilePath(DiskShardRoot, ShardIdentifier);
+auto Buffer = MemoryBuffer::getFile(ShardPath);
+if (!Buffer) {
+  elog("Couldn't retrieve {0}: {1}", ShardPath,
+   Buffer.getError().message());
+  return llvm::make_error(Buffer.getError());
+}
+// FIXME: Change readIndexFile to also look at Hash of the source that
+// generated index and skip if there is a mismatch.
+return readIndexFile(Buffer->get()->getBuffer());
+  }
+
+  bool storeShard(llvm::StringRef ShardIdentifier,
+  IndexFileOut Shard) const override {
+auto ShardPath = getShardPathFromFilePath(DiskShardRoot, ShardIdentifier);
+std::error_code EC;
+llvm::raw_fd_ostream OS(ShardPath, EC);
+if (EC) {
+  elog("Failed to open {0} for writing: {1}", ShardPath, EC.message());
+  return false;
+}
+OS << Shard;
+return true;
+  }
+
+  // Initializes DiskShardRoot to (Directory + ".clangd-index/") which is the
+  // base directory for all shard files. After the initialization succeeds all
+  // subsequent calls are no-op.
+  DiskBackedIndexStorage(llvm::StringRef Directory) : DiskShardRoot(Directory) 
{
+sys::path::append(DiskShardRoot, ".clangd-index/");
+if (!llvm::sys::fs::exists(DiskShardRoot)) {
+  std::error_code OK;
+  std::error_code EC = llvm::sys::fs::create_directory(DiskShardRoot);
+  if (EC != OK) {
+elog("Failed to create {0}: {1}", DiskShardRoot, EC.message());
+  }
+}
+  }
+};
+
+SmallString<128> getAbsolutePath(const tooling::CompileCommand &Cmd) {
+  SmallString<128> AbsolutePath;
+  if (sys::path::is_absolute(Cmd.Filename)) {
+AbsolutePath = Cmd.Filename;
+  } else {
+AbsolutePath = Cmd.Directory;
+sys::path::append(AbsolutePath, Cmd.Filename);
+  }
+  return AbsolutePath;
+}
+
 } // namespace
 
-BackgroundIndex::BackgroundIndex(
-Context BackgroundContext, StringRef ResourceDir,
-const FileSystemProvider &FSProvider, ArrayRef URISchemes,
-std::unique_ptr IndexShardStorage, size_t ThreadPoolSize)
+BackgroundIndex::BackgroundIndex(Context BackgroundContext,
+ StringRef ResourceDir,
+ const FileSystemProvider &FSProvider,
+ ArrayRef URISchemes,
+ size_t ThreadPoolSize)
 : SwapIndex(make_unique()), ResourceDir(ResourceDir),
   FSProvider(FSProvider), BackgroundContext(std::move(BackgroundContext)),
-  URISchemes(URISchemes), IndexShardStorage(std::move(IndexShardStorage)) {
+  URISchemes(URISchemes) {
   assert(ThreadPoolSize > 0 && "Thread pool size can't be zero.");
   while (ThreadPoolSize--) {
 ThreadPool.emplace_back([this] { run(); });
@@ -123,19 +185,58 @@ void BackgroundIndex::blockUntilIdleForT
 
 void BackgroundIndex::enqueue(StringRef Directory,
   tooling::CompileCommand Cmd) {
+  auto IndexStorage = BackgroundIndexStorage::getForDirectory(Directory);
+  if (IndexStorage)
+loadShard(IndexStorage.get(), Cmd);
+  else
+elog("No index storage for: {0}", Directory);
   {
 std::lock_guard Lock(QueueMu);
-enqueueLocked(std::move(Cmd));
+enqueueLocked(std::move(Cmd), std::move(IndexStorage));
   }
   QueueCV.notify_all();
 }
 
+void BackgroundIndex::loadShard(BackgroundIndexStorage *IndexStorage,
+const tooling::CompileCommand &Cmd) {
+  assert(IndexStorage && "No index storage to load from?");
+  auto AbsolutePath = getAbsolutePath(Cmd);
+  auto Shard = IndexStorage->retrieveShard(AbsolutePath);
+  if (Shard) {
+// FIXME: Updated hashes once we have th

[clang-tools-extra] r346939 - clang-format

[Kadir Cetinkaya via cfe-commits]

Author: kadircet
Date: Thu Nov 15 02:31:15 2018
New Revision: 346939

URL: http://llvm.org/viewvc/llvm-project?rev=346939&view=rev
Log:
clang-format

Modified:
clang-tools-extra/trunk/clangd/index/Background.cpp
clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp

Modified: clang-tools-extra/trunk/clangd/index/Background.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Background.cpp?rev=346939&r1=346938&r2=346939&view=diff
==
--- clang-tools-extra/trunk/clangd/index/Background.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Background.cpp Thu Nov 15 02:31:15 2018
@@ -24,10 +24,10 @@
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/SHA1.h"
+#include 
+#include 
 #include 
 #include 
-#include 
-#include 
 
 using namespace llvm;
 namespace clang {

Modified: clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp?rev=346939&r1=346938&r2=346939&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp Thu Nov 
15 02:31:15 2018
@@ -82,7 +82,7 @@ TEST(BackgroundIndexTest, ShardStorageTe
   class MemoryShardStorage : public ShardStorage {
 mutable std::mutex StorageMu;
 llvm::StringMap &Storage;
-size_t& CacheHits;
+size_t &CacheHits;
 
   public:
 MemoryShardStorage(llvm::StringMap &Storage, size_t 
&CacheHits)
@@ -103,7 +103,7 @@ TEST(BackgroundIndexTest, ShardStorageTe
 return llvm::make_error(
 "Shard not found.", llvm::inconvertibleErrorCode());
   auto IndexFile = readIndexFile(Storage[ShardIdentifier]);
-  if(!IndexFile)
+  if (!IndexFile)
 return IndexFile;
   CacheHits++;
   return IndexFile;


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

[clang-tools-extra] r346941 - Address comments.

[Kadir Cetinkaya via cfe-commits]

Author: kadircet
Date: Thu Nov 15 02:31:23 2018
New Revision: 346941

URL: http://llvm.org/viewvc/llvm-project?rev=346941&view=rev
Log:
Address comments.

Modified:
clang-tools-extra/trunk/clangd/index/Background.cpp
clang-tools-extra/trunk/clangd/index/Background.h
clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp

Modified: clang-tools-extra/trunk/clangd/index/Background.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Background.cpp?rev=346941&r1=346940&r2=346941&view=diff
==
--- clang-tools-extra/trunk/clangd/index/Background.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Background.cpp Thu Nov 15 02:31:23 2018
@@ -48,54 +48,49 @@ static Optional
-getShardPathFromFilePath(llvm::SmallString<128> ShardRoot,
- llvm::StringRef FilePath) {
-  sys::path::append(ShardRoot, sys::path::filename(FilePath) +
-   toHex(digest(FilePath)) + ".idx");
-  return ShardRoot;
+std::string getShardPathFromFilePath(llvm::StringRef ShardRoot,
+ llvm::StringRef FilePath) {
+  llvm::SmallString<128> ShardRootSS(ShardRoot);
+  sys::path::append(ShardRootSS, sys::path::filename(FilePath) +
+ toHex(digest(FilePath)) + ".idx");
+  return ShardRoot.str();
 }
 
 // Uses disk as a storage for index shards. Creates a directory called
-// ".clangd-index/" under the path provided during initialize.
-// Note: Requires initialize to be called before storing or retrieval.
+// ".clangd-index/" under the path provided during construction.
 // This class is thread-safe.
 class DiskBackedIndexStorage : public BackgroundIndexStorage {
-  llvm::SmallString<128> DiskShardRoot;
+  std::string DiskShardRoot;
 
 public:
-  llvm::Expected
-  retrieveShard(llvm::StringRef ShardIdentifier) const override {
-auto ShardPath = getShardPathFromFilePath(DiskShardRoot, ShardIdentifier);
+  llvm::Expected loadShard(llvm::StringRef ShardIdentifier) const 
{
+const std::string ShardPath =
+getShardPathFromFilePath(DiskShardRoot, ShardIdentifier);
 auto Buffer = MemoryBuffer::getFile(ShardPath);
 if (!Buffer) {
-  elog("Couldn't retrieve {0}: {1}", ShardPath,
-   Buffer.getError().message());
+  elog("Couldn't load {0}: {1}", ShardPath, Buffer.getError().message());
   return llvm::make_error(Buffer.getError());
 }
-// FIXME: Change readIndexFile to also look at Hash of the source that
-// generated index and skip if there is a mismatch.
 return readIndexFile(Buffer->get()->getBuffer());
   }
 
-  bool storeShard(llvm::StringRef ShardIdentifier,
-  IndexFileOut Shard) const override {
+  llvm::Error storeShard(llvm::StringRef ShardIdentifier,
+ IndexFileOut Shard) const override {
 auto ShardPath = getShardPathFromFilePath(DiskShardRoot, ShardIdentifier);
 std::error_code EC;
 llvm::raw_fd_ostream OS(ShardPath, EC);
-if (EC) {
-  elog("Failed to open {0} for writing: {1}", ShardPath, EC.message());
-  return false;
-}
+if (EC)
+  return errorCodeToError(EC);
 OS << Shard;
-return true;
+return llvm::Error::success();
   }
 
-  // Initializes DiskShardRoot to (Directory + ".clangd-index/") which is the
-  // base directory for all shard files. After the initialization succeeds all
-  // subsequent calls are no-op.
-  DiskBackedIndexStorage(llvm::StringRef Directory) : DiskShardRoot(Directory) 
{
-sys::path::append(DiskShardRoot, ".clangd-index/");
+  // Sets DiskShardRoot to (Directory + ".clangd-index/") which is the base
+  // directory for all shard files.
+  DiskBackedIndexStorage(llvm::StringRef Directory) {
+llvm::SmallString<128> CDBDirectory(Directory);
+sys::path::append(CDBDirectory, ".clangd-index/");
+DiskShardRoot = CDBDirectory.str();
 if (!llvm::sys::fs::exists(DiskShardRoot)) {
   std::error_code OK;
   std::error_code EC = llvm::sys::fs::create_directory(DiskShardRoot);
@@ -106,7 +101,7 @@ public:
   }
 };
 
-SmallString<128> getAbsolutePath(const tooling::CompileCommand &Cmd) {
+std::string getAbsoluteFilePath(const tooling::CompileCommand &Cmd) {
   SmallString<128> AbsolutePath;
   if (sys::path::is_absolute(Cmd.Filename)) {
 AbsolutePath = Cmd.Filename;
@@ -114,7 +109,7 @@ SmallString<128> getAbsolutePath(const t
 AbsolutePath = Cmd.Directory;
 sys::path::append(AbsolutePath, Cmd.Filename);
   }
-  return AbsolutePath;
+  return AbsolutePath.str();
 }
 
 } // namespace
@@ -123,10 +118,12 @@ BackgroundIndex::BackgroundIndex(Context
  StringRef ResourceDir,
  const FileSystemProvider &FSProvider,
  ArrayRef URISchemes,
+ IndexStorageFactory IndexStorageCreator,
   

[PATCH] D54269: Introduce shard storage to auto-index.

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL346938: Introduce shard storage to auto-index. (authored by 
kadircet, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D54269?vs=174052&id=174172#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D54269

Files:
  clang-tools-extra/trunk/clangd/index/Background.cpp
  clang-tools-extra/trunk/clangd/index/Background.h
  clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp

Index: clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp
@@ -78,5 +78,79 @@
FileURI("unittest:///root/B.cc")}));
 }
 
+TEST(BackgroundIndexTest, ShardStorageTest) {
+  class MemoryShardStorage : public ShardStorage {
+mutable std::mutex StorageMu;
+llvm::StringMap &Storage;
+size_t& CacheHits;
+
+  public:
+MemoryShardStorage(llvm::StringMap &Storage, size_t &CacheHits)
+: Storage(Storage), CacheHits(CacheHits) {}
+
+bool storeShard(llvm::StringRef ShardIdentifier, IndexFileOut Shard) const {
+  std::lock_guard Lock(StorageMu);
+  std::string &str = Storage[ShardIdentifier];
+  llvm::raw_string_ostream OS(str);
+  OS << Shard;
+  OS.flush();
+  return true;
+}
+llvm::Expected retrieveShard(llvm::StringRef ShardIdentifier,
+  FileDigest Hash) const {
+  std::lock_guard Lock(StorageMu);
+  if (Storage.find(ShardIdentifier) == Storage.end())
+return llvm::make_error(
+"Shard not found.", llvm::inconvertibleErrorCode());
+  auto IndexFile = readIndexFile(Storage[ShardIdentifier]);
+  if(!IndexFile)
+return IndexFile;
+  CacheHits++;
+  return IndexFile;
+}
+bool initialize(llvm::StringRef Directory) { return true; }
+  };
+  MockFSProvider FS;
+  FS.Files[testPath("root/A.h")] = R"cpp(
+  void common();
+  void f_b();
+  class A_CC {};
+  )cpp";
+  FS.Files[testPath("root/A.cc")] =
+  "#include \"A.h\"\nvoid g() { (void)common; }";
+  llvm::StringMap Storage;
+  size_t CacheHits = 0;
+  tooling::CompileCommand Cmd;
+  Cmd.Filename = testPath("root/A.cc");
+  Cmd.Directory = testPath("root");
+  Cmd.CommandLine = {"clang++", testPath("root/A.cc")};
+  {
+BackgroundIndex Idx(
+Context::empty(), "", FS, /*URISchemes=*/{"unittest"},
+/*IndexShardStorage=*/
+llvm::make_unique(Storage, CacheHits));
+Idx.enqueue(testPath("root"), Cmd);
+Idx.blockUntilIdleForTest();
+  }
+  EXPECT_EQ(CacheHits, 0U);
+  EXPECT_EQ(Storage.size(), 2U);
+  EXPECT_NE(Storage.find(testPath("root/A.h")), Storage.end());
+  EXPECT_NE(Storage.find(testPath("root/A.cc")), Storage.end());
+
+  {
+BackgroundIndex Idx(
+Context::empty(), "", FS, /*URISchemes=*/{"unittest"},
+/*IndexShardStorage=*/
+llvm::make_unique(Storage, CacheHits));
+Idx.enqueue(testPath("root"), Cmd);
+Idx.blockUntilIdleForTest();
+  }
+  EXPECT_EQ(CacheHits, 1U);
+  EXPECT_EQ(Storage.size(), 2U);
+  EXPECT_NE(Storage.find(testPath("root/A.h")), Storage.end());
+  EXPECT_NE(Storage.find(testPath("root/A.cc")), Storage.end());
+  // B_CC is dropped as we don't collect symbols from A.h in this compilation.
+}
+
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/trunk/clangd/index/Background.cpp
===
--- clang-tools-extra/trunk/clangd/index/Background.cpp
+++ clang-tools-extra/trunk/clangd/index/Background.cpp
@@ -26,26 +26,52 @@
 #include "llvm/Support/SHA1.h"
 #include 
 #include 
+#include 
+#include 
 
 using namespace llvm;
 namespace clang {
 namespace clangd {
 
-BackgroundIndex::BackgroundIndex(Context BackgroundContext,
- StringRef ResourceDir,
- const FileSystemProvider &FSProvider,
- ArrayRef URISchemes,
- size_t ThreadPoolSize)
+namespace {
+
+static BackgroundIndex::FileDigest digest(StringRef Content) {
+  return SHA1::hash({(const uint8_t *)Content.data(), Content.size()});
+}
+
+static Optional digestFile(const SourceManager &SM,
+FileID FID) {
+  bool Invalid = false;
+  StringRef Content = SM.getBufferData(FID, &Invalid);
+  if (Invalid)
+return None;
+  return digest(Content);
+}
+
+llvm::SmallString<128>
+getShardPathFromFilePath(llvm::SmallString<128> ShardRoot,
+ llvm::StringRef FilePath) {
+  sys::path::append(ShardRoot, sys::path::filename(FilePath) +
+

[clang-tools-extra] r346945 - Revert "Introduce shard storage to auto-index."

[Kadir Cetinkaya via cfe-commits]

Author: kadircet
Date: Thu Nov 15 02:34:47 2018
New Revision: 346945

URL: http://llvm.org/viewvc/llvm-project?rev=346945&view=rev
Log:
Revert "Introduce shard storage to auto-index."

This reverts commit 6dd1f24aead10a8d375d0311001987198d26e900.

Modified:
clang-tools-extra/trunk/clangd/index/Background.cpp
clang-tools-extra/trunk/clangd/index/Background.h
clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp

Modified: clang-tools-extra/trunk/clangd/index/Background.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Background.cpp?rev=346945&r1=346944&r2=346945&view=diff
==
--- clang-tools-extra/trunk/clangd/index/Background.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Background.cpp Thu Nov 15 02:34:47 2018
@@ -26,52 +26,26 @@
 #include "llvm/Support/SHA1.h"
 #include 
 #include 
-#include 
-#include 
 
 using namespace llvm;
 namespace clang {
 namespace clangd {
 
-namespace {
-
-static BackgroundIndex::FileDigest digest(StringRef Content) {
-  return SHA1::hash({(const uint8_t *)Content.data(), Content.size()});
-}
-
-static Optional digestFile(const SourceManager 
&SM,
-FileID FID) {
-  bool Invalid = false;
-  StringRef Content = SM.getBufferData(FID, &Invalid);
-  if (Invalid)
-return None;
-  return digest(Content);
-}
-
-llvm::SmallString<128>
-getShardPathFromFilePath(llvm::SmallString<128> ShardRoot,
- llvm::StringRef FilePath) {
-  sys::path::append(ShardRoot, sys::path::filename(FilePath) +
-   toHex(digest(FilePath)) + ".idx");
-  return ShardRoot;
-}
-
-} // namespace
-
-BackgroundIndex::BackgroundIndex(
-Context BackgroundContext, StringRef ResourceDir,
-const FileSystemProvider &FSProvider, ArrayRef URISchemes,
-std::unique_ptr IndexShardStorage, size_t ThreadPoolSize)
+BackgroundIndex::BackgroundIndex(Context BackgroundContext,
+ StringRef ResourceDir,
+ const FileSystemProvider &FSProvider,
+ ArrayRef URISchemes,
+ size_t ThreadPoolSize)
 : SwapIndex(make_unique()), ResourceDir(ResourceDir),
   FSProvider(FSProvider), BackgroundContext(std::move(BackgroundContext)),
-  URISchemes(URISchemes), IndexShardStorage(std::move(IndexShardStorage)) {
+  URISchemes(URISchemes) {
   assert(ThreadPoolSize > 0 && "Thread pool size can't be zero.");
   while (ThreadPoolSize--) {
 ThreadPool.emplace_back([this] { run(); });
 // Set priority to low, since background indexing is a long running task we
 // do not want to eat up cpu when there are any other high priority 
threads.
 // FIXME: In the future we might want a more general way of handling this 
to
-// support tasks with various priorities.
+// support a tasks with various priorities.
 setThreadPriority(ThreadPool.back(), ThreadPriority::Low);
   }
 }
@@ -149,12 +123,6 @@ void BackgroundIndex::enqueueAll(StringR
 }
 
 void BackgroundIndex::enqueueLocked(tooling::CompileCommand Cmd) {
-  // Initialize storage to project root. Since Initialize is no-op for multiple
-  // calls we can simply call it for each file.
-  if (IndexShardStorage && !IndexShardStorage->initialize(Cmd.Directory)) {
-elog("Failed to initialize shard storage");
-IndexShardStorage.reset();
-  }
   Queue.push_back(Bind(
   [this](tooling::CompileCommand Cmd) {
 std::string Filename = Cmd.Filename;
@@ -165,6 +133,19 @@ void BackgroundIndex::enqueueLocked(tool
   std::move(Cmd)));
 }
 
+static BackgroundIndex::FileDigest digest(StringRef Content) {
+  return SHA1::hash({(const uint8_t *)Content.data(), Content.size()});
+}
+
+static Optional digestFile(const SourceManager 
&SM,
+FileID FID) {
+  bool Invalid = false;
+  StringRef Content = SM.getBufferData(FID, &Invalid);
+  if (Invalid)
+return None;
+  return digest(Content);
+}
+
 // Resolves URI to file paths with cache.
 class URIToFileCache {
 public:
@@ -246,25 +227,14 @@ void BackgroundIndex::update(StringRef M
 for (const auto *R : F.second.Refs)
   Refs.insert(RefToIDs[R], *R);
 
-auto SS = llvm::make_unique(std::move(Syms).build());
-auto RS = llvm::make_unique(std::move(Refs).build());
-
-auto Hash = FilesToUpdate.lookup(Path);
-// Put shards into storage for subsequent use.
-// FIXME: Store Hash in the Shard.
-if (IndexShardStorage) {
-  IndexFileOut Shard;
-  Shard.Symbols = SS.get();
-  Shard.Refs = RS.get();
-  IndexShardStorage->storeShard(Path, Shard);
-}
-
 std::lock_guard Lock(DigestsMu);
 // This can override a newer version that is added in another thread,
 // if this thread sees the older version but finishes later. This shoul

[clang-tools-extra] r346944 - Revert "clang-format"

[Kadir Cetinkaya via cfe-commits]

Author: kadircet
Date: Thu Nov 15 02:34:43 2018
New Revision: 346944

URL: http://llvm.org/viewvc/llvm-project?rev=346944&view=rev
Log:
Revert "clang-format"

This reverts commit 0a37e9c3d88a2e21863657df2f7735fb7e5f746e.

Modified:
clang-tools-extra/trunk/clangd/index/Background.cpp
clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp

Modified: clang-tools-extra/trunk/clangd/index/Background.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Background.cpp?rev=346944&r1=346943&r2=346944&view=diff
==
--- clang-tools-extra/trunk/clangd/index/Background.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Background.cpp Thu Nov 15 02:34:43 2018
@@ -24,10 +24,10 @@
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/SHA1.h"
-#include 
-#include 
 #include 
 #include 
+#include 
+#include 
 
 using namespace llvm;
 namespace clang {

Modified: clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp?rev=346944&r1=346943&r2=346944&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp Thu Nov 
15 02:34:43 2018
@@ -82,7 +82,7 @@ TEST(BackgroundIndexTest, ShardStorageTe
   class MemoryShardStorage : public ShardStorage {
 mutable std::mutex StorageMu;
 llvm::StringMap &Storage;
-size_t &CacheHits;
+size_t& CacheHits;
 
   public:
 MemoryShardStorage(llvm::StringMap &Storage, size_t 
&CacheHits)
@@ -103,7 +103,7 @@ TEST(BackgroundIndexTest, ShardStorageTe
 return llvm::make_error(
 "Shard not found.", llvm::inconvertibleErrorCode());
   auto IndexFile = readIndexFile(Storage[ShardIdentifier]);
-  if (!IndexFile)
+  if(!IndexFile)
 return IndexFile;
   CacheHits++;
   return IndexFile;


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

[clang-tools-extra] r346942 - Revert "Address comments."

[Kadir Cetinkaya via cfe-commits]

Author: kadircet
Date: Thu Nov 15 02:34:35 2018
New Revision: 346942

URL: http://llvm.org/viewvc/llvm-project?rev=346942&view=rev
Log:
Revert "Address comments."

This reverts commit b43c4d1c731e07172a382567f3146b3c461c5b69.

Modified:
clang-tools-extra/trunk/clangd/index/Background.cpp
clang-tools-extra/trunk/clangd/index/Background.h
clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp

Modified: clang-tools-extra/trunk/clangd/index/Background.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Background.cpp?rev=346942&r1=346941&r2=346942&view=diff
==
--- clang-tools-extra/trunk/clangd/index/Background.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Background.cpp Thu Nov 15 02:34:35 2018
@@ -48,49 +48,54 @@ static Optional ShardRootSS(ShardRoot);
-  sys::path::append(ShardRootSS, sys::path::filename(FilePath) +
- toHex(digest(FilePath)) + ".idx");
-  return ShardRoot.str();
+llvm::SmallString<128>
+getShardPathFromFilePath(llvm::SmallString<128> ShardRoot,
+ llvm::StringRef FilePath) {
+  sys::path::append(ShardRoot, sys::path::filename(FilePath) +
+   toHex(digest(FilePath)) + ".idx");
+  return ShardRoot;
 }
 
 // Uses disk as a storage for index shards. Creates a directory called
-// ".clangd-index/" under the path provided during construction.
+// ".clangd-index/" under the path provided during initialize.
+// Note: Requires initialize to be called before storing or retrieval.
 // This class is thread-safe.
 class DiskBackedIndexStorage : public BackgroundIndexStorage {
-  std::string DiskShardRoot;
+  llvm::SmallString<128> DiskShardRoot;
 
 public:
-  llvm::Expected loadShard(llvm::StringRef ShardIdentifier) const 
{
-const std::string ShardPath =
-getShardPathFromFilePath(DiskShardRoot, ShardIdentifier);
+  llvm::Expected
+  retrieveShard(llvm::StringRef ShardIdentifier) const override {
+auto ShardPath = getShardPathFromFilePath(DiskShardRoot, ShardIdentifier);
 auto Buffer = MemoryBuffer::getFile(ShardPath);
 if (!Buffer) {
-  elog("Couldn't load {0}: {1}", ShardPath, Buffer.getError().message());
+  elog("Couldn't retrieve {0}: {1}", ShardPath,
+   Buffer.getError().message());
   return llvm::make_error(Buffer.getError());
 }
+// FIXME: Change readIndexFile to also look at Hash of the source that
+// generated index and skip if there is a mismatch.
 return readIndexFile(Buffer->get()->getBuffer());
   }
 
-  llvm::Error storeShard(llvm::StringRef ShardIdentifier,
- IndexFileOut Shard) const override {
+  bool storeShard(llvm::StringRef ShardIdentifier,
+  IndexFileOut Shard) const override {
 auto ShardPath = getShardPathFromFilePath(DiskShardRoot, ShardIdentifier);
 std::error_code EC;
 llvm::raw_fd_ostream OS(ShardPath, EC);
-if (EC)
-  return errorCodeToError(EC);
+if (EC) {
+  elog("Failed to open {0} for writing: {1}", ShardPath, EC.message());
+  return false;
+}
 OS << Shard;
-return llvm::Error::success();
+return true;
   }
 
-  // Sets DiskShardRoot to (Directory + ".clangd-index/") which is the base
-  // directory for all shard files.
-  DiskBackedIndexStorage(llvm::StringRef Directory) {
-llvm::SmallString<128> CDBDirectory(Directory);
-sys::path::append(CDBDirectory, ".clangd-index/");
-DiskShardRoot = CDBDirectory.str();
+  // Initializes DiskShardRoot to (Directory + ".clangd-index/") which is the
+  // base directory for all shard files. After the initialization succeeds all
+  // subsequent calls are no-op.
+  DiskBackedIndexStorage(llvm::StringRef Directory) : DiskShardRoot(Directory) 
{
+sys::path::append(DiskShardRoot, ".clangd-index/");
 if (!llvm::sys::fs::exists(DiskShardRoot)) {
   std::error_code OK;
   std::error_code EC = llvm::sys::fs::create_directory(DiskShardRoot);
@@ -101,7 +106,7 @@ public:
   }
 };
 
-std::string getAbsoluteFilePath(const tooling::CompileCommand &Cmd) {
+SmallString<128> getAbsolutePath(const tooling::CompileCommand &Cmd) {
   SmallString<128> AbsolutePath;
   if (sys::path::is_absolute(Cmd.Filename)) {
 AbsolutePath = Cmd.Filename;
@@ -109,7 +114,7 @@ std::string getAbsoluteFilePath(const to
 AbsolutePath = Cmd.Directory;
 sys::path::append(AbsolutePath, Cmd.Filename);
   }
-  return AbsolutePath.str();
+  return AbsolutePath;
 }
 
 } // namespace
@@ -118,12 +123,10 @@ BackgroundIndex::BackgroundIndex(Context
  StringRef ResourceDir,
  const FileSystemProvider &FSProvider,
  ArrayRef URISchemes,
- IndexStorageFactory IndexStorageCreator,
  size_t ThreadPoolSize)
 : 

[clang-tools-extra] r346943 - Revert "Address comments"

[Kadir Cetinkaya via cfe-commits]

Author: kadircet
Date: Thu Nov 15 02:34:39 2018
New Revision: 346943

URL: http://llvm.org/viewvc/llvm-project?rev=346943&view=rev
Log:
Revert "Address comments"

This reverts commit 19a39b14eab2b5339325e276262b177357d6b412.

Modified:
clang-tools-extra/trunk/clangd/index/Background.cpp
clang-tools-extra/trunk/clangd/index/Background.h
clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp

Modified: clang-tools-extra/trunk/clangd/index/Background.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Background.cpp?rev=346943&r1=346942&r2=346943&view=diff
==
--- clang-tools-extra/trunk/clangd/index/Background.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Background.cpp Thu Nov 15 02:34:39 2018
@@ -56,77 +56,15 @@ getShardPathFromFilePath(llvm::SmallStri
   return ShardRoot;
 }
 
-// Uses disk as a storage for index shards. Creates a directory called
-// ".clangd-index/" under the path provided during initialize.
-// Note: Requires initialize to be called before storing or retrieval.
-// This class is thread-safe.
-class DiskBackedIndexStorage : public BackgroundIndexStorage {
-  llvm::SmallString<128> DiskShardRoot;
-
-public:
-  llvm::Expected
-  retrieveShard(llvm::StringRef ShardIdentifier) const override {
-auto ShardPath = getShardPathFromFilePath(DiskShardRoot, ShardIdentifier);
-auto Buffer = MemoryBuffer::getFile(ShardPath);
-if (!Buffer) {
-  elog("Couldn't retrieve {0}: {1}", ShardPath,
-   Buffer.getError().message());
-  return llvm::make_error(Buffer.getError());
-}
-// FIXME: Change readIndexFile to also look at Hash of the source that
-// generated index and skip if there is a mismatch.
-return readIndexFile(Buffer->get()->getBuffer());
-  }
-
-  bool storeShard(llvm::StringRef ShardIdentifier,
-  IndexFileOut Shard) const override {
-auto ShardPath = getShardPathFromFilePath(DiskShardRoot, ShardIdentifier);
-std::error_code EC;
-llvm::raw_fd_ostream OS(ShardPath, EC);
-if (EC) {
-  elog("Failed to open {0} for writing: {1}", ShardPath, EC.message());
-  return false;
-}
-OS << Shard;
-return true;
-  }
-
-  // Initializes DiskShardRoot to (Directory + ".clangd-index/") which is the
-  // base directory for all shard files. After the initialization succeeds all
-  // subsequent calls are no-op.
-  DiskBackedIndexStorage(llvm::StringRef Directory) : DiskShardRoot(Directory) 
{
-sys::path::append(DiskShardRoot, ".clangd-index/");
-if (!llvm::sys::fs::exists(DiskShardRoot)) {
-  std::error_code OK;
-  std::error_code EC = llvm::sys::fs::create_directory(DiskShardRoot);
-  if (EC != OK) {
-elog("Failed to create {0}: {1}", DiskShardRoot, EC.message());
-  }
-}
-  }
-};
-
-SmallString<128> getAbsolutePath(const tooling::CompileCommand &Cmd) {
-  SmallString<128> AbsolutePath;
-  if (sys::path::is_absolute(Cmd.Filename)) {
-AbsolutePath = Cmd.Filename;
-  } else {
-AbsolutePath = Cmd.Directory;
-sys::path::append(AbsolutePath, Cmd.Filename);
-  }
-  return AbsolutePath;
-}
-
 } // namespace
 
-BackgroundIndex::BackgroundIndex(Context BackgroundContext,
- StringRef ResourceDir,
- const FileSystemProvider &FSProvider,
- ArrayRef URISchemes,
- size_t ThreadPoolSize)
+BackgroundIndex::BackgroundIndex(
+Context BackgroundContext, StringRef ResourceDir,
+const FileSystemProvider &FSProvider, ArrayRef URISchemes,
+std::unique_ptr IndexShardStorage, size_t ThreadPoolSize)
 : SwapIndex(make_unique()), ResourceDir(ResourceDir),
   FSProvider(FSProvider), BackgroundContext(std::move(BackgroundContext)),
-  URISchemes(URISchemes) {
+  URISchemes(URISchemes), IndexShardStorage(std::move(IndexShardStorage)) {
   assert(ThreadPoolSize > 0 && "Thread pool size can't be zero.");
   while (ThreadPoolSize--) {
 ThreadPool.emplace_back([this] { run(); });
@@ -185,58 +123,19 @@ void BackgroundIndex::blockUntilIdleForT
 
 void BackgroundIndex::enqueue(StringRef Directory,
   tooling::CompileCommand Cmd) {
-  auto IndexStorage = BackgroundIndexStorage::getForDirectory(Directory);
-  if (IndexStorage)
-loadShard(IndexStorage.get(), Cmd);
-  else
-elog("No index storage for: {0}", Directory);
   {
 std::lock_guard Lock(QueueMu);
-enqueueLocked(std::move(Cmd), std::move(IndexStorage));
+enqueueLocked(std::move(Cmd));
   }
   QueueCV.notify_all();
 }
 
-void BackgroundIndex::loadShard(BackgroundIndexStorage *IndexStorage,
-const tooling::CompileCommand &Cmd) {
-  assert(IndexStorage && "No index storage to load from?");
-  auto AbsolutePath = getAbsolutePath(Cmd);
-  auto Shard = IndexStorage->retrieveShard(Abso

[PATCH] D53764: [OpenCL] Enable address spaces for references in C++

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia marked 4 inline comments as done.
Anastasia added a comment.

Do you think there is anything else to do for this patch?




Comment at: lib/Sema/SemaExprCXX.cpp:4289
+ /*BasePath=*/nullptr, CCK)
+   .get();
 

rjmccall wrote:
> Anastasia wrote:
> > rjmccall wrote:
> > > Anastasia wrote:
> > > > rjmccall wrote:
> > > > > Anastasia wrote:
> > > > > > rjmccall wrote:
> > > > > > > Okay.  But if `ToType` *isn't* a reference type, this will never 
> > > > > > > be an address-space conversion.  I feel like this code could be 
> > > > > > > written more clearly to express what it's trying to do.
> > > > > > I hope it makes more sense now. Btw, it also applies to pointer 
> > > > > > type.
> > > > > The logic is wrong for pointer types; if you're converting pointers, 
> > > > > you need to be checking the address space of the pointee type of the 
> > > > > from type.
> > > > > 
> > > > > It sounds like this is totally inadequately tested; please flesh out 
> > > > > the test with all of these cases.  While you're at it, please ensure 
> > > > > that there are tests verifying that we don't allowing address-space 
> > > > > changes in nested positions.
> > > > Thanks for spotting this bug! The generated IR for the test was still 
> > > > correct because AS of `FromType` happened to correctly mismatch AS of 
> > > > pointee of `ToType`.
> > > > 
> > > > I failed to construct the test case where it would miss classifying 
> > > > `addrspacecast` due to OpenCL or C++ sema rules but I managed to add a 
> > > > case in which `addrspacecast` was incorrectly added for pointers where 
> > > > it wasn't needed (see line 36 of the test). I think this code is 
> > > > covered now.
> > > > 
> > > > As for the address space position in pointers, the following test 
> > > > checks the address spaces of pointers in `addrspacecast`. For the other 
> > > > program paths we also have a test with similar checks in 
> > > > `test/CodeGenOpenCL/address-spaces-conversions.cl` that we now run for 
> > > > C++ mode too.
> > > > 
> > > > BTW, while trying to construct a test case for the bug, I have 
> > > > discovered that multiple pointer indirection casting isn't working 
> > > > correctly. I.e. for the following program:
> > > >   kernel void foo(){
> > > >  __private int** loc;
> > > >  int** loc_p = loc;
> > > >  **loc_p = 1;
> > > >   }
> > > > We generate:
> > > >   bitcast i32* addrspace(4)* %0 to i32 addrspace(4)* addrspace(4)*
> > > > in OpenCL C and then perform `store` over pointer in AS 4 (generic). We 
> > > > have now lost the information that the original pointer was in 
> > > > `private` AS and that the adjustment of AS segment has to be performed 
> > > > before accessing memory pointed by the pointer. Based on the current 
> > > > specification of `addrspacecast` in 
> > > > https://llvm.org/docs/LangRef.html#addrspacecast-to-instruction I am 
> > > > not very clear whether it can be used for this case without any 
> > > > modifications or clarifications and also what would happen if there are 
> > > > multiple AS mismatches. I am going to look at this issue separately in 
> > > > more details. In OpenCL C++ an ICE is triggered for this though. Let me 
> > > > know if you have any thoughts on this.
> > > Thanks, the check looks good now.
> > > 
> > > > BTW, while trying to construct a test case for the bug, I have 
> > > > discovered that multiple pointer indirection casting isn't working 
> > > > correctly.
> > > 
> > > This needs to be an error in Sema.  The only qualification conversions 
> > > that should be allowed in general on nested pointers (i.e. on `T` in 
> > > `T**` or `T*&`) are the basic C qualifiers: `const`, `volatile`, and 
> > > `restrict`; any other qualification change there is unsound.
> > I see. I guess it's because C++ rules don't cover address spaces.
> > 
> > It feels like it would be a regression for OpenCL C++ vs OpenCL C to reject 
> > nested pointers with address spaces because it was allowed before. :(
> > 
> > However, the generation for OpenCL C and C are incorrect currently. I will 
> > try to sort that all out as a separate patch though, if it makes sense? 
> C++'s rules assume that qualifiers don't introduce real representation 
> differences and that operations on qualified types are compatible with 
> operations on unqualified types.  That's not true of qualifiers in general: 
> address space qualifiers can change representations, ARC qualifiers can have 
> incompatible semantics, etc.  There is no way to soundly implement a 
> conversion from `__private int **` to `__generic int **`, just there's no way 
> to soundly implement a conversion from `Derived **` to `Base **`.
> 
> If you want to allow this conversion anyway for source-compatibility reasons 
> (and I don't think that's a good idea), it should be a bitcast.
Ok, then `bitcast` is not a good solution because it has an issue of loosin

[PATCH] D53900: [CodeComplete] Penalize inherited ObjC properties for auto-completion

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

As mentioned offline - sounds like someone from Apple might take a look.
If not, ping me again to land!

(Sorry for the delay)


Repository:
  rC Clang

https://reviews.llvm.org/D53900



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

[PATCH] D54475: [clangd] Allow observation of changes to global CDBs.

[Ilya Biryukov via Phabricator via cfe-commits]

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

LGTM




Comment at: clangd/Function.h:147
+private:
+  static_assert(std::is_same::type, T>::value,
+"use a plain type: event values are always passed by const&");

NIT: Maybe move this static_assert to the top of the class?
I'd argue this is part of the public interface as it puts constraints on the 
template parameters.



Comment at: clangd/GlobalCompilationDatabase.cpp:73
+  tooling::CompilationDatabase * CDB = nullptr;
+  bool Cached;
   std::lock_guard Lock(Mutex);

NIT: maybe initialize `Cached` to a specific value as a precaution against 
introducing UB with some future changes? The current behavior itself seems 
correct, it's never accessed when uninitialized, albeit it takes some time to 
figure it out.



Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54475



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

[PATCH] D54553: [clangd] Fix crash hovering on non-decltype trailing return

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a reviewer: ilya-biryukov.
ilya-biryukov added a comment.

This seems ready, so putting myself in as a reviewer.
Let me know if there's more work to do and I you don't want the review yet.

Thanks for the fix, just a single NIT comment.




Comment at: clangd/XRefs.cpp:628
   DeducedType = AT->getDeducedType();
-} else {
+} else if (const DecltypeType *DT = 
dyn_cast(D->getReturnType())) {
   // auto in a trailing return type just points to a DecltypeType and

NIT: use 'auto' here, since the type is spelled explicitly in the initializer.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54553



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

[PATCH] D53900: [CodeComplete] Penalize inherited ObjC properties for auto-completion

[Jan Korous via Phabricator via cfe-commits]

jkorous added reviewers: arphaman, benlangmuir.
jkorous added a comment.

This looks reasonable to me but asked people with more context to take a look.


Repository:
  rC Clang

https://reviews.llvm.org/D53900



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

[clang-tools-extra] r346947 - [clangd] Fix no results returned for global symbols in dexp

[Haojian Wu via cfe-commits]

Author: hokein
Date: Thu Nov 15 04:17:41 2018
New Revision: 346947

URL: http://llvm.org/viewvc/llvm-project?rev=346947&view=rev
Log:
[clangd] Fix no results returned for global symbols in dexp

Summary:
For symbols in global namespace (without any scope), we need to
add global scope "" to the fuzzy request.

Reviewers: ioeric

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

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

Modified:
clang-tools-extra/trunk/clangd/index/dex/dexp/Dexp.cpp

Modified: clang-tools-extra/trunk/clangd/index/dex/dexp/Dexp.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/dex/dexp/Dexp.cpp?rev=346947&r1=346946&r2=346947&view=diff
==
--- clang-tools-extra/trunk/clangd/index/dex/dexp/Dexp.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/dex/dexp/Dexp.cpp Thu Nov 15 04:17:41 
2018
@@ -58,6 +58,10 @@ std::vector getSymbolIDsFromIn
   auto Names = splitQualifiedName(QualifiedName);
   if (IsGlobalScope || !Names.first.empty())
 Request.Scopes = {Names.first};
+  else
+// QualifiedName refers to a symbol in global scope (e.g. "GlobalSymbol"),
+// add the global scope to the request.
+Request.Scopes = {""};
 
   Request.Query = Names.second;
   std::vector SymIDs;


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

[PATCH] D54269: Introduce shard storage to auto-index.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 174187.
kadircet added a comment.

- Change factory design to use llvm::unique_function.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54269

Files:
  clangd/CMakeLists.txt
  clangd/index/Background.cpp
  clangd/index/Background.h
  clangd/index/BackgroundIndexStorage.cpp
  unittests/clangd/BackgroundIndexTests.cpp

Index: unittests/clangd/BackgroundIndexTests.cpp
===
--- unittests/clangd/BackgroundIndexTests.cpp
+++ unittests/clangd/BackgroundIndexTests.cpp
@@ -22,6 +22,25 @@
   return ElementsAre(testing::Pair(_, UnorderedElementsAreArray(Matchers)));
 }
 
+class DummyIndexStorage : public BackgroundIndexStorage {
+public:
+  std::unique_ptr
+  loadShard(llvm::StringRef ShardIdentifier) const override {
+vlog("Dropping loadShard({0})", ShardIdentifier);
+return llvm::make_unique();
+  }
+  llvm::Error storeShard(llvm::StringRef ShardIdentifier,
+ IndexFileOut Shard) const override {
+vlog("Dropping storeShard({0}, ...)", ShardIdentifier);
+return llvm::Error::success();
+  }
+  BackgroundIndexStorage* operator()(llvm::StringRef CDBDirectory) {
+vlog("Creating dummy storage for: {0}", CDBDirectory);
+return this;
+  }
+};
+
+
 TEST(BackgroundIndexTest, IndexTwoFiles) {
   MockFSProvider FS;
   // a.h yields different symbols when included by A.cc vs B.cc.
@@ -43,7 +62,8 @@
   void f_b() {
 (void)common;
   })cpp";
-  BackgroundIndex Idx(Context::empty(), "", FS, /*URISchemes=*/{"unittest"});
+  BackgroundIndex Idx(Context::empty(), "", FS, /*URISchemes=*/{"unittest"},
+  DummyIndexStorage());
 
   tooling::CompileCommand Cmd;
   Cmd.Filename = testPath("root/A.cc");
@@ -76,5 +96,72 @@
FileURI("unittest:///root/B.cc")}));
 }
 
+TEST(BackgroundIndexTest, ShardStorageWriteTest) {
+  class MemoryShardStorage : public BackgroundIndexStorage {
+mutable std::mutex StorageMu;
+llvm::StringMap &Storage;
+size_t &CacheHits;
+
+  public:
+MemoryShardStorage(llvm::StringMap &Storage, size_t &CacheHits)
+: Storage(Storage), CacheHits(CacheHits) {}
+llvm::Error storeShard(llvm::StringRef ShardIdentifier,
+   IndexFileOut Shard) const override {
+  std::lock_guard Lock(StorageMu);
+  std::string &str = Storage[ShardIdentifier];
+  llvm::raw_string_ostream OS(str);
+  OS << Shard;
+  OS.flush();
+  return llvm::Error::success();
+}
+std::unique_ptr
+loadShard(llvm::StringRef ShardIdentifier) const override {
+  std::lock_guard Lock(StorageMu);
+  if (Storage.find(ShardIdentifier) == Storage.end()) {
+ADD_FAILURE() << "Shard " << ShardIdentifier << ": not found.";
+return nullptr;
+  }
+  auto IndexFile = readIndexFile(Storage[ShardIdentifier]);
+  if (!IndexFile) {
+ADD_FAILURE() << "Error while reading " << ShardIdentifier << ':'
+  << IndexFile.takeError();
+return nullptr;
+  }
+  CacheHits++;
+  return llvm::make_unique(std::move(*IndexFile));
+}
+  };
+  MockFSProvider FS;
+  FS.Files[testPath("root/A.h")] = R"cpp(
+  void common();
+  void f_b();
+  class A_CC {};
+  )cpp";
+  FS.Files[testPath("root/A.cc")] =
+  "#include \"A.h\"\nvoid g() { (void)common; }";
+
+  llvm::StringMap Storage;
+  size_t CacheHits = 0;
+  MemoryShardStorage MSS(Storage, CacheHits);
+  BackgroundIndexStorage::Factory MemoryStorageCreator = [&](llvm::StringRef) {
+return &MSS;
+  };
+
+  tooling::CompileCommand Cmd;
+  Cmd.Filename = testPath("root/A.cc");
+  Cmd.Directory = testPath("root");
+  Cmd.CommandLine = {"clang++", testPath("root/A.cc")};
+  // Check nothing is loaded from Storage, but A.cc and A.h has been stored.
+  {
+BackgroundIndex Idx(Context::empty(), "", FS, /*URISchemes=*/{"unittest"},
+std::move(MemoryStorageCreator));
+Idx.enqueue(testPath("root"), Cmd);
+Idx.blockUntilIdleForTest();
+  }
+  EXPECT_EQ(Storage.size(), 2U);
+  EXPECT_NE(Storage.find(testPath("root/A.h")), Storage.end());
+  EXPECT_NE(Storage.find(testPath("root/A.cc")), Storage.end());
+}
+
 } // namespace clangd
 } // namespace clang
Index: clangd/index/BackgroundIndexStorage.cpp
===
--- /dev/null
+++ clangd/index/BackgroundIndexStorage.cpp
@@ -0,0 +1,111 @@
+//== BackgroundIndexStorage.cpp - Provide caching support to BackgroundIndex ==/
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "Logger.h"
+#include "index/Background.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/MemoryBuffer.h"
+

[PATCH] D54519: [clangd] Fix no results returned for global symbols in dexp

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL346947: [clangd] Fix no results returned for global symbols 
in dexp (authored by hokein, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D54519

Files:
  clang-tools-extra/trunk/clangd/index/dex/dexp/Dexp.cpp


Index: clang-tools-extra/trunk/clangd/index/dex/dexp/Dexp.cpp
===
--- clang-tools-extra/trunk/clangd/index/dex/dexp/Dexp.cpp
+++ clang-tools-extra/trunk/clangd/index/dex/dexp/Dexp.cpp
@@ -58,6 +58,10 @@
   auto Names = splitQualifiedName(QualifiedName);
   if (IsGlobalScope || !Names.first.empty())
 Request.Scopes = {Names.first};
+  else
+// QualifiedName refers to a symbol in global scope (e.g. "GlobalSymbol"),
+// add the global scope to the request.
+Request.Scopes = {""};
 
   Request.Query = Names.second;
   std::vector SymIDs;


Index: clang-tools-extra/trunk/clangd/index/dex/dexp/Dexp.cpp
===
--- clang-tools-extra/trunk/clangd/index/dex/dexp/Dexp.cpp
+++ clang-tools-extra/trunk/clangd/index/dex/dexp/Dexp.cpp
@@ -58,6 +58,10 @@
   auto Names = splitQualifiedName(QualifiedName);
   if (IsGlobalScope || !Names.first.empty())
 Request.Scopes = {Names.first};
+  else
+// QualifiedName refers to a symbol in global scope (e.g. "GlobalSymbol"),
+// add the global scope to the request.
+Request.Scopes = {""};
 
   Request.Query = Names.second;
   std::vector SymIDs;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54547: PTH-- Remove feature entirely-

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D54547#1299105, @chandlerc wrote:

> Should likely add release notes about this.
>
> Also might be worth sending a note to cfe-dev as a heads up and give folks 
> some time to say "wait wait".


+1 to both of these points, but if doesn't cause too big of a burden for folks, 
I'm all in favor of it.


Repository:
  rC Clang

https://reviews.llvm.org/D54547



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

[PATCH] D54425: [AArch64] Add aarch64_vector_pcs function attribute to Clang

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Missing sema tests that demonstrate the attribute can only appertain to 
functions and types, accepts no arguments, has the expected semantic behavior 
for typecasts to function pointers, etc.




Comment at: include/clang/Basic/Attr.td:1792
+  let Spellings = [Clang<"aarch64_vector_pcs">];
+  let Documentation = [ AArch64VectorPcsDocs ];
+}

Spurious whitespace around the square brackets.



Comment at: include/clang/Basic/AttrDocs.td:1749-1750
+On 64-bit ARM targets, this argument causes the function to obey the vector
+procedural call standard (VPCS) rules as described in the Vector ABI for
+AArch64. In particular, the register caller/callee saves ratio is set to 16/16.
+  }];

If you can add a link to the vector ABI here, or to something that gives more 
detail about the ABI, that'd be great.


https://reviews.llvm.org/D54425



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

[PATCH] D54547: PTH-- Remove feature entirely-

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added a comment.

I don't think I can comment more broadly on this, but
there is some PTH-related code in `Basic/IdentifierTable`,
in `IdentifierInfo::getNameStart` and `IdentifierInfo::getLength`.
Maybe this should be removed too ?


Repository:
  rC Clang

https://reviews.llvm.org/D54547



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

[PATCH] D50256: [Analyzer] [WIP] Basic support for multiplication and division in the constraint manager (for == and != only)

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 174189.
baloghadamsoftware added a comment.
Herald added a subscriber: gamesh411.

Tests updated to the now default `eagerly assume` mode. Range scaling fixed.


https://reviews.llvm.org/D50256

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h
  lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  lib/StaticAnalyzer/Core/RangedConstraintManager.cpp
  test/Analysis/multiplicative-folding.c

Index: test/Analysis/multiplicative-folding.c
===
--- /dev/null
+++ test/Analysis/multiplicative-folding.c
@@ -0,0 +1,686 @@
+// RUN: %clang_analyze_cc1 --std=c11 -analyzer-checker=core,debug.ExprInspection -verify %s
+
+void clang_analyzer_eval(int);
+void clang_analyzer_warnIfReached(void);
+
+#define UINT_MAX (~0U)
+#define INT_MAX (int)(UINT_MAX & (UINT_MAX >> 1))
+#define INT_MIN (-INT_MAX - 1)
+
+#define ULONG_LONG_MAX (~0UL)
+#define LONG_LONG_MAX (long long)(ULONG_LONG_MAX & (ULONG_LONG_MAX >> 1))
+#define LONG_LONG_MIN (-LONG_LONG_MAX - 1)
+
+extern void __assert_fail (__const char *__assertion, __const char *__file,
+unsigned int __line, __const char *__function)
+ __attribute__ ((__noreturn__));
+#define assert(expr) \
+  ((expr)  ? (void)(0)  : __assert_fail (#expr, __FILE__, __LINE__, __func__))
+
+typedef int int32_t;
+typedef unsigned int uint32_t;
+typedef long long int64_t;
+typedef unsigned long long uint64_t;
+
+void signed_multiplication_eq(int32_t n) {
+  if (n * 2 == 3) {
+clang_analyzer_warnIfReached(); // no-warning
+
+  } else if (n * 2 == 4) {
+const int32_t C1 = 0x8002, C2 = 2;
+
+assert(C1 * 2 == 4);
+assert(C2 * 2 == 4);
+
+clang_analyzer_eval(n == INT_MIN); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C1 - 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C1); //expected-warning{{FALSE}}
+  //expected-warning@-1{{TRUE}}
+clang_analyzer_eval(n == C1 + 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == 0); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C2 - 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C2); //expected-warning{{FALSE}}
+  //expected-warning@-1{{TRUE}}
+clang_analyzer_eval(n == C2 + 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == INT_MAX); //expected-warning{{FALSE}}
+
+  } else if (n * 3 == 4) {
+const int32_t C1 = 0xaaac;
+
+assert(C1 * 3 == 4);
+
+clang_analyzer_eval(n == INT_MIN); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C1 - 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C1); //expected-warning{{TRUE}}
+clang_analyzer_eval(n == C1 + 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == 0); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == INT_MAX); //expected-warning{{FALSE}}
+
+  } else if (n * 4 == -5) {
+clang_analyzer_warnIfReached(); // no-warning
+
+  } else if (n * 4 == -8) {
+const int32_t C1 = 0xbffe, C2 = 0xfffe,
+  C3 = 0x3ffe, C4 = 0x7ffe;
+
+assert(C1 * 4 == -8);
+assert(C2 * 4 == -8);
+assert(C3 * 4 == -8);
+assert(C4 * 4 == -8);
+
+clang_analyzer_eval(n == INT_MIN); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C1 - 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C1); //expected-warning{{FALSE}}
+  //expected-warning@-1{{TRUE}}
+clang_analyzer_eval(n == C1 + 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C2 - 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C2); //expected-warning{{FALSE}}
+  //expected-warning@-1{{TRUE}}
+clang_analyzer_eval(n == C2 + 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == 0); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C3 - 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C3); //expected-warning{{FALSE}}
+  //expected-warning@-1{{TRUE}}
+clang_analyzer_eval(n == C3 + 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C4 - 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C4); //expected-warning{{FALSE}}
+  //expected-warning@-1{{TRUE}}
+clang_analyzer_eval(n == C4 + 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == INT_MAX); //expected-warning{{FALSE}}
+
+  } else if (n * 6 == -7) {
+clang_analyzer_warnIfReached(); // no-warning
+
+  } else if (n * 6 == -2) {
+const int32_t C1 = 0xd555, C2 = 0x;
+
+assert(C1 * 6 == -2);
+assert(C2 * 6 == -2);
+
+clang_analyzer_eval(n == INT_MIN); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C1 - 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C1); //expected-warning{{FALSE}}
+  //expected-

[PATCH] D49074: [Analyzer] [WIP] Basic support for multiplication and division in the constraint manager

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 174190.
baloghadamsoftware added a comment.
Herald added a subscriber: gamesh411.

Rebased on the previous part. Tests updated for the now default `eagerly 
assume` mode.


https://reviews.llvm.org/D49074

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h
  lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  lib/StaticAnalyzer/Core/RangedConstraintManager.cpp
  test/Analysis/multiplicative-folding.c

Index: test/Analysis/multiplicative-folding.c
===
--- test/Analysis/multiplicative-folding.c
+++ test/Analysis/multiplicative-folding.c
@@ -570,6 +570,1478 @@
   }
 }
 
+void signed_multiplication_lt_0(int32_t n) {
+  if (n * 2 < 3) {
+int32_t  U1 = 0x8001,
+L2 = 0xc000, U2 = 1,
+L3 = 0x4000;
+
+assert(INT_MIN * 2 < 3);
+assert(U1 * 2 < 3);
+assert((U1 + 1) * 2 >= 3);
+assert(L2 * 2 < 3);
+assert((L2 - 1) * 2 >= 3);
+assert(U2 * 2 < 3);
+assert((U2 + 1) * 2 >= 3);
+assert(L3 * 2 < 3);
+assert((L3 - 1) * 2 >= 3);
+assert(INT_MAX * 2 < 3);
+
+if (n < INT_MIN / 2) {
+  clang_analyzer_eval(n == INT_MIN); //expected-warning{{FALSE}}
+ //expected-warning@-1{{TRUE}}
+  clang_analyzer_eval(n <= U1); //expected-warning{{TRUE}}
+  clang_analyzer_eval(n > U1); //expected-warning{{FALSE}}
+} else if (n < INT_MAX / 2){
+  clang_analyzer_eval(n < L2); //expected-warning{{FALSE}}
+  clang_analyzer_eval(n >= L2); //expected-warning{{TRUE}}
+  clang_analyzer_eval(n <= U2); //expected-warning{{TRUE}}
+  clang_analyzer_eval(n > U2); //expected-warning{{FALSE}}
+} else {
+  clang_analyzer_eval(n < L3); //expected-warning{{FALSE}}
+  clang_analyzer_eval(n >= L3); //expected-warning{{TRUE}}
+  clang_analyzer_eval(n == INT_MAX); //expected-warning{{FALSE}}
+ //expected-warning@-1{{TRUE}}
+}
+  }
+}
+
+void signed_multiplication_lt_1(int32_t n) {
+  if (n * 2 < 4) {
+int32_t  U1 = 0x8001,
+L2 = 0xc000, U2 = 1,
+L3 = 0x4000;
+
+assert(INT_MIN * 2 < 4);
+assert(U1 * 2 < 4);
+assert((U1 + 1) * 2 >= 4);
+assert(L2 * 2 < 4);
+assert((L2 - 1) * 2 >= 4);
+assert(U2 * 2 < 4);
+assert((U2 + 1) * 2 >= 4);
+assert(L3 * 2 < 4);
+assert((L3 - 1) * 2 >= 4);
+assert(INT_MAX * 2 < 4);
+
+if (n < INT_MIN / 2) {
+  clang_analyzer_eval(n == INT_MIN); //expected-warning{{FALSE}}
+ //expected-warning@-1{{TRUE}}
+  clang_analyzer_eval(n <= U1); //expected-warning{{TRUE}}
+  clang_analyzer_eval(n > U1); //expected-warning{{FALSE}}
+} else if (n < INT_MAX / 2){
+  clang_analyzer_eval(n < L2); //expected-warning{{FALSE}}
+  clang_analyzer_eval(n >= L2); //expected-warning{{TRUE}}
+  clang_analyzer_eval(n <= U2); //expected-warning{{TRUE}}
+  clang_analyzer_eval(n > U2); //expected-warning{{FALSE}}
+} else {
+  clang_analyzer_eval(n < L3); //expected-warning{{FALSE}}
+  clang_analyzer_eval(n >= L3); //expected-warning{{TRUE}}
+  clang_analyzer_eval(n == INT_MAX); //expected-warning{{FALSE}}
+ //expected-warning@-1{{TRUE}}
+}
+  }
+}
+
+void signed_multiplication_lt_2(int32_t n) {
+  if (n * 2 < 5) {
+int32_t  U1 = 0x8002,
+L2 = 0xc000, U2 = 2,
+L3 = 0x4000;
+
+assert(INT_MIN * 2 < 5);
+assert(U1 * 2 < 5);
+assert((U1 + 1) * 2 >= 5);
+assert(L2 * 2 < 5);
+assert((L2 - 1) * 2 >= 5);
+assert(U2 * 2 < 5);
+assert((U2 + 1) * 2 >= 5);
+assert(L3 * 2 < 5);
+assert((L3 - 1) * 2 >= 5);
+assert(INT_MAX * 2 < 5);
+
+if (n < INT_MIN / 2) {
+  clang_analyzer_eval(n == INT_MIN); //expected-warning{{FALSE}}
+ //expected-warning@-1{{TRUE}}
+  clang_analyzer_eval(n <= U1); //expected-warning{{TRUE}}
+  clang_analyzer_eval(n > U1); //expected-warning{{FALSE}}
+} else if (n < INT_MAX / 2){
+  clang_analyzer_eval(n < L2); //expected-warning{{FALSE}}
+  clang_analyzer_eval(n >= L2); //expected-warning{{TRUE}}
+  clang_analyzer_eval(n <= U2); //expected-warning{{TRUE}}
+  clang_analyzer_eval(n > U2); //expected-warning{{FALSE}}
+} else {
+  clang_analyzer_eval(n < L3); //expected-warning{{FALSE}}
+  clang_analyzer_eval(n >= L3); //expected-warning{{TRUE}}
+  clang_analyzer_eval(n == INT_MAX); //expected-warning{{FALSE}}
+ //expected-warning@-1{{TRUE}}
+}
+  }
+}
+
+void signed_multiplication_lt_3(int32_t n) {
+  if (n * 3 < 4) {
+int32_t  U1 = 0xaaab,
+L2 = 0xd556, U2 = 1,
+L3 = 0x2aab, U3 = 0x5

[PATCH] D54556: [analyzer] MoveChecker Pt.1: Give MisusedMovedObject checker a more consistent name.

[Gábor Horváth via Phabricator via cfe-commits]

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

Looks good. Do we plan to detect problems other than use after move? Maybe it 
would be worth to synchronize with the tidy checker name use-after-move or is 
it going to cause more confusion?


Repository:
  rC Clang

https://reviews.llvm.org/D54556



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

[PATCH] D54557: [analyzer] MoveChecker Pt.2: Restrict the warning to STL objects and locals.

[Gábor Horváth via Phabricator via cfe-commits]

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

It would be great to have a way to extend the list of (possibly non-stl) types 
to check. But I do understand that the analyzer does not have a great way to 
set such configuration options right now.


Repository:
  rC Clang

https://reviews.llvm.org/D54557



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

[PATCH] D54269: Introduce shard storage to auto-index.

[Sam McCall via Phabricator via cfe-commits]

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

Thanks! Looks good, rest of the nits are obvious or up to you.(




Comment at: clangd/index/Background.cpp:37
 
-BackgroundIndex::BackgroundIndex(Context BackgroundContext,
- StringRef ResourceDir,
- const FileSystemProvider &FSProvider,
- ArrayRef URISchemes,
- size_t ThreadPoolSize)
+namespace {
+

nit: I think the moves of the helper functions 
(digest/digestFile/getAbsoluteFilePath) can be reverted now



Comment at: clangd/index/Background.h:45
+
+  using Factory =
+  llvm::unique_function;

Some docs? Maybe add a comment like
```
// The factory provides storage for each CDB. 
// It keeps ownership of the storage instances, and should manage caching 
itself.
// Factory must be threadsafe and never returns nullptr.
```



Comment at: clangd/index/Background.h:50
+  // CDBDirectory + ".clangd-index/" as the folder to save shards.
+  static Factory getDiskBackedStorageFactory();
+};

nit: `create` rather than `get` would be clearer that this returns an 
independent object each time.



Comment at: clangd/index/Background.h:103
 
+  // index storage
+  BackgroundIndexStorage::Factory IndexStorageFactory;

(nit: this comment seems redundant with the type/name)



Comment at: clangd/index/BackgroundIndexStorage.cpp:30
+  llvm::SmallString<128> ShardRootSS(ShardRoot);
+  llvm::sys::path::append(ShardRootSS, llvm::sys::path::filename(FilePath) +
+   llvm::toHex(digest(FilePath)) +

nit: can we put a separator like between the path and digest?
I'd suggest `.` so the file extension is most obviously preserved.

e.g. `Foo.cpp.12345.idx` vs `Foo.cpp_123456.idx` vs `Foo.cpp123456.idx'



Comment at: clangd/index/BackgroundIndexStorage.cpp:51
+if (EC != OK) {
+  elog("Failed to create directory {0} for disk backed index storage: {1}",
+   DiskShardRoot, EC.message());

nit: just "for index storage"?
it's clearly disk backed if it's going into a directory!
(and the extra qualification may confuse users)



Comment at: clangd/index/BackgroundIndexStorage.cpp:79
+OS << Shard;
+return llvm::Error::success();
+  }

You're ignoring write errors here (and in fact errors may not have occurred yet 
due to buffering).
```
OS.close();
return OS.error();
```



Comment at: unittests/clangd/BackgroundIndexTests.cpp:100
+TEST(BackgroundIndexTest, ShardStorageWriteTest) {
+  class MemoryShardStorage : public BackgroundIndexStorage {
+mutable std::mutex StorageMu;

Consider pulling this class to the top level, and use it any time you don't 
care about the storage (to avoid the need for DummyStorage)



Comment at: unittests/clangd/BackgroundIndexTests.cpp:111
+  std::lock_guard Lock(StorageMu);
+  std::string &str = Storage[ShardIdentifier];
+  llvm::raw_string_ostream OS(str);

I think this can just be 
`Storage[ShardIdentifier] = llvm::to_string(Shard)`

(need to include ScopedPrinters.h)



Comment at: unittests/clangd/BackgroundIndexTests.cpp:121
+  if (Storage.find(ShardIdentifier) == Storage.end()) {
+ADD_FAILURE() << "Shard " << ShardIdentifier << ": not found.";
+return nullptr;

this one shouldn't be a failure I think - just return nullptr? (If the test 
cares about it, the test should depend on it more explicitly).

The corrupt index file is a more obvious "should never happen" case.



Comment at: unittests/clangd/BackgroundIndexTests.cpp:130
+  }
+  CacheHits++;
+  return llvm::make_unique(std::move(*IndexFile));

You never actually assert on this value.

(I'm not sure you need to, but if not just remove it)



Comment at: unittests/clangd/BackgroundIndexTests.cpp:146
+  MemoryShardStorage MSS(Storage, CacheHits);
+  BackgroundIndexStorage::Factory MemoryStorageCreator = [&](llvm::StringRef) {
+return &MSS;

seems clearer just to inline this in the constructor(, but up to you



Comment at: unittests/clangd/BackgroundIndexTests.cpp:162
+  EXPECT_EQ(Storage.size(), 2U);
+  EXPECT_NE(Storage.find(testPath("root/A.h")), Storage.end());
+  EXPECT_NE(Storage.find(testPath("root/A.cc")), Storage.end());

(even if the StringMap was encapsulated in the MemoryStorage, you can still 
write this assertion through the public interface: 
ASSERT_NE(Storage.loadShard("root/A.h"), None)

(you could even assert the actual stored symbols, but may not be worth it)


Repository

[PATCH] D49074: [Analyzer] [WIP] Basic support for multiplication and division in the constraint manager

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

Here is the proof of the algorithm for `signed char` and `unsigned char` types. 
The dumper which tests every `n` for the `n*k  m` relation 
and creates ranges from values satisfying the expression for every `k` and `m` 
produces exactly the same output as the generator which generates the ranges 
using the algorithm from the patch. One the machine I run them they take 
between 30 and 60 seconds to run.

F7551311: proof.tar.bz2 


https://reviews.llvm.org/D49074



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

r346950 - Fix warning about unused variable [NFC]

[Mikael Holmen via cfe-commits]

Author: uabelho
Date: Thu Nov 15 05:01:54 2018
New Revision: 346950

URL: http://llvm.org/viewvc/llvm-project?rev=346950&view=rev
Log:
Fix warning about unused variable [NFC]

Modified:
cfe/trunk/lib/CodeGen/CGDecl.cpp

Modified: cfe/trunk/lib/CodeGen/CGDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDecl.cpp?rev=346950&r1=346949&r2=346950&view=diff
==
--- cfe/trunk/lib/CodeGen/CGDecl.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDecl.cpp Thu Nov 15 05:01:54 2018
@@ -976,9 +976,9 @@ static Address createUnnamedGlobalFrom(C
   return CGM.getMangledName(FD);
 } else if (const auto *OM = dyn_cast(DC)) {
   return OM->getNameAsString();
-} else if (const auto *OM = dyn_cast(DC)) {
+} else if (isa(DC)) {
   return "";
-} else if (const auto *OM = dyn_cast(DC)) {
+} else if (isa(DC)) {
   return "";
 } else {
   llvm::llvm_unreachable_internal("expected a function or method");


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

[PATCH] D54576: [clang] - Simplify tools::SplitDebugName.

[George Rimar via Phabricator via cfe-commits]

grimar added a comment.

`SplitDebugName` is called from the following 4 places:

1. void Clang::ConstructJob()

(https://github.com/llvm-mirror/clang/blob/master/lib/Driver/ToolChains/Clang.cpp#L3933)

We can get here when clang construct jobs for regular case (cpp files):
EX: main.cpp -g -gsplit-dwarf  -o ooo

Clang by itself does not recognize the '-fdebug-compilation-dir':
clang main.cpp -g -gsplit-dwarf  -o ooo -### -fdebug-compilation-dir
clang-8: error: unknown argument: '-fdebug-compilation-dir'

So I believe this option can not be used, be in Args 
and hence affect on anything during this flow.

In other places clang constructs assembly jobs:

2. void ClangAs::ConstructJob()

(https://github.com/llvm-mirror/clang/blob/master/lib/Driver/ToolChains/Clang.cpp#L5900)

Here it tries to use the OPT_fdebug_compilation_dir option from Args:
https://github.com/llvm-mirror/clang/blob/master/lib/Driver/ToolChains/CommonArgs.cpp#L824

And this is used to construct the argument for -split-dwarf-file.

The following also would not work:
clang main.s -g -gsplit-dwarf  -o ooo -fdebug-compilation-dir,xxx
clang-8: error: unknown argument: '-fdebug-compilation-dir,xxx'

So I do not see the way to add the '-fdebug-compilation-dir' to Args here too,
so `SplitDebugName` does not use the '-fdebug-compilation-dir' I think
and its logic is dead for this case too I believe)

3, 4) These are similar:

tools::gnutools::Assembler::ConstructJob()
(https://github.com/llvm-mirror/clang/blob/master/lib/Driver/ToolChains/Gnu.cpp#L820)

tools::MinGW::Assembler::ConstructJob()
https://github.com/llvm-mirror/clang/blob/master/lib/Driver/ToolChains/MinGW.cpp#L57

Both call `SplitDebugName` to construct calls for objcopy:
https://github.com/llvm-mirror/clang/blob/master/lib/Driver/ToolChains/CommonArgs.cpp#L833

But again, `Args` for their `ConstructJob` seems can never contain the 
`-fdebug-compilation-dir`
I just do not see any way to achieve that.

Clang's code adds the `-fdebug-compilation-dir` by itself in 
`Clang::ConstructJob()` and
`ClangAs::ConstructJob()`, but it will never be used in `SplitDebugName` I 
think:
https://github.com/llvm-mirror/clang/blob/master/lib/Driver/ToolChains/Clang.cpp#L601
https://github.com/llvm-mirror/clang/blob/master/lib/Driver/ToolChains/Clang.cpp#L5795
https://github.com/llvm-mirror/clang/blob/master/lib/Driver/ToolChains/Clang.cpp#L4172

That `-fdebug-compilation-dir` is used in another places, not in the 
`SplitDebugName`
(used in invocations for -cc1 and -cc1as:
https://github.com/llvm-mirror/clang/blob/master/tools/driver/cc1as_main.cpp#L234
https://github.com/llvm-mirror/clang/blob/master/lib/Frontend/CompilerInvocation.cpp#L944)

After changes done by this patch, no check-llvm/check-clang tests started to 
fail for me.


https://reviews.llvm.org/D54576



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

[PATCH] D54576: [clang] - Simplify tools::SplitDebugName.

[George Rimar via Phabricator via cfe-commits]

grimar created this revision.
grimar added a reviewer: dblaikie.
grimar added a comment.

`SplitDebugName` is called from the following 4 places:

1. void Clang::ConstructJob()

(https://github.com/llvm-mirror/clang/blob/master/lib/Driver/ToolChains/Clang.cpp#L3933)

We can get here when clang construct jobs for regular case (cpp files):
EX: main.cpp -g -gsplit-dwarf  -o ooo

Clang by itself does not recognize the '-fdebug-compilation-dir':
clang main.cpp -g -gsplit-dwarf  -o ooo -### -fdebug-compilation-dir
clang-8: error: unknown argument: '-fdebug-compilation-dir'

So I believe this option can not be used, be in Args 
and hence affect on anything during this flow.

In other places clang constructs assembly jobs:

2. void ClangAs::ConstructJob()

(https://github.com/llvm-mirror/clang/blob/master/lib/Driver/ToolChains/Clang.cpp#L5900)

Here it tries to use the OPT_fdebug_compilation_dir option from Args:
https://github.com/llvm-mirror/clang/blob/master/lib/Driver/ToolChains/CommonArgs.cpp#L824

And this is used to construct the argument for -split-dwarf-file.

The following also would not work:
clang main.s -g -gsplit-dwarf  -o ooo -fdebug-compilation-dir,xxx
clang-8: error: unknown argument: '-fdebug-compilation-dir,xxx'

So I do not see the way to add the '-fdebug-compilation-dir' to Args here too,
so `SplitDebugName` does not use the '-fdebug-compilation-dir' I think
and its logic is dead for this case too I believe)

3, 4) These are similar:

tools::gnutools::Assembler::ConstructJob()
(https://github.com/llvm-mirror/clang/blob/master/lib/Driver/ToolChains/Gnu.cpp#L820)

tools::MinGW::Assembler::ConstructJob()
https://github.com/llvm-mirror/clang/blob/master/lib/Driver/ToolChains/MinGW.cpp#L57

Both call `SplitDebugName` to construct calls for objcopy:
https://github.com/llvm-mirror/clang/blob/master/lib/Driver/ToolChains/CommonArgs.cpp#L833

But again, `Args` for their `ConstructJob` seems can never contain the 
`-fdebug-compilation-dir`
I just do not see any way to achieve that.

Clang's code adds the `-fdebug-compilation-dir` by itself in 
`Clang::ConstructJob()` and
`ClangAs::ConstructJob()`, but it will never be used in `SplitDebugName` I 
think:
https://github.com/llvm-mirror/clang/blob/master/lib/Driver/ToolChains/Clang.cpp#L601
https://github.com/llvm-mirror/clang/blob/master/lib/Driver/ToolChains/Clang.cpp#L5795
https://github.com/llvm-mirror/clang/blob/master/lib/Driver/ToolChains/Clang.cpp#L4172

That `-fdebug-compilation-dir` is used in another places, not in the 
`SplitDebugName`
(used in invocations for -cc1 and -cc1as:
https://github.com/llvm-mirror/clang/blob/master/tools/driver/cc1as_main.cpp#L234
https://github.com/llvm-mirror/clang/blob/master/lib/Frontend/CompilerInvocation.cpp#L944)

After changes done by this patch, no check-llvm/check-clang tests started to 
fail for me.


This should be NFC change.

`SplitDebugName` started to accept the `Output` that
can be used to simplify the logic a bit, also it
seems that code in `SplitDebugName` that uses
`OPT_fdebug_compilation_dir` is simply dead.

My understanding may be wrong because I am
not very familiar with clang, but I investigated this and
below in the comment, there are my
arguments for that.


https://reviews.llvm.org/D54576

Files:
  lib/Driver/ToolChains/Clang.cpp
  lib/Driver/ToolChains/CommonArgs.cpp
  lib/Driver/ToolChains/CommonArgs.h
  lib/Driver/ToolChains/Gnu.cpp
  lib/Driver/ToolChains/MinGW.cpp


Index: lib/Driver/ToolChains/MinGW.cpp
===
--- lib/Driver/ToolChains/MinGW.cpp
+++ lib/Driver/ToolChains/MinGW.cpp
@@ -54,7 +54,7 @@
 
   if (Args.hasArg(options::OPT_gsplit_dwarf))
 SplitDebugInfo(getToolChain(), C, *this, JA, Args, Output,
-   SplitDebugName(Args, Inputs[0], Output));
+   SplitDebugName(Args, Output));
 }
 
 void tools::MinGW::Linker::AddLibGCC(const ArgList &Args,
Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -817,7 +817,7 @@
   if (Args.hasArg(options::OPT_gsplit_dwarf) &&
   getToolChain().getTriple().isOSLinux())
 SplitDebugInfo(getToolChain(), C, *this, JA, Args, Output,
-   SplitDebugName(Args, Inputs[0], Output));
+   SplitDebugName(Args, Output));
 }
 
 namespace {
Index: lib/Driver/ToolChains/CommonArgs.h
===
--- lib/Driver/ToolChains/CommonArgs.h
+++ lib/Driver/ToolChains/CommonArgs.h
@@ -63,7 +63,7 @@
 const Tool &T);
 
 const char *SplitDebugName(const llvm::opt::ArgList &Args,
-   const InputInfo &Input, const InputInfo &Output);
+   const InputInfo &Output);
 
 void SplitDebugInfo(const ToolChain &TC, Compilation &C, const Tool &T,
 const JobAction &JA, const llvm::opt::ArgL

[PATCH] D53807: Create a diagnostic group for warn_call_to_pure_virtual_member_function_from_ctor_dtor, so it can be turned into an error using Werror

[Arnaud Bienner via Phabricator via cfe-commits]

ArnaudBienner added a comment.

@rsmith do you have any thoughts about this change?


Repository:
  rC Clang

https://reviews.llvm.org/D53807



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

r346951 - [AST] Pack UnaryOperator

[Bruno Ricci via cfe-commits]

Author: brunoricci
Date: Thu Nov 15 05:30:38 2018
New Revision: 346951

URL: http://llvm.org/viewvc/llvm-project?rev=346951&view=rev
Log:
[AST] Pack UnaryOperator

Use the newly available space in the bit-fields of Stmt
to store some data from UnaryOperator.
This saves 8 bytes per UnaryOperator.

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

Reviewed By: dblaikie


Modified:
cfe/trunk/include/clang/AST/Expr.h
cfe/trunk/include/clang/AST/Stmt.h

Modified: cfe/trunk/include/clang/AST/Expr.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=346951&r1=346950&r2=346951&view=diff
==
--- cfe/trunk/include/clang/AST/Expr.h (original)
+++ cfe/trunk/include/clang/AST/Expr.h Thu Nov 15 05:30:38 2018
@@ -1869,15 +1869,11 @@ public:
 ///   later returns zero in the type of the operand.
 ///
 class UnaryOperator : public Expr {
+  Stmt *Val;
+
 public:
   typedef UnaryOperatorKind Opcode;
 
-private:
-  unsigned Opc : 5;
-  unsigned CanOverflow : 1;
-  SourceLocation Loc;
-  Stmt *Val;
-public:
   UnaryOperator(Expr *input, Opcode opc, QualType type, ExprValueKind VK,
 ExprObjectKind OK, SourceLocation l, bool CanOverflow)
   : Expr(UnaryOperatorClass, type, VK, OK,
@@ -1886,21 +1882,28 @@ public:
  (input->isInstantiationDependent() ||
   type->isInstantiationDependentType()),
  input->containsUnexpandedParameterPack()),
-Opc(opc), CanOverflow(CanOverflow), Loc(l), Val(input) {}
+Val(input) {
+UnaryOperatorBits.Opc = opc;
+UnaryOperatorBits.CanOverflow = CanOverflow;
+UnaryOperatorBits.Loc = l;
+  }
 
   /// Build an empty unary operator.
-  explicit UnaryOperator(EmptyShell Empty)
-: Expr(UnaryOperatorClass, Empty), Opc(UO_AddrOf) { }
+  explicit UnaryOperator(EmptyShell Empty) : Expr(UnaryOperatorClass, Empty) {
+UnaryOperatorBits.Opc = UO_AddrOf;
+  }
 
-  Opcode getOpcode() const { return static_cast(Opc); }
-  void setOpcode(Opcode O) { Opc = O; }
+  Opcode getOpcode() const {
+return static_cast(UnaryOperatorBits.Opc);
+  }
+  void setOpcode(Opcode Opc) { UnaryOperatorBits.Opc = Opc; }
 
   Expr *getSubExpr() const { return cast(Val); }
   void setSubExpr(Expr *E) { Val = E; }
 
   /// getOperatorLoc - Return the location of the operator.
-  SourceLocation getOperatorLoc() const { return Loc; }
-  void setOperatorLoc(SourceLocation L) { Loc = L; }
+  SourceLocation getOperatorLoc() const { return UnaryOperatorBits.Loc; }
+  void setOperatorLoc(SourceLocation L) { UnaryOperatorBits.Loc = L; }
 
   /// Returns true if the unary operator can cause an overflow. For instance,
   ///   signed int i = INT_MAX; i++;
@@ -1908,8 +1911,8 @@ public:
   /// Due to integer promotions, c++ is promoted to an int before the postfix
   /// increment, and the result is an int that cannot overflow. However, i++
   /// can overflow.
-  bool canOverflow() const { return CanOverflow; }
-  void setCanOverflow(bool C) { CanOverflow = C; }
+  bool canOverflow() const { return UnaryOperatorBits.CanOverflow; }
+  void setCanOverflow(bool C) { UnaryOperatorBits.CanOverflow = C; }
 
   /// isPostfix - Return true if this is a postfix operation, like x++.
   static bool isPostfix(Opcode Op) {
@@ -1961,12 +1964,12 @@ public:
   static OverloadedOperatorKind getOverloadedOperator(Opcode Opc);
 
   SourceLocation getBeginLoc() const LLVM_READONLY {
-return isPostfix() ? Val->getBeginLoc() : Loc;
+return isPostfix() ? Val->getBeginLoc() : getOperatorLoc();
   }
   SourceLocation getEndLoc() const LLVM_READONLY {
-return isPostfix() ? Loc : Val->getEndLoc();
+return isPostfix() ? getOperatorLoc() : Val->getEndLoc();
   }
-  SourceLocation getExprLoc() const LLVM_READONLY { return Loc; }
+  SourceLocation getExprLoc() const { return getOperatorLoc(); }
 
   static bool classof(const Stmt *T) {
 return T->getStmtClass() == UnaryOperatorClass;

Modified: cfe/trunk/include/clang/AST/Stmt.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Stmt.h?rev=346951&r1=346950&r2=346951&view=diff
==
--- cfe/trunk/include/clang/AST/Stmt.h (original)
+++ cfe/trunk/include/clang/AST/Stmt.h Thu Nov 15 05:30:38 2018
@@ -374,6 +374,17 @@ protected:
 unsigned Kind : 3;
   };
 
+  class UnaryOperatorBitfields {
+friend class UnaryOperator;
+
+unsigned : NumExprBits;
+
+unsigned Opc : 5;
+unsigned CanOverflow : 1;
+
+SourceLocation Loc;
+  };
+
   class UnaryExprOrTypeTraitExprBitfields {
 friend class UnaryExprOrTypeTraitExpr;
 
@@ -513,6 +524,7 @@ protected:
 DeclRefExprBitfields DeclRefExprBits;
 FloatingLiteralBitfields FloatingLiteralBits;
 CharacterLiteralBitfields CharacterLiteralBits;
+UnaryOperatorBitfields UnaryOperatorBits;
 UnaryExprOrTypeTraitExprBitfields UnaryExprOrTypeTraitExprB

[PATCH] D54524: [AST] Pack UnaryOperator

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL346951: [AST] Pack UnaryOperator (authored by brunoricci, 
committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D54524?vs=174024&id=174195#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D54524

Files:
  cfe/trunk/include/clang/AST/Expr.h
  cfe/trunk/include/clang/AST/Stmt.h

Index: cfe/trunk/include/clang/AST/Stmt.h
===
--- cfe/trunk/include/clang/AST/Stmt.h
+++ cfe/trunk/include/clang/AST/Stmt.h
@@ -374,6 +374,17 @@
 unsigned Kind : 3;
   };
 
+  class UnaryOperatorBitfields {
+friend class UnaryOperator;
+
+unsigned : NumExprBits;
+
+unsigned Opc : 5;
+unsigned CanOverflow : 1;
+
+SourceLocation Loc;
+  };
+
   class UnaryExprOrTypeTraitExprBitfields {
 friend class UnaryExprOrTypeTraitExpr;
 
@@ -513,6 +524,7 @@
 DeclRefExprBitfields DeclRefExprBits;
 FloatingLiteralBitfields FloatingLiteralBits;
 CharacterLiteralBitfields CharacterLiteralBits;
+UnaryOperatorBitfields UnaryOperatorBits;
 UnaryExprOrTypeTraitExprBitfields UnaryExprOrTypeTraitExprBits;
 CallExprBitfields CallExprBits;
 CastExprBitfields CastExprBits;
Index: cfe/trunk/include/clang/AST/Expr.h
===
--- cfe/trunk/include/clang/AST/Expr.h
+++ cfe/trunk/include/clang/AST/Expr.h
@@ -1869,47 +1869,50 @@
 ///   later returns zero in the type of the operand.
 ///
 class UnaryOperator : public Expr {
+  Stmt *Val;
+
 public:
   typedef UnaryOperatorKind Opcode;
 
-private:
-  unsigned Opc : 5;
-  unsigned CanOverflow : 1;
-  SourceLocation Loc;
-  Stmt *Val;
-public:
   UnaryOperator(Expr *input, Opcode opc, QualType type, ExprValueKind VK,
 ExprObjectKind OK, SourceLocation l, bool CanOverflow)
   : Expr(UnaryOperatorClass, type, VK, OK,
  input->isTypeDependent() || type->isDependentType(),
  input->isValueDependent(),
  (input->isInstantiationDependent() ||
   type->isInstantiationDependentType()),
  input->containsUnexpandedParameterPack()),
-Opc(opc), CanOverflow(CanOverflow), Loc(l), Val(input) {}
+Val(input) {
+UnaryOperatorBits.Opc = opc;
+UnaryOperatorBits.CanOverflow = CanOverflow;
+UnaryOperatorBits.Loc = l;
+  }
 
   /// Build an empty unary operator.
-  explicit UnaryOperator(EmptyShell Empty)
-: Expr(UnaryOperatorClass, Empty), Opc(UO_AddrOf) { }
+  explicit UnaryOperator(EmptyShell Empty) : Expr(UnaryOperatorClass, Empty) {
+UnaryOperatorBits.Opc = UO_AddrOf;
+  }
 
-  Opcode getOpcode() const { return static_cast(Opc); }
-  void setOpcode(Opcode O) { Opc = O; }
+  Opcode getOpcode() const {
+return static_cast(UnaryOperatorBits.Opc);
+  }
+  void setOpcode(Opcode Opc) { UnaryOperatorBits.Opc = Opc; }
 
   Expr *getSubExpr() const { return cast(Val); }
   void setSubExpr(Expr *E) { Val = E; }
 
   /// getOperatorLoc - Return the location of the operator.
-  SourceLocation getOperatorLoc() const { return Loc; }
-  void setOperatorLoc(SourceLocation L) { Loc = L; }
+  SourceLocation getOperatorLoc() const { return UnaryOperatorBits.Loc; }
+  void setOperatorLoc(SourceLocation L) { UnaryOperatorBits.Loc = L; }
 
   /// Returns true if the unary operator can cause an overflow. For instance,
   ///   signed int i = INT_MAX; i++;
   ///   signed char c = CHAR_MAX; c++;
   /// Due to integer promotions, c++ is promoted to an int before the postfix
   /// increment, and the result is an int that cannot overflow. However, i++
   /// can overflow.
-  bool canOverflow() const { return CanOverflow; }
-  void setCanOverflow(bool C) { CanOverflow = C; }
+  bool canOverflow() const { return UnaryOperatorBits.CanOverflow; }
+  void setCanOverflow(bool C) { UnaryOperatorBits.CanOverflow = C; }
 
   /// isPostfix - Return true if this is a postfix operation, like x++.
   static bool isPostfix(Opcode Op) {
@@ -1961,12 +1964,12 @@
   static OverloadedOperatorKind getOverloadedOperator(Opcode Opc);
 
   SourceLocation getBeginLoc() const LLVM_READONLY {
-return isPostfix() ? Val->getBeginLoc() : Loc;
+return isPostfix() ? Val->getBeginLoc() : getOperatorLoc();
   }
   SourceLocation getEndLoc() const LLVM_READONLY {
-return isPostfix() ? Loc : Val->getEndLoc();
+return isPostfix() ? getOperatorLoc() : Val->getEndLoc();
   }
-  SourceLocation getExprLoc() const LLVM_READONLY { return Loc; }
+  SourceLocation getExprLoc() const { return getOperatorLoc(); }
 
   static bool classof(const Stmt *T) {
 return T->getStmtClass() == UnaryOperatorClass;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54526: [AST] Pack BinaryOperator

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno marked 6 inline comments as done.
riccibruno added a comment.

Marked the inline comments as done since I believe I
answered each of them. If not I can fix it in a subsequent commit.


Repository:
  rC Clang

https://reviews.llvm.org/D54526



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

r346952 - [AST][NFC] Move the friend decls to the top of MemberExpr

[Bruno Ricci via cfe-commits]

Author: brunoricci
Date: Thu Nov 15 05:49:32 2018
New Revision: 346952

URL: http://llvm.org/viewvc/llvm-project?rev=346952&view=rev
Log:
[AST][NFC] Move the friend decls to the top of MemberExpr

The norm is to have them at the top, and having them
at the bottom is painful for the reader.


Modified:
cfe/trunk/include/clang/AST/Expr.h

Modified: cfe/trunk/include/clang/AST/Expr.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=346952&r1=346951&r2=346952&view=diff
==
--- cfe/trunk/include/clang/AST/Expr.h (original)
+++ cfe/trunk/include/clang/AST/Expr.h Thu Nov 15 05:49:32 2018
@@ -2559,6 +2559,10 @@ class MemberExpr final
   private llvm::TrailingObjects {
+  friend class ASTReader;
+  friend class ASTStmtWriter;
+  friend TrailingObjects;
+
   /// Base - the expression for the base pointer or structure references.  In
   /// X.F, this is "X".
   Stmt *Base;
@@ -2796,10 +2800,6 @@ public:
   const_child_range children() const {
 return const_child_range(&Base, &Base + 1);
   }
-
-  friend TrailingObjects;
-  friend class ASTReader;
-  friend class ASTStmtWriter;
 };
 
 /// CompoundLiteralExpr - [C99 6.5.2.5]


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

r346953 - [AST] Pack MemberExpr

[Bruno Ricci via cfe-commits]

Author: brunoricci
Date: Thu Nov 15 05:56:22 2018
New Revision: 346953

URL: http://llvm.org/viewvc/llvm-project?rev=346953&view=rev
Log:
[AST] Pack MemberExpr

Use the newly available space in the bit-fields of Stmt
to store some data from MemberExpr. This saves
one pointer per MemberExpr.

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

Reviewed By: dblaikie


Modified:
cfe/trunk/include/clang/AST/Expr.h
cfe/trunk/include/clang/AST/Stmt.h
cfe/trunk/lib/AST/Expr.cpp
cfe/trunk/lib/Serialization/ASTWriterStmt.cpp

Modified: cfe/trunk/include/clang/AST/Expr.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=346953&r1=346952&r2=346953&view=diff
==
--- cfe/trunk/include/clang/AST/Expr.h (original)
+++ cfe/trunk/include/clang/AST/Expr.h Thu Nov 15 05:56:22 2018
@@ -2578,35 +2578,20 @@ class MemberExpr final
   /// MemberLoc - This is the location of the member name.
   SourceLocation MemberLoc;
 
-  /// This is the location of the -> or . in the expression.
-  SourceLocation OperatorLoc;
-
-  /// IsArrow - True if this is "X->F", false if this is "X.F".
-  bool IsArrow : 1;
-
-  /// True if this member expression used a nested-name-specifier to
-  /// refer to the member, e.g., "x->Base::f", or found its member via a using
-  /// declaration.  When true, a MemberExprNameQualifier
-  /// structure is allocated immediately after the MemberExpr.
-  bool HasQualifierOrFoundDecl : 1;
-
-  /// True if this member expression specified a template keyword
-  /// and/or a template argument list explicitly, e.g., x->f,
-  /// x->template f, x->template f.
-  /// When true, an ASTTemplateKWAndArgsInfo structure and its
-  /// TemplateArguments (if any) are present.
-  bool HasTemplateKWAndArgsInfo : 1;
-
-  /// True if this member expression refers to a method that
-  /// was resolved from an overloaded set having size greater than 1.
-  bool HadMultipleCandidates : 1;
-
   size_t numTrailingObjects(OverloadToken) const {
-return HasQualifierOrFoundDecl ? 1 : 0;
+return hasQualifierOrFoundDecl();
   }
 
   size_t numTrailingObjects(OverloadToken) const {
-return HasTemplateKWAndArgsInfo ? 1 : 0;
+return hasTemplateKWAndArgsInfo();
+  }
+
+  bool hasQualifierOrFoundDecl() const {
+return MemberExprBits.HasQualifierOrFoundDecl;
+  }
+
+  bool hasTemplateKWAndArgsInfo() const {
+return MemberExprBits.HasTemplateKWAndArgsInfo;
   }
 
 public:
@@ -2617,10 +2602,13 @@ public:
  base->isValueDependent(), base->isInstantiationDependent(),
  base->containsUnexpandedParameterPack()),
 Base(base), MemberDecl(memberdecl), MemberDNLoc(NameInfo.getInfo()),
-MemberLoc(NameInfo.getLoc()), OperatorLoc(operatorloc),
-IsArrow(isarrow), HasQualifierOrFoundDecl(false),
-HasTemplateKWAndArgsInfo(false), HadMultipleCandidates(false) {
+MemberLoc(NameInfo.getLoc()) {
 assert(memberdecl->getDeclName() == NameInfo.getName());
+MemberExprBits.IsArrow = isarrow;
+MemberExprBits.HasQualifierOrFoundDecl = false;
+MemberExprBits.HasTemplateKWAndArgsInfo = false;
+MemberExprBits.HadMultipleCandidates = false;
+MemberExprBits.OperatorLoc = operatorloc;
   }
 
   // NOTE: this constructor should be used only when it is known that
@@ -2633,10 +2621,13 @@ public:
   : Expr(MemberExprClass, ty, VK, OK, base->isTypeDependent(),
  base->isValueDependent(), base->isInstantiationDependent(),
  base->containsUnexpandedParameterPack()),
-Base(base), MemberDecl(memberdecl), MemberDNLoc(), MemberLoc(l),
-OperatorLoc(operatorloc), IsArrow(isarrow),
-HasQualifierOrFoundDecl(false), HasTemplateKWAndArgsInfo(false),
-HadMultipleCandidates(false) {}
+Base(base), MemberDecl(memberdecl), MemberDNLoc(), MemberLoc(l) {
+MemberExprBits.IsArrow = isarrow;
+MemberExprBits.HasQualifierOrFoundDecl = false;
+MemberExprBits.HasTemplateKWAndArgsInfo = false;
+MemberExprBits.HadMultipleCandidates = false;
+MemberExprBits.OperatorLoc = operatorloc;
+  }
 
   static MemberExpr *Create(const ASTContext &C, Expr *base, bool isarrow,
 SourceLocation OperatorLoc,
@@ -2659,7 +2650,7 @@ public:
 
   /// Retrieves the declaration found by lookup.
   DeclAccessPair getFoundDecl() const {
-if (!HasQualifierOrFoundDecl)
+if (!hasQualifierOrFoundDecl())
   return DeclAccessPair::make(getMemberDecl(),
   getMemberDecl()->getAccess());
 return getTrailingObjects()->FoundDecl;
@@ -2674,9 +2665,8 @@ public:
   /// nested-name-specifier that precedes the member name, with source-location
   /// information.
   NestedNameSpecifierLoc getQualifierLoc() const {
-if (!HasQualifierOrFoundDecl)
+if (!hasQualifierOrFoundDecl())
   return NestedNameSpecifierLoc();
-
 return getTrailingObjects()

[PATCH] D54525: [AST] Pack MemberExpr

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC346953: [AST] Pack MemberExpr (authored by brunoricci, 
committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D54525?vs=174026&id=174196#toc

Repository:
  rC Clang

https://reviews.llvm.org/D54525

Files:
  include/clang/AST/Expr.h
  include/clang/AST/Stmt.h
  lib/AST/Expr.cpp
  lib/Serialization/ASTWriterStmt.cpp

Index: lib/AST/Expr.cpp
===
--- lib/AST/Expr.cpp
+++ lib/AST/Expr.cpp
@@ -1528,15 +1528,16 @@
  QualifierLoc.getNestedNameSpecifier()->isInstantiationDependent())
   E->setInstantiationDependent(true);
 
-E->HasQualifierOrFoundDecl = true;
+E->MemberExprBits.HasQualifierOrFoundDecl = true;
 
 MemberExprNameQualifier *NQ =
 E->getTrailingObjects();
 NQ->QualifierLoc = QualifierLoc;
 NQ->FoundDecl = founddecl;
   }
 
-  E->HasTemplateKWAndArgsInfo = (targs || TemplateKWLoc.isValid());
+  E->MemberExprBits.HasTemplateKWAndArgsInfo =
+  (targs || TemplateKWLoc.isValid());
 
   if (targs) {
 bool Dependent = false;
Index: lib/Serialization/ASTWriterStmt.cpp
===
--- lib/Serialization/ASTWriterStmt.cpp
+++ lib/Serialization/ASTWriterStmt.cpp
@@ -655,8 +655,8 @@
   if (E->hasQualifier())
 Record.AddNestedNameSpecifierLoc(E->getQualifierLoc());
 
-  Record.push_back(E->HasTemplateKWAndArgsInfo);
-  if (E->HasTemplateKWAndArgsInfo) {
+  Record.push_back(E->hasTemplateKWAndArgsInfo());
+  if (E->hasTemplateKWAndArgsInfo()) {
 Record.AddSourceLocation(E->getTemplateKeywordLoc());
 unsigned NumTemplateArgs = E->getNumTemplateArgs();
 Record.push_back(NumTemplateArgs);
Index: include/clang/AST/Expr.h
===
--- include/clang/AST/Expr.h
+++ include/clang/AST/Expr.h
@@ -2578,35 +2578,20 @@
   /// MemberLoc - This is the location of the member name.
   SourceLocation MemberLoc;
 
-  /// This is the location of the -> or . in the expression.
-  SourceLocation OperatorLoc;
-
-  /// IsArrow - True if this is "X->F", false if this is "X.F".
-  bool IsArrow : 1;
-
-  /// True if this member expression used a nested-name-specifier to
-  /// refer to the member, e.g., "x->Base::f", or found its member via a using
-  /// declaration.  When true, a MemberExprNameQualifier
-  /// structure is allocated immediately after the MemberExpr.
-  bool HasQualifierOrFoundDecl : 1;
-
-  /// True if this member expression specified a template keyword
-  /// and/or a template argument list explicitly, e.g., x->f,
-  /// x->template f, x->template f.
-  /// When true, an ASTTemplateKWAndArgsInfo structure and its
-  /// TemplateArguments (if any) are present.
-  bool HasTemplateKWAndArgsInfo : 1;
-
-  /// True if this member expression refers to a method that
-  /// was resolved from an overloaded set having size greater than 1.
-  bool HadMultipleCandidates : 1;
-
   size_t numTrailingObjects(OverloadToken) const {
-return HasQualifierOrFoundDecl ? 1 : 0;
+return hasQualifierOrFoundDecl();
   }
 
   size_t numTrailingObjects(OverloadToken) const {
-return HasTemplateKWAndArgsInfo ? 1 : 0;
+return hasTemplateKWAndArgsInfo();
+  }
+
+  bool hasQualifierOrFoundDecl() const {
+return MemberExprBits.HasQualifierOrFoundDecl;
+  }
+
+  bool hasTemplateKWAndArgsInfo() const {
+return MemberExprBits.HasTemplateKWAndArgsInfo;
   }
 
 public:
@@ -2617,10 +2602,13 @@
  base->isValueDependent(), base->isInstantiationDependent(),
  base->containsUnexpandedParameterPack()),
 Base(base), MemberDecl(memberdecl), MemberDNLoc(NameInfo.getInfo()),
-MemberLoc(NameInfo.getLoc()), OperatorLoc(operatorloc),
-IsArrow(isarrow), HasQualifierOrFoundDecl(false),
-HasTemplateKWAndArgsInfo(false), HadMultipleCandidates(false) {
+MemberLoc(NameInfo.getLoc()) {
 assert(memberdecl->getDeclName() == NameInfo.getName());
+MemberExprBits.IsArrow = isarrow;
+MemberExprBits.HasQualifierOrFoundDecl = false;
+MemberExprBits.HasTemplateKWAndArgsInfo = false;
+MemberExprBits.HadMultipleCandidates = false;
+MemberExprBits.OperatorLoc = operatorloc;
   }
 
   // NOTE: this constructor should be used only when it is known that
@@ -2633,10 +2621,13 @@
   : Expr(MemberExprClass, ty, VK, OK, base->isTypeDependent(),
  base->isValueDependent(), base->isInstantiationDependent(),
  base->containsUnexpandedParameterPack()),
-Base(base), MemberDecl(memberdecl), MemberDNLoc(), MemberLoc(l),
-OperatorLoc(operatorloc), IsArrow(isarrow),
-HasQualifierOrFoundDecl(false), HasTemplateKWAndArgsInfo(false),
-HadMultipleCandidates(false) {}
+Base(base), MemberDecl(memberdecl), MemberDNLoc(), MemberLoc(l) {
+MemberExprBits.IsArrow = isarrow;
+MemberExprB

r346954 - [AST] Pack BinaryOperator

[Bruno Ricci via cfe-commits]

Author: brunoricci
Date: Thu Nov 15 06:12:51 2018
New Revision: 346954

URL: http://llvm.org/viewvc/llvm-project?rev=346954&view=rev
Log:
[AST] Pack BinaryOperator

Use the newly available space in the bit-fields of Stmt.
This saves 8 bytes per BinaryOperator.

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

Reviewed By: dblaikie


Modified:
cfe/trunk/include/clang/AST/Expr.h
cfe/trunk/include/clang/AST/Stmt.h

Modified: cfe/trunk/include/clang/AST/Expr.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=346954&r1=346953&r2=346954&view=diff
==
--- cfe/trunk/include/clang/AST/Expr.h (original)
+++ cfe/trunk/include/clang/AST/Expr.h Thu Nov 15 06:12:51 2018
@@ -3187,20 +3187,11 @@ public:
 /// "+" resolves to an overloaded operator, CXXOperatorCallExpr will
 /// be used to express the computation.
 class BinaryOperator : public Expr {
-public:
-  typedef BinaryOperatorKind Opcode;
-
-private:
-  unsigned Opc : 6;
-
-  // This is only meaningful for operations on floating point types and 0
-  // otherwise.
-  unsigned FPFeatures : 3;
-  SourceLocation OpLoc;
-
   enum { LHS, RHS, END_EXPR };
-  Stmt* SubExprs[END_EXPR];
+  Stmt *SubExprs[END_EXPR];
+
 public:
+  typedef BinaryOperatorKind Opcode;
 
   BinaryOperator(Expr *lhs, Expr *rhs, Opcode opc, QualType ResTy,
  ExprValueKind VK, ExprObjectKind OK,
@@ -3211,8 +3202,10 @@ public:
(lhs->isInstantiationDependent() ||
 rhs->isInstantiationDependent()),
(lhs->containsUnexpandedParameterPack() ||
-rhs->containsUnexpandedParameterPack())),
-  Opc(opc), FPFeatures(FPFeatures.getInt()), OpLoc(opLoc) {
+rhs->containsUnexpandedParameterPack())) {
+BinaryOperatorBits.Opc = opc;
+BinaryOperatorBits.FPFeatures = FPFeatures.getInt();
+BinaryOperatorBits.OpLoc = opLoc;
 SubExprs[LHS] = lhs;
 SubExprs[RHS] = rhs;
 assert(!isCompoundAssignmentOp() &&
@@ -3220,15 +3213,18 @@ public:
   }
 
   /// Construct an empty binary operator.
-  explicit BinaryOperator(EmptyShell Empty)
-: Expr(BinaryOperatorClass, Empty), Opc(BO_Comma) { }
+  explicit BinaryOperator(EmptyShell Empty) : Expr(BinaryOperatorClass, Empty) 
{
+BinaryOperatorBits.Opc = BO_Comma;
+  }
 
-  SourceLocation getExprLoc() const LLVM_READONLY { return OpLoc; }
-  SourceLocation getOperatorLoc() const { return OpLoc; }
-  void setOperatorLoc(SourceLocation L) { OpLoc = L; }
+  SourceLocation getExprLoc() const { return getOperatorLoc(); }
+  SourceLocation getOperatorLoc() const { return BinaryOperatorBits.OpLoc; }
+  void setOperatorLoc(SourceLocation L) { BinaryOperatorBits.OpLoc = L; }
 
-  Opcode getOpcode() const { return static_cast(Opc); }
-  void setOpcode(Opcode O) { Opc = O; }
+  Opcode getOpcode() const {
+return static_cast(BinaryOperatorBits.Opc);
+  }
+  void setOpcode(Opcode Opc) { BinaryOperatorBits.Opc = Opc; }
 
   Expr *getLHS() const { return cast(SubExprs[LHS]); }
   void setLHS(Expr *E) { SubExprs[LHS] = E; }
@@ -3257,7 +3253,11 @@ public:
   static OverloadedOperatorKind getOverloadedOperator(Opcode Opc);
 
   /// predicates to categorize the respective opcodes.
-  bool isPtrMemOp() const { return Opc == BO_PtrMemD || Opc == BO_PtrMemI; }
+  static bool isPtrMemOp(Opcode Opc) {
+return Opc == BO_PtrMemD || Opc == BO_PtrMemI;
+  }
+  bool isPtrMemOp() const { return isPtrMemOp(getOpcode()); }
+
   static bool isMultiplicativeOp(Opcode Opc) {
 return Opc >= BO_Mul && Opc <= BO_Rem;
   }
@@ -3356,21 +3356,23 @@ public:
 
   // Set the FP contractability status of this operator. Only meaningful for
   // operations on floating point types.
-  void setFPFeatures(FPOptions F) { FPFeatures = F.getInt(); }
+  void setFPFeatures(FPOptions F) {
+BinaryOperatorBits.FPFeatures = F.getInt();
+  }
 
-  FPOptions getFPFeatures() const { return FPOptions(FPFeatures); }
+  FPOptions getFPFeatures() const {
+return FPOptions(BinaryOperatorBits.FPFeatures);
+  }
 
   // Get the FP contractability status of this operator. Only meaningful for
   // operations on floating point types.
   bool isFPContractableWithinStatement() const {
-return FPOptions(FPFeatures).allowFPContractWithinStatement();
+return getFPFeatures().allowFPContractWithinStatement();
   }
 
   // Get the FENV_ACCESS status of this operator. Only meaningful for
   // operations on floating point types.
-  bool isFEnvAccessOn() const {
-return FPOptions(FPFeatures).allowFEnvAccess();
-  }
+  bool isFEnvAccessOn() const { return getFPFeatures().allowFEnvAccess(); }
 
 protected:
   BinaryOperator(Expr *lhs, Expr *rhs, Opcode opc, QualType ResTy,
@@ -3382,14 +3384,17 @@ protected:
(lhs->isInstantiationDependent() ||
 rhs->isInstantiationDependent()),
(lhs->containsUnexpandedParameterPack() ||
-rhs->containsUnexpandedParameterPack())),
- 

[PATCH] D54526: [AST] Pack BinaryOperator

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC346954: [AST] Pack BinaryOperator (authored by brunoricci, 
committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D54526

Files:
  include/clang/AST/Expr.h
  include/clang/AST/Stmt.h

Index: include/clang/AST/Stmt.h
===
--- include/clang/AST/Stmt.h
+++ include/clang/AST/Stmt.h
@@ -442,6 +442,17 @@
 unsigned BasePathIsEmpty : 1;
   };
 
+  class BinaryOperatorBitfields {
+friend class BinaryOperator;
+
+unsigned : NumExprBits;
+
+unsigned Opc : 6;
+unsigned FPFeatures : 3;
+
+SourceLocation OpLoc;
+  };
+
   class InitListExprBitfields {
 friend class InitListExpr;
 
@@ -558,6 +569,7 @@
 CallExprBitfields CallExprBits;
 MemberExprBitfields MemberExprBits;
 CastExprBitfields CastExprBits;
+BinaryOperatorBitfields BinaryOperatorBits;
 InitListExprBitfields InitListExprBits;
 PseudoObjectExprBitfields PseudoObjectExprBits;
 
Index: include/clang/AST/Expr.h
===
--- include/clang/AST/Expr.h
+++ include/clang/AST/Expr.h
@@ -3187,20 +3187,11 @@
 /// "+" resolves to an overloaded operator, CXXOperatorCallExpr will
 /// be used to express the computation.
 class BinaryOperator : public Expr {
-public:
-  typedef BinaryOperatorKind Opcode;
-
-private:
-  unsigned Opc : 6;
-
-  // This is only meaningful for operations on floating point types and 0
-  // otherwise.
-  unsigned FPFeatures : 3;
-  SourceLocation OpLoc;
-
   enum { LHS, RHS, END_EXPR };
-  Stmt* SubExprs[END_EXPR];
+  Stmt *SubExprs[END_EXPR];
+
 public:
+  typedef BinaryOperatorKind Opcode;
 
   BinaryOperator(Expr *lhs, Expr *rhs, Opcode opc, QualType ResTy,
  ExprValueKind VK, ExprObjectKind OK,
@@ -3211,24 +3202,29 @@
(lhs->isInstantiationDependent() ||
 rhs->isInstantiationDependent()),
(lhs->containsUnexpandedParameterPack() ||
-rhs->containsUnexpandedParameterPack())),
-  Opc(opc), FPFeatures(FPFeatures.getInt()), OpLoc(opLoc) {
+rhs->containsUnexpandedParameterPack())) {
+BinaryOperatorBits.Opc = opc;
+BinaryOperatorBits.FPFeatures = FPFeatures.getInt();
+BinaryOperatorBits.OpLoc = opLoc;
 SubExprs[LHS] = lhs;
 SubExprs[RHS] = rhs;
 assert(!isCompoundAssignmentOp() &&
"Use CompoundAssignOperator for compound assignments");
   }
 
   /// Construct an empty binary operator.
-  explicit BinaryOperator(EmptyShell Empty)
-: Expr(BinaryOperatorClass, Empty), Opc(BO_Comma) { }
+  explicit BinaryOperator(EmptyShell Empty) : Expr(BinaryOperatorClass, Empty) {
+BinaryOperatorBits.Opc = BO_Comma;
+  }
 
-  SourceLocation getExprLoc() const LLVM_READONLY { return OpLoc; }
-  SourceLocation getOperatorLoc() const { return OpLoc; }
-  void setOperatorLoc(SourceLocation L) { OpLoc = L; }
+  SourceLocation getExprLoc() const { return getOperatorLoc(); }
+  SourceLocation getOperatorLoc() const { return BinaryOperatorBits.OpLoc; }
+  void setOperatorLoc(SourceLocation L) { BinaryOperatorBits.OpLoc = L; }
 
-  Opcode getOpcode() const { return static_cast(Opc); }
-  void setOpcode(Opcode O) { Opc = O; }
+  Opcode getOpcode() const {
+return static_cast(BinaryOperatorBits.Opc);
+  }
+  void setOpcode(Opcode Opc) { BinaryOperatorBits.Opc = Opc; }
 
   Expr *getLHS() const { return cast(SubExprs[LHS]); }
   void setLHS(Expr *E) { SubExprs[LHS] = E; }
@@ -3257,7 +3253,11 @@
   static OverloadedOperatorKind getOverloadedOperator(Opcode Opc);
 
   /// predicates to categorize the respective opcodes.
-  bool isPtrMemOp() const { return Opc == BO_PtrMemD || Opc == BO_PtrMemI; }
+  static bool isPtrMemOp(Opcode Opc) {
+return Opc == BO_PtrMemD || Opc == BO_PtrMemI;
+  }
+  bool isPtrMemOp() const { return isPtrMemOp(getOpcode()); }
+
   static bool isMultiplicativeOp(Opcode Opc) {
 return Opc >= BO_Mul && Opc <= BO_Rem;
   }
@@ -3356,21 +3356,23 @@
 
   // Set the FP contractability status of this operator. Only meaningful for
   // operations on floating point types.
-  void setFPFeatures(FPOptions F) { FPFeatures = F.getInt(); }
+  void setFPFeatures(FPOptions F) {
+BinaryOperatorBits.FPFeatures = F.getInt();
+  }
 
-  FPOptions getFPFeatures() const { return FPOptions(FPFeatures); }
+  FPOptions getFPFeatures() const {
+return FPOptions(BinaryOperatorBits.FPFeatures);
+  }
 
   // Get the FP contractability status of this operator. Only meaningful for
   // operations on floating point types.
   bool isFPContractableWithinStatement() const {
-return FPOptions(FPFeatures).allowFPContractWithinStatement();
+return getFPFeatures().allowFPContractWithinStatement();
   }
 
   // Get the FENV_ACCESS status of this operator. Only meaningful for
   // operations on floating point types.
-  bool isFEnvAccessOn() const {
-return FPOptions(FPFeatur

[clang-tools-extra] r346955 - [clangd] global-symbol-builder => clangd-indexer

[Haojian Wu via cfe-commits]

Author: hokein
Date: Thu Nov 15 06:15:19 2018
New Revision: 346955

URL: http://llvm.org/viewvc/llvm-project?rev=346955&view=rev
Log:
[clangd] global-symbol-builder => clangd-indexer

Modified:
clang-tools-extra/trunk/clangd/index/dex/dexp/Dexp.cpp

Modified: clang-tools-extra/trunk/clangd/index/dex/dexp/Dexp.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/dex/dexp/Dexp.cpp?rev=346955&r1=346954&r2=346955&view=diff
==
--- clang-tools-extra/trunk/clangd/index/dex/dexp/Dexp.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/dex/dexp/Dexp.cpp Thu Nov 15 06:15:19 
2018
@@ -33,7 +33,7 @@ cl::opt IndexPath("index-pa
 
 static const std::string Overview = R"(
 This is an **experimental** interactive tool to process user-provided search
-queries over given symbol collection obtained via global-symbol-builder. The
+queries over given symbol collection obtained via clangd-indexer. The
 tool can be used to evaluate search quality of existing index implementations
 and manually construct non-trivial test cases.
 


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

[PATCH] D54579: [clang-tidy] Update checks to play nicely with limited traversal scope added in r346847

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: hokein.
Herald added subscribers: cfe-commits, xazax.hun.

(See https://reviews.llvm.org/D54204 for original review)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54579

Files:
  clang-tidy/misc/UnusedParametersCheck.cpp
  clang-tidy/modernize/LoopConvertCheck.cpp
  clang-tidy/modernize/LoopConvertUtils.h
  clang-tidy/readability/SimplifyBooleanExprCheck.cpp
  clang-tidy/readability/SimplifyBooleanExprCheck.h


Index: clang-tidy/readability/SimplifyBooleanExprCheck.h
===
--- clang-tidy/readability/SimplifyBooleanExprCheck.h
+++ clang-tidy/readability/SimplifyBooleanExprCheck.h
@@ -79,6 +79,7 @@
  SourceLocation Loc, StringRef Description,
  SourceRange ReplacementRange, StringRef Replacement);
 
+  bool MatchedOnce = false;
   const bool ChainedConditionalReturn;
   const bool ChainedConditionalAssignment;
 };
Index: clang-tidy/readability/SimplifyBooleanExprCheck.cpp
===
--- clang-tidy/readability/SimplifyBooleanExprCheck.cpp
+++ clang-tidy/readability/SimplifyBooleanExprCheck.cpp
@@ -507,8 +507,15 @@
 ChainedConditionalAssignment);
 }
 
+// This is a silly hack to let us run a RecursiveASTVisitor on the Context.
+AST_MATCHER_P(Decl, matchOnce, bool *, Matched) {
+  if (*Matched)
+return false;
+  return *Matched = true;
+}
+
 void SimplifyBooleanExprCheck::registerMatchers(MatchFinder *Finder) {
-  Finder->addMatcher(translationUnitDecl().bind("top"), this);
+  Finder->addMatcher(matchOnce(&MatchedOnce), this);
 
   matchBoolCondition(Finder, true, ConditionThenStmtId);
   matchBoolCondition(Finder, false, ConditionElseStmtId);
@@ -556,8 +563,8 @@
   else if (const auto *Compound =
Result.Nodes.getNodeAs(CompoundNotBoolId))
 replaceCompoundReturnWithCondition(Result, Compound, true);
-  else if (const auto TU = Result.Nodes.getNodeAs("top"))
-Visitor(this, Result).TraverseDecl(const_cast(TU));
+  else // MatchOnce matcher
+Visitor(this, Result).TraverseAST(*Result.Context);
 }
 
 void SimplifyBooleanExprCheck::issueDiag(
Index: clang-tidy/modernize/LoopConvertUtils.h
===
--- clang-tidy/modernize/LoopConvertUtils.h
+++ clang-tidy/modernize/LoopConvertUtils.h
@@ -59,9 +59,9 @@
   /// \brief Run the analysis on the TranslationUnitDecl.
   ///
   /// In case we're running this analysis multiple times, don't repeat the 
work.
-  void gatherAncestors(const clang::TranslationUnitDecl *T) {
+  void gatherAncestors(ASTContext &Ctx) {
 if (StmtAncestors.empty())
-  TraverseDecl(const_cast(T));
+  TraverseAST(Ctx);
   }
 
   /// Accessor for StmtAncestors.
Index: clang-tidy/modernize/LoopConvertCheck.cpp
===
--- clang-tidy/modernize/LoopConvertCheck.cpp
+++ clang-tidy/modernize/LoopConvertCheck.cpp
@@ -899,7 +899,7 @@
   // variable declared inside the loop outside of it.
   // FIXME: Determine when the external dependency isn't an expression 
converted
   // by another loop.
-  TUInfo->getParentFinder().gatherAncestors(Context->getTranslationUnitDecl());
+  TUInfo->getParentFinder().gatherAncestors(*Context);
   DependencyFinderASTVisitor DependencyFinder(
   &TUInfo->getParentFinder().getStmtToParentStmtMap(),
   &TUInfo->getParentFinder().getDeclToParentStmtMap(),
Index: clang-tidy/misc/UnusedParametersCheck.cpp
===
--- clang-tidy/misc/UnusedParametersCheck.cpp
+++ clang-tidy/misc/UnusedParametersCheck.cpp
@@ -74,7 +74,7 @@
 class UnusedParametersCheck::IndexerVisitor
 : public RecursiveASTVisitor {
 public:
-  IndexerVisitor(TranslationUnitDecl *Top) { TraverseDecl(Top); }
+  IndexerVisitor(ASTContext &Ctx) { TraverseAST(Ctx); }
 
   const std::unordered_set &
   getFnCalls(const FunctionDecl *Fn) {
@@ -136,8 +136,7 @@
   auto MyDiag = diag(Param->getLocation(), "parameter %0 is unused") << Param;
 
   if (!Indexer) {
-Indexer = llvm::make_unique(
-Result.Context->getTranslationUnitDecl());
+Indexer = llvm::make_unique(*Result.Context);
   }
 
   // Comment out parameter name for non-local functions.


Index: clang-tidy/readability/SimplifyBooleanExprCheck.h
===
--- clang-tidy/readability/SimplifyBooleanExprCheck.h
+++ clang-tidy/readability/SimplifyBooleanExprCheck.h
@@ -79,6 +79,7 @@
  SourceLocation Loc, StringRef Description,
  SourceRange ReplacementRange, StringRef Replacement);
 
+  bool MatchedOnce = false;
   const bool ChainedConditionalReturn;
   const bool ChainedConditionalAssignment;
 };
Index: clang-tidy/readability/SimplifyBooleanExprCheck.cpp
===

[PATCH] D54553: [clangd] Fix crash hovering on non-decltype trailing return

[Marc-Andre Laperle via Phabricator via cfe-commits]

malaperle updated this revision to Diff 174202.
malaperle added a comment.

Address comment.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54553

Files:
  clangd/XRefs.cpp
  unittests/clangd/XRefsTests.cpp


Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -835,6 +835,14 @@
   )cpp",
   "",
   },
+  {
+  R"cpp(// simple trailing return type
+^auto main() -> int {
+  return 0;
+}
+  )cpp",
+  "int",
+  },
   {
   R"cpp(// auto function return with trailing type
 struct Bar {};
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -605,6 +605,7 @@
   // Handle auto return types:
   //- auto foo() {}
   //- auto& foo() {}
+  //- auto foo() -> int {}
   //- auto foo() -> decltype(1+1) {}
   //- operator auto() const { return 10; }
   bool VisitFunctionDecl(FunctionDecl *D) {
@@ -624,12 +625,13 @@
 const AutoType *AT = D->getReturnType()->getContainedAutoType();
 if (AT && !AT->getDeducedType().isNull()) {
   DeducedType = AT->getDeducedType();
-} else {
+} else if (auto DT = dyn_cast(D->getReturnType())) {
   // auto in a trailing return type just points to a DecltypeType and
   // getContainedAutoType does not unwrap it.
-  const DecltypeType *DT = dyn_cast(D->getReturnType());
   if (!DT->getUnderlyingType().isNull())
 DeducedType = DT->getUnderlyingType();
+} else if (!D->getReturnType().isNull()) {
+  DeducedType = D->getReturnType();
 }
 return true;
   }


Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -835,6 +835,14 @@
   )cpp",
   "",
   },
+  {
+  R"cpp(// simple trailing return type
+^auto main() -> int {
+  return 0;
+}
+  )cpp",
+  "int",
+  },
   {
   R"cpp(// auto function return with trailing type
 struct Bar {};
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -605,6 +605,7 @@
   // Handle auto return types:
   //- auto foo() {}
   //- auto& foo() {}
+  //- auto foo() -> int {}
   //- auto foo() -> decltype(1+1) {}
   //- operator auto() const { return 10; }
   bool VisitFunctionDecl(FunctionDecl *D) {
@@ -624,12 +625,13 @@
 const AutoType *AT = D->getReturnType()->getContainedAutoType();
 if (AT && !AT->getDeducedType().isNull()) {
   DeducedType = AT->getDeducedType();
-} else {
+} else if (auto DT = dyn_cast(D->getReturnType())) {
   // auto in a trailing return type just points to a DecltypeType and
   // getContainedAutoType does not unwrap it.
-  const DecltypeType *DT = dyn_cast(D->getReturnType());
   if (!DT->getUnderlyingType().isNull())
 DeducedType = DT->getUnderlyingType();
+} else if (!D->getReturnType().isNull()) {
+  DeducedType = D->getReturnType();
 }
 return true;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54579: [clang-tidy] Update checks to play nicely with limited traversal scope added in r346847

[Sam McCall via Phabricator via cfe-commits]

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

Address comments from the last round of review in 
https://reviews.llvm.org/D54204.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54579

Files:
  clang-tidy/misc/UnusedParametersCheck.cpp
  clang-tidy/modernize/LoopConvertCheck.cpp
  clang-tidy/modernize/LoopConvertUtils.h
  clang-tidy/readability/SimplifyBooleanExprCheck.cpp
  clang-tidy/readability/SimplifyBooleanExprCheck.h

Index: clang-tidy/readability/SimplifyBooleanExprCheck.h
===
--- clang-tidy/readability/SimplifyBooleanExprCheck.h
+++ clang-tidy/readability/SimplifyBooleanExprCheck.h
@@ -79,6 +79,7 @@
  SourceLocation Loc, StringRef Description,
  SourceRange ReplacementRange, StringRef Replacement);
 
+  bool MatchedOnce = false;
   const bool ChainedConditionalReturn;
   const bool ChainedConditionalAssignment;
 };
Index: clang-tidy/readability/SimplifyBooleanExprCheck.cpp
===
--- clang-tidy/readability/SimplifyBooleanExprCheck.cpp
+++ clang-tidy/readability/SimplifyBooleanExprCheck.cpp
@@ -507,8 +507,16 @@
 ChainedConditionalAssignment);
 }
 
+// This is a silly hack to let us run a RecursiveASTVisitor on the Context.
+// We want to match exactly one node in the AST, doesn't matter which.
+AST_MATCHER_P(Decl, matchOnce, bool *, Matched) {
+  if (*Matched)
+return false;
+  return *Matched = true;
+}
+
 void SimplifyBooleanExprCheck::registerMatchers(MatchFinder *Finder) {
-  Finder->addMatcher(translationUnitDecl().bind("top"), this);
+  Finder->addMatcher(matchOnce(&MatchedOnce), this);
 
   matchBoolCondition(Finder, true, ConditionThenStmtId);
   matchBoolCondition(Finder, false, ConditionElseStmtId);
@@ -556,8 +564,8 @@
   else if (const auto *Compound =
Result.Nodes.getNodeAs(CompoundNotBoolId))
 replaceCompoundReturnWithCondition(Result, Compound, true);
-  else if (const auto TU = Result.Nodes.getNodeAs("top"))
-Visitor(this, Result).TraverseDecl(const_cast(TU));
+  else // MatchOnce matcher
+Visitor(this, Result).TraverseAST(*Result.Context);
 }
 
 void SimplifyBooleanExprCheck::issueDiag(
Index: clang-tidy/modernize/LoopConvertUtils.h
===
--- clang-tidy/modernize/LoopConvertUtils.h
+++ clang-tidy/modernize/LoopConvertUtils.h
@@ -56,12 +56,12 @@
 public:
   StmtAncestorASTVisitor() { StmtStack.push_back(nullptr); }
 
-  /// \brief Run the analysis on the TranslationUnitDecl.
+  /// \brief Run the analysis on the AST.
   ///
   /// In case we're running this analysis multiple times, don't repeat the work.
-  void gatherAncestors(const clang::TranslationUnitDecl *T) {
+  void gatherAncestors(ASTContext &Ctx) {
 if (StmtAncestors.empty())
-  TraverseDecl(const_cast(T));
+  TraverseAST(Ctx);
   }
 
   /// Accessor for StmtAncestors.
Index: clang-tidy/modernize/LoopConvertCheck.cpp
===
--- clang-tidy/modernize/LoopConvertCheck.cpp
+++ clang-tidy/modernize/LoopConvertCheck.cpp
@@ -899,7 +899,7 @@
   // variable declared inside the loop outside of it.
   // FIXME: Determine when the external dependency isn't an expression converted
   // by another loop.
-  TUInfo->getParentFinder().gatherAncestors(Context->getTranslationUnitDecl());
+  TUInfo->getParentFinder().gatherAncestors(*Context);
   DependencyFinderASTVisitor DependencyFinder(
   &TUInfo->getParentFinder().getStmtToParentStmtMap(),
   &TUInfo->getParentFinder().getDeclToParentStmtMap(),
Index: clang-tidy/misc/UnusedParametersCheck.cpp
===
--- clang-tidy/misc/UnusedParametersCheck.cpp
+++ clang-tidy/misc/UnusedParametersCheck.cpp
@@ -74,7 +74,7 @@
 class UnusedParametersCheck::IndexerVisitor
 : public RecursiveASTVisitor {
 public:
-  IndexerVisitor(TranslationUnitDecl *Top) { TraverseDecl(Top); }
+  IndexerVisitor(ASTContext &Ctx) { TraverseAST(Ctx); }
 
   const std::unordered_set &
   getFnCalls(const FunctionDecl *Fn) {
@@ -136,8 +136,7 @@
   auto MyDiag = diag(Param->getLocation(), "parameter %0 is unused") << Param;
 
   if (!Indexer) {
-Indexer = llvm::make_unique(
-Result.Context->getTranslationUnitDecl());
+Indexer = llvm::make_unique(*Result.Context);
   }
 
   // Comment out parameter name for non-local functions.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54204: [clangd] Initial clang-tidy diagnostics support.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Moved the clang-tidy changes to https://reviews.llvm.org/D54579.
Sorry for mixing everything up!




Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.cpp:518
 void SimplifyBooleanExprCheck::registerMatchers(MatchFinder *Finder) {
-  Finder->addMatcher(translationUnitDecl().bind("top"), this);
+  Finder->addMatcher(matchOnce(&MatchedOnce), this);
 

hokein wrote:
> maybe add a comment  describing we are trying to find the top level decl?
We're not, we're trying to match any node (but only one). Extended the comment 
on the matcher.



Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.cpp:566
 replaceCompoundReturnWithCondition(Result, Compound, true);
-  else if (const auto TU = Result.Nodes.getNodeAs("top"))
-Visitor(this, Result).TraverseDecl(const_cast(TU));
+  else // MatchOnce matcher
+Visitor(this, Result).TraverseAST(*Result.Context);

hokein wrote:
> add an `assert (MatchOnce)`?
That doesn't compile, I'm not sure what you want here.



Comment at: clangd/ClangdUnit.cpp:158
+  // Clang-tidy has some limitiations to ensure reasonable performance:
+  //  - checks don't see all preprocessor events in the preamble
+  //  - matchers run only over the main-file top-level decls (and can't see

hokein wrote:
> just want to make sure -- do we receive all preprocessor events in the main 
> file when we use preamble? We have a few checks that generate `#include` 
> insertion as part of FixIt.
> 
> `TestTU` doesn't use preamble, it would be nice to have a test running on a 
> main file with a real preamble, but this can be a follow-up I think.
Nope, you're right: `#include` from the preamble are not replayed. Can't fix 
this now, added a FIXME. (Neither of the hardcoded checks care).

TestTU does use a preamble now :-)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54204



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

[PATCH] D54204: [clangd] Initial clang-tidy diagnostics support.

[Sam McCall via Phabricator via cfe-commits]

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

Remove clang-tidy changes, add FIXME comment.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54204

Files:
  clangd/CMakeLists.txt
  clangd/ClangdUnit.cpp
  clangd/XRefs.cpp
  unittests/clangd/ClangdUnitTests.cpp

Index: unittests/clangd/ClangdUnitTests.cpp
===
--- unittests/clangd/ClangdUnitTests.cpp
+++ unittests/clangd/ClangdUnitTests.cpp
@@ -24,6 +24,7 @@
 using testing::Field;
 using testing::IsEmpty;
 using testing::Pair;
+using testing::UnorderedElementsAre;
 
 testing::Matcher WithFix(testing::Matcher FixMatcher) {
   return Field(&Diag::Fixes, ElementsAre(FixMatcher));
@@ -128,6 +129,30 @@
   WithFix(Fix(Test.range(), "int", "change return type to 'int'");
 }
 
+TEST(DiagnosticsTest, ClangTidy) {
+  Annotations Test(R"cpp(
+#define $macrodef[[SQUARE]](X) (X)*(X)
+int main() {
+  return [[sizeof]](sizeof(int));
+  int y = 4;
+  return SQUARE($macroarg[[++]]y);
+}
+  )cpp");
+  auto TU = TestTU::withCode(Test.code());
+  EXPECT_THAT(
+  TU.build().getDiagnostics(),
+  UnorderedElementsAre(
+  Diag(Test.range(), "suspicious usage of 'sizeof(sizeof(...))' "
+ "[bugprone-sizeof-expression]"),
+  AllOf(
+  Diag(Test.range("macroarg"),
+   "side effects in the 1st macro argument 'X' are repeated in "
+   "macro expansion [bugprone-macro-repeated-side-effects]"),
+  WithNote(Diag(Test.range("macrodef"),
+"macro 'SQUARE' defined here "
+"[bugprone-macro-repeated-side-effects]");
+}
+
 TEST(DiagnosticsTest, Preprocessor) {
   // This looks like a preamble, but there's an #else in the middle!
   // Check that:
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -672,8 +672,7 @@
 return {};
 
   DeducedTypeVisitor V(SourceLocationBeg);
-  for (Decl *D : AST.getLocalTopLevelDecls())
-V.TraverseDecl(D);
+  V.TraverseAST(AST.getASTContext());
   return V.getDeducedType();
 }
 
Index: clangd/ClangdUnit.cpp
===
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -8,6 +8,8 @@
 //===--===//
 
 #include "ClangdUnit.h"
+#include "../clang-tidy/ClangTidyDiagnosticConsumer.h"
+#include "../clang-tidy/ClangTidyModuleRegistry.h"
 #include "Compiler.h"
 #include "Diagnostics.h"
 #include "Logger.h"
@@ -151,6 +153,40 @@
 return None;
   }
 
+  // Set up ClangTidy. Must happen after BeginSourceFile() so ASTContext exists.
+  // Clang-tidy has some limitiations to ensure reasonable performance:
+  //  - checks don't see all preprocessor events in the preamble
+  //  - matchers run only over the main-file top-level decls (and can't see
+  //ancestors outside this scope).
+  // In practice almost all checks work well without modifications.
+  std::vector> CTChecks;
+  ast_matchers::MatchFinder CTFinder;
+  llvm::Optional CTContext;
+  {
+trace::Span Tracer("ClangTidyInit");
+tidy::ClangTidyCheckFactories CTFactories;
+for (const auto &E : tidy::ClangTidyModuleRegistry::entries())
+  E.instantiate()->addCheckFactories(CTFactories);
+auto CTOpts = tidy::ClangTidyOptions::getDefaults();
+// FIXME: this needs to be configurable, and we need to support .clang-tidy
+// files and other options providers.
+// These checks exercise the matcher- and preprocessor-based hooks.
+CTOpts.Checks =
+"bugprone-sizeof-expression,bugprone-macro-repeated-side-effects";
+CTContext.emplace(llvm::make_unique(
+tidy::ClangTidyGlobalOptions(), CTOpts));
+CTContext->setDiagnosticsEngine(&Clang->getDiagnostics());
+CTContext->setASTContext(&Clang->getASTContext());
+CTContext->setCurrentFile(MainInput.getFile());
+CTFactories.createChecks(CTContext.getPointer(), CTChecks);
+for (const auto &Check : CTChecks) {
+  // FIXME: the PP callbacks skip the entire preamble.
+  // Checks that want to see #includes in the main file do not see them.
+  Check->registerPPCallbacks(*Clang);
+  Check->registerMatchers(&CTFinder);
+}
+  }
+
   // Copy over the includes from the preamble, then combine with the
   // non-preamble includes below.
   auto Includes = Preamble ? Preamble->Includes : IncludeStructure{};
@@ -160,13 +196,26 @@
   if (!Action->Execute())
 log("Execute() failed when building AST for {0}", MainInput.getFile());
 
+  std::vector ParsedDecls = Action->takeTopLevelDecls();
+  // AST traversals should exclude the preamble, to avoid performance cliffs.
+  Clang->getASTContext().setTraversalScope(ParsedDecls);
+  {
+// Run the AST-dependent part of the clang-tidy che

r346957 - [AST][NFC] Re-add comment in BinaryOperator which was removed by r346954

[Bruno Ricci via cfe-commits]

Author: brunoricci
Date: Thu Nov 15 06:30:18 2018
New Revision: 346957

URL: http://llvm.org/viewvc/llvm-project?rev=346957&view=rev
Log:
[AST][NFC] Re-add comment in BinaryOperator which was removed by r346954


Modified:
cfe/trunk/include/clang/AST/Stmt.h

Modified: cfe/trunk/include/clang/AST/Stmt.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Stmt.h?rev=346957&r1=346956&r2=346957&view=diff
==
--- cfe/trunk/include/clang/AST/Stmt.h (original)
+++ cfe/trunk/include/clang/AST/Stmt.h Thu Nov 15 06:30:18 2018
@@ -448,6 +448,9 @@ protected:
 unsigned : NumExprBits;
 
 unsigned Opc : 6;
+
+/// This is only meaningful for operations on floating point
+/// types and 0 otherwise.
 unsigned FPFeatures : 3;
 
 SourceLocation OpLoc;


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

[PATCH] D54489: Implement -frecord-command-line (-frecord-gcc-switches)

[Scott Linder via Phabricator via cfe-commits]

scott.linder added inline comments.



Comment at: docs/ClangCommandLineReference.rst:797
+
+Generate a section .LLVM.command.line containing the clang driver command line.
+

rjmccall wrote:
> 1. Is this section always called `.LLVM.command.line`, or does it differ by 
> target object-file format?
> 2. For that matter, is this actually supported by all object-file formats?
> 3. Please describe the format of the section.  Is it just a flat C string?  
> Is it nul-terminated?  Are different options combined with spaces, and if so, 
> does that mean that interior spaces in command line arguments are impossible 
> to distinguish?  If the latter, I assume that this is required behavior for 
> backward compatibility; please at least apologize for that in the 
> documentation. :)
For (1) I think the answer should be that we emit `.LLVM.command.line` (or 
whatever name we land on) for every object-file format with the concept of 
names for sections.

For (2), I have only implemented this for ELF so far in LLVM. I don't know how 
reasonable that is, and if it isn't I can look at adding it to other common 
object-file formats that LLVM supports.

For (3), my current proposal for the format is the same as the `.comment` 
section for idents: null-surrounded strings. Interior spaces are escaped, in 
the same manner as for the -g variant. There may still be more thought to put 
into the format, as the GCC version null-terminates each individual option; the 
reason I avoided this is that during object linking the options of many 
command-lines simply get mixed together, which seems less than useful.

As an example, for ELF `clang -frecord-command-line -S "foo .c"` produces the 
ASM:

```
.section.LLVM.command.line,"MS",@progbits,1
.zero   1
.ascii  "/usr/local/bin/clang-8 -frecord-command-line -S foo\\ .c"
.zero   1
```

And for multiple command-lines in a single object (e.g. linking three objects 
with recorded command-lines) this would be:

```
.section.LLVM.command.line,"MS",@progbits,1
.zero   1
.ascii  "/usr/local/bin/clang-8 -frecord-command-line -c foo\\ .c"
.zero   1
.ascii  "/usr/local/bin/clang-8 -frecord-command-line -some -unique -options -c 
bar.c"
.zero   1
.ascii  "/usr/local/bin/clang-8 -frecord-command-line -something -else -c baz.c"
.zero   1
```

I will try to capture more of this in the documentation.


https://reviews.llvm.org/D54489



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

[PATCH] D45045: [DebugInfo] Generate debug information for labels.

[Hsiangkai Wang via Phabricator via cfe-commits]

HsiangKai added a comment.

In https://reviews.llvm.org/D45045#1299507, @tyb0807 wrote:

> Hello all,
>
> This commit has been reverted by https://reviews.llvm.org/rC345026. It was 
> reported that this broke the Chromium build (again). Have you had a look to 
> fix this, @HsiangKai?


I have fixed these bugs found in Chromium build in following two commits.

https://reviews.llvm.org/D54199
https://reviews.llvm.org/D54465

These two commits are under reviewing.
Thanks for your reminding.


Repository:
  rC Clang

https://reviews.llvm.org/D45045



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

[PATCH] D54579: [clang-tidy] Update checks to play nicely with limited traversal scope added in r346847

[Haojian Wu via Phabricator via cfe-commits]

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

LGTM




Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.cpp:567
 replaceCompoundReturnWithCondition(Result, Compound, true);
-  else if (const auto TU = Result.Nodes.getNodeAs("top"))
-Visitor(this, Result).TraverseDecl(const_cast(TU));
+  else // MatchOnce matcher
+Visitor(this, Result).TraverseAST(*Result.Context);

Ah, what I mean is `assert(MatchedOnce)`.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54579



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

[PATCH] D54204: [clangd] Initial clang-tidy diagnostics support.

[Haojian Wu via Phabricator via cfe-commits]

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

looks good!


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54204



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

[clang-tools-extra] r346961 - [clang-tidy] Update checks to play nicely with limited traversal scope added in r346847

[Sam McCall via cfe-commits]

Author: sammccall
Date: Thu Nov 15 07:06:11 2018
New Revision: 346961

URL: http://llvm.org/viewvc/llvm-project?rev=346961&view=rev
Log:
[clang-tidy] Update checks to play nicely with limited traversal scope added in 
r346847

Summary: (See D54204 for original review)

Reviewers: hokein

Subscribers: xazax.hun, cfe-commits

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

Modified:
clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp
clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp
clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertUtils.h
clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.h

Modified: clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp?rev=346961&r1=346960&r2=346961&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp Thu Nov 
15 07:06:11 2018
@@ -74,7 +74,7 @@ static FixItHint removeArgument(const Ma
 class UnusedParametersCheck::IndexerVisitor
 : public RecursiveASTVisitor {
 public:
-  IndexerVisitor(TranslationUnitDecl *Top) { TraverseDecl(Top); }
+  IndexerVisitor(ASTContext &Ctx) { TraverseAST(Ctx); }
 
   const std::unordered_set &
   getFnCalls(const FunctionDecl *Fn) {
@@ -136,8 +136,7 @@ void UnusedParametersCheck::warnOnUnused
   auto MyDiag = diag(Param->getLocation(), "parameter %0 is unused") << Param;
 
   if (!Indexer) {
-Indexer = llvm::make_unique(
-Result.Context->getTranslationUnitDecl());
+Indexer = llvm::make_unique(*Result.Context);
   }
 
   // Comment out parameter name for non-local functions.

Modified: clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp?rev=346961&r1=346960&r2=346961&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp Thu Nov 
15 07:06:11 2018
@@ -899,7 +899,7 @@ void LoopConvertCheck::check(const Match
   // variable declared inside the loop outside of it.
   // FIXME: Determine when the external dependency isn't an expression 
converted
   // by another loop.
-  TUInfo->getParentFinder().gatherAncestors(Context->getTranslationUnitDecl());
+  TUInfo->getParentFinder().gatherAncestors(*Context);
   DependencyFinderASTVisitor DependencyFinder(
   &TUInfo->getParentFinder().getStmtToParentStmtMap(),
   &TUInfo->getParentFinder().getDeclToParentStmtMap(),

Modified: clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertUtils.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertUtils.h?rev=346961&r1=346960&r2=346961&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertUtils.h (original)
+++ clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertUtils.h Thu Nov 15 
07:06:11 2018
@@ -56,12 +56,12 @@ class StmtAncestorASTVisitor
 public:
   StmtAncestorASTVisitor() { StmtStack.push_back(nullptr); }
 
-  /// \brief Run the analysis on the TranslationUnitDecl.
+  /// \brief Run the analysis on the AST.
   ///
   /// In case we're running this analysis multiple times, don't repeat the 
work.
-  void gatherAncestors(const clang::TranslationUnitDecl *T) {
+  void gatherAncestors(ASTContext &Ctx) {
 if (StmtAncestors.empty())
-  TraverseDecl(const_cast(T));
+  TraverseAST(Ctx);
   }
 
   /// Accessor for StmtAncestors.

Modified: 
clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.cpp?rev=346961&r1=346960&r2=346961&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.cpp 
Thu Nov 15 07:06:11 2018
@@ -507,8 +507,16 @@ void SimplifyBooleanExprCheck::storeOpti
 ChainedConditionalAssignment);
 }
 
+// This is a silly hack to let us run a RecursiveASTVisitor on the Context.
+// We want to match exactly one node in the AST, doesn't matter which.
+AST_MATCHER_P(Decl, matchOnce, bool *, Matched) {
+  if (*Matched)
+return false;
+  return *Matched = true;
+}
+
 void SimplifyBooleanExprCheck::registerMatchers(MatchFinder *Finder) {
-  Finder->addMatcher(translation

[PATCH] D54579: [clang-tidy] Update checks to play nicely with limited traversal scope added in r346847

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL346961: [clang-tidy] Update checks to play nicely with 
limited traversal scope added in… (authored by sammccall, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D54579?vs=174203&id=174209#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D54579

Files:
  clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp
  clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp
  clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertUtils.h
  clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
  clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.h

Index: clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp
@@ -899,7 +899,7 @@
   // variable declared inside the loop outside of it.
   // FIXME: Determine when the external dependency isn't an expression converted
   // by another loop.
-  TUInfo->getParentFinder().gatherAncestors(Context->getTranslationUnitDecl());
+  TUInfo->getParentFinder().gatherAncestors(*Context);
   DependencyFinderASTVisitor DependencyFinder(
   &TUInfo->getParentFinder().getStmtToParentStmtMap(),
   &TUInfo->getParentFinder().getDeclToParentStmtMap(),
Index: clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertUtils.h
===
--- clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertUtils.h
+++ clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertUtils.h
@@ -56,12 +56,12 @@
 public:
   StmtAncestorASTVisitor() { StmtStack.push_back(nullptr); }
 
-  /// \brief Run the analysis on the TranslationUnitDecl.
+  /// \brief Run the analysis on the AST.
   ///
   /// In case we're running this analysis multiple times, don't repeat the work.
-  void gatherAncestors(const clang::TranslationUnitDecl *T) {
+  void gatherAncestors(ASTContext &Ctx) {
 if (StmtAncestors.empty())
-  TraverseDecl(const_cast(T));
+  TraverseAST(Ctx);
   }
 
   /// Accessor for StmtAncestors.
Index: clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp
@@ -74,7 +74,7 @@
 class UnusedParametersCheck::IndexerVisitor
 : public RecursiveASTVisitor {
 public:
-  IndexerVisitor(TranslationUnitDecl *Top) { TraverseDecl(Top); }
+  IndexerVisitor(ASTContext &Ctx) { TraverseAST(Ctx); }
 
   const std::unordered_set &
   getFnCalls(const FunctionDecl *Fn) {
@@ -136,8 +136,7 @@
   auto MyDiag = diag(Param->getLocation(), "parameter %0 is unused") << Param;
 
   if (!Indexer) {
-Indexer = llvm::make_unique(
-Result.Context->getTranslationUnitDecl());
+Indexer = llvm::make_unique(*Result.Context);
   }
 
   // Comment out parameter name for non-local functions.
Index: clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
@@ -507,8 +507,16 @@
 ChainedConditionalAssignment);
 }
 
+// This is a silly hack to let us run a RecursiveASTVisitor on the Context.
+// We want to match exactly one node in the AST, doesn't matter which.
+AST_MATCHER_P(Decl, matchOnce, bool *, Matched) {
+  if (*Matched)
+return false;
+  return *Matched = true;
+}
+
 void SimplifyBooleanExprCheck::registerMatchers(MatchFinder *Finder) {
-  Finder->addMatcher(translationUnitDecl().bind("top"), this);
+  Finder->addMatcher(matchOnce(&MatchedOnce), this);
 
   matchBoolCondition(Finder, true, ConditionThenStmtId);
   matchBoolCondition(Finder, false, ConditionElseStmtId);
@@ -556,8 +564,10 @@
   else if (const auto *Compound =
Result.Nodes.getNodeAs(CompoundNotBoolId))
 replaceCompoundReturnWithCondition(Result, Compound, true);
-  else if (const auto TU = Result.Nodes.getNodeAs("top"))
-Visitor(this, Result).TraverseDecl(const_cast(TU));
+  else { // MatchOnce matcher
+assert(MatchedOnce);
+Visitor(this, Result).TraverseAST(*Result.Context);
+  }
 }
 
 void SimplifyBooleanExprCheck::issueDiag(
Index: clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.h
===
--- clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.h
+++ clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.h
@@ -79,6 +79,7 @@

[PATCH] D54556: [analyzer] MoveChecker Pt.1: Give MisusedMovedObject checker a more consistent name.

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

Szelethus accepted this revision.
Szelethus added a comment.

LGTM, both the concept and the patch! I have read your followup patches up to 
part 4, I'll leave some thoughts there.


Repository:
  rC Clang

https://reviews.llvm.org/D54556



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

[PATCH] D54269: Introduce shard storage to auto-index.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 174215.
kadircet marked 13 inline comments as done.
kadircet added a comment.

- Address comments.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54269

Files:
  clangd/CMakeLists.txt
  clangd/index/Background.cpp
  clangd/index/Background.h
  clangd/index/BackgroundIndexStorage.cpp
  unittests/clangd/BackgroundIndexTests.cpp

Index: unittests/clangd/BackgroundIndexTests.cpp
===
--- unittests/clangd/BackgroundIndexTests.cpp
+++ unittests/clangd/BackgroundIndexTests.cpp
@@ -1,6 +1,7 @@
 #include "SyncAPI.h"
 #include "TestFS.h"
 #include "index/Background.h"
+#include "llvm/Support/ScopedPrinter.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -22,6 +23,37 @@
   return ElementsAre(testing::Pair(_, UnorderedElementsAreArray(Matchers)));
 }
 
+class MemoryShardStorage : public BackgroundIndexStorage {
+  mutable std::mutex StorageMu;
+  llvm::StringMap &Storage;
+  size_t &CacheHits;
+
+public:
+  MemoryShardStorage(llvm::StringMap &Storage, size_t &CacheHits)
+  : Storage(Storage), CacheHits(CacheHits) {}
+  llvm::Error storeShard(llvm::StringRef ShardIdentifier,
+ IndexFileOut Shard) const override {
+std::lock_guard Lock(StorageMu);
+Storage[ShardIdentifier] = llvm::to_string(Shard);
+return llvm::Error::success();
+  }
+  std::unique_ptr
+  loadShard(llvm::StringRef ShardIdentifier) const override {
+std::lock_guard Lock(StorageMu);
+if (Storage.find(ShardIdentifier) == Storage.end()) {
+  return nullptr;
+}
+auto IndexFile = readIndexFile(Storage[ShardIdentifier]);
+if (!IndexFile) {
+  ADD_FAILURE() << "Error while reading " << ShardIdentifier << ':'
+<< IndexFile.takeError();
+  return nullptr;
+}
+CacheHits++;
+return llvm::make_unique(std::move(*IndexFile));
+  }
+};
+
 TEST(BackgroundIndexTest, IndexTwoFiles) {
   MockFSProvider FS;
   // a.h yields different symbols when included by A.cc vs B.cc.
@@ -43,7 +75,11 @@
   void f_b() {
 (void)common;
   })cpp";
-  BackgroundIndex Idx(Context::empty(), "", FS, /*URISchemes=*/{"unittest"});
+  llvm::StringMap Storage;
+  size_t CacheHits = 0;
+  MemoryShardStorage MSS(Storage, CacheHits);
+  BackgroundIndex Idx(Context::empty(), "", FS, /*URISchemes=*/{"unittest"},
+  [&](llvm::StringRef) { return &MSS; });
 
   tooling::CompileCommand Cmd;
   Cmd.Filename = testPath("root/A.cc");
@@ -76,5 +112,49 @@
FileURI("unittest:///root/B.cc")}));
 }
 
+TEST(BackgroundIndexTest, ShardStorageWriteTest) {
+  MockFSProvider FS;
+  FS.Files[testPath("root/A.h")] = R"cpp(
+  void common();
+  void f_b();
+  class A_CC {};
+  )cpp";
+  FS.Files[testPath("root/A.cc")] =
+  "#include \"A.h\"\nvoid g() { (void)common; }";
+
+  llvm::StringMap Storage;
+  size_t CacheHits = 0;
+  MemoryShardStorage MSS(Storage, CacheHits);
+
+  tooling::CompileCommand Cmd;
+  Cmd.Filename = testPath("root/A.cc");
+  Cmd.Directory = testPath("root");
+  Cmd.CommandLine = {"clang++", testPath("root/A.cc")};
+  // Check nothing is loaded from Storage, but A.cc and A.h has been stored.
+  {
+BackgroundIndex Idx(Context::empty(), "", FS, /*URISchemes=*/{"unittest"},
+[&](llvm::StringRef) { return &MSS; });
+Idx.enqueue(testPath("root"), Cmd);
+Idx.blockUntilIdleForTest();
+  }
+  EXPECT_EQ(CacheHits, 0U);
+  EXPECT_EQ(Storage.size(), 2U);
+
+  auto ShardHeader = MSS.loadShard(testPath("root/A.h"));
+  EXPECT_NE(ShardHeader, nullptr);
+  EXPECT_THAT(
+  *ShardHeader->Symbols,
+  UnorderedElementsAre(Named("common"), Named("A_CC"),
+   AllOf(Named("f_b"), Declared(), Not(Defined();
+  for (const auto &Ref : *ShardHeader->Refs)
+EXPECT_THAT(Ref.second,
+UnorderedElementsAre(FileURI("unittest:///root/A.h")));
+
+  auto ShardSource = MSS.loadShard(testPath("root/A.cc"));
+  EXPECT_NE(ShardSource, nullptr);
+  EXPECT_THAT(*ShardSource->Symbols, UnorderedElementsAre());
+  EXPECT_THAT(*ShardSource->Refs, RefsAre({FileURI("unittest:///root/A.cc")}));
+}
+
 } // namespace clangd
 } // namespace clang
Index: clangd/index/BackgroundIndexStorage.cpp
===
--- /dev/null
+++ clangd/index/BackgroundIndexStorage.cpp
@@ -0,0 +1,112 @@
+//== BackgroundIndexStorage.cpp - Provide caching support to BackgroundIndex ==/
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "Logger.h"
+#include "index/Background.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Path.h"
+#include "llv

[PATCH] D54557: [analyzer] MoveChecker Pt.2: Restrict the warning to STL objects and locals.

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

Szelethus added a comment.

I think we should either remove the non-default functionality (which wouldn't 
be ideal), or emphasise somewhere (open projects?) that there is still work to 
be done, but leaving it to be forgotten and essentially making it an extra 
maintenance work would be, in my optinion, the worst case scenario. 
`Aggressive` isn't `Pedantic` because it actually emits warnings on correct 
code, and it's not a simple matter of too many reports being emitted, let's 
also document that this is an experimental feature, not a power-user-only thing.

Whenever I compile `Rtags`, I get a bunch of move related warnings, I'm curious 
how this patch behaves on that project. I'll take a look.




Comment at: lib/StaticAnalyzer/Checkers/MoveChecker.cpp:92-95
+  bool IsAggressive = false;
+
+public:
+  void setAggressiveness(bool Aggressive) { IsAggressive = Aggressive; }

How about a simple public data member? Since all callbacks are const, it 
wouldn't be modified after registration anyways.



Comment at: lib/StaticAnalyzer/Checkers/MoveChecker.cpp:134
+static const MemRegion *unwrapRValueReferenceIndirection(const MemRegion *MR) {
+  if (const auto *SR = dyn_cast_or_null(MR)) {
+SymbolRef Sym = SR->getSymbol();

You use `dyn_cast_or_null`, which to me implies that the returned value may be 
`nullptr`, but you never check it on the call sites.



Comment at: lib/StaticAnalyzer/Checkers/MoveChecker.cpp:273-282
+  // In non-aggressive mode, only warn on use-after-move of local variables (or
+  // local rvalue references) and of STL objects. The former is possible 
because
+  // local variables (or local rvalue references) are not tempting their user 
to
+  // re-use the storage. The latter is possible because STL objects are known
+  // to end up in a valid but unspecified state after the move and their
+  // state-reset methods are also known, which allows us to predict
+  // precisely when use-after-move is invalid.

I've seen checker option descriptions in registry functions, in-class where 
they are declared, and on top of the file, I think any of them would be more 
ideal. Since my checker option related efforts are still far in the distance 
(which will put all of them in `Checkers.td`), let's move (or at least copy) 
this somewhere else.



Comment at: test/Analysis/use-after-move.cpp:47-52
+template 
+class vector {
+public:
+  vector();
+  void push_back(const T &t);
+};

Any particular reason for not using `cxx-system-header-simulator.h`?


Repository:
  rC Clang

https://reviews.llvm.org/D54557



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

[PATCH] D54560: [analyzer] MoveChecker Pt.3: Improve warning messages a bit.

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

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

Awesome! :D




Comment at: lib/StaticAnalyzer/Checkers/MoveChecker.cpp:385-386
+  }
+  // Provide the caller with the classification of the object
+  // we've obtained here accidentally, for later use.
+  return OK;

Maybe move this in-class?


Repository:
  rC Clang

https://reviews.llvm.org/D54560



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

[PATCH] D54563: [analyzer] MoveChecker Pt.4: Add a few more state reset methods.

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

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

How about `std::list::splice`?


Repository:
  rC Clang

https://reviews.llvm.org/D54563



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

[PATCH] D54547: PTH-- Remove feature entirely-

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added a comment.

In https://reviews.llvm.org/D54547#1299883, @erichkeane wrote:

> Added to the release notes.  Also, an email was sent out to cfe-dev.
>
> @riccibruno and I are separately looking into IdentifierInfo, because it 
> seems that there are some pretty massive optimization opportunities if we can 
> remove PTH/indirect identifiers.


As discussed on IRC:
In any case this is not blocking the removal of PTH.  The code around 
`IdentifierInfo`
is very performance sensitive and it will take some time to evaluate the 
possible
changes, if any.


https://reviews.llvm.org/D54547



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

[PATCH] D54557: [analyzer] MoveChecker Pt.2: Restrict the warning to STL objects and locals.

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

Szelethus added a comment.

Alpha checkers, at least to developers, are clearly stating "Please finish 
me!", but a non-alpha, enabled-by-default checker with a flag that you'd never 
know about unless you looked at the source code (at least up until I fix these) 
sounds like it'll be forgotten like many other similar flags.


Repository:
  rC Clang

https://reviews.llvm.org/D54557



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

[PATCH] D52273: [clangd] Initial implementation of expected types

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 174217.
ilya-biryukov marked 10 inline comments as done.
ilya-biryukov added a comment.

- Address comments


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52273

Files:
  clangd/CMakeLists.txt
  clangd/ExpectedTypes.cpp
  clangd/ExpectedTypes.h
  unittests/clangd/CMakeLists.txt
  unittests/clangd/ExpectedTypeTest.cpp

Index: unittests/clangd/ExpectedTypeTest.cpp
===
--- /dev/null
+++ unittests/clangd/ExpectedTypeTest.cpp
@@ -0,0 +1,198 @@
+//===-- ExpectedTypeTest.cpp  ---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "ClangdUnit.h"
+#include "ExpectedTypes.h"
+#include "TestTU.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "llvm/ADT/StringRef.h"
+#include "gmock/gmock-matchers.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+using ::testing::Field;
+using ::testing::Matcher;
+using ::testing::SizeIs;
+using ::testing::UnorderedElementsAreArray;
+
+class ASTTest : public ::testing::Test {
+protected:
+  void build(llvm::StringRef Code) {
+assert(!AST && "AST built twice");
+AST = TestTU::withCode(Code).build();
+  }
+
+  const ValueDecl *decl(llvm::StringRef Name) {
+return &llvm::cast(findDecl(*AST, Name));
+  }
+
+  QualType typeOf(llvm::StringRef Name) {
+return decl(Name)->getType().getCanonicalType();
+  }
+
+  ASTContext &ASTCtx() { return AST->getASTContext(); }
+
+private:
+  // Set after calling build().
+  llvm::Optional AST;
+};
+
+class ConvertibleToMatcher
+: public ::testing::MatcherInterface {
+  ASTContext &Ctx;
+  QualType To;
+  OpaqueType PreferredType;
+
+public:
+  ConvertibleToMatcher(ASTContext &Ctx, QualType To)
+  : Ctx(Ctx), To(To.getCanonicalType()),
+PreferredType(*OpaqueType::fromPreferredType(Ctx, To)) {}
+
+  void DescribeTo(std::ostream *OS) const override {
+*OS << "Is convertible to type '" << To.getAsString() << "'";
+  }
+
+  bool MatchAndExplain(const ValueDecl *V,
+   ::testing::MatchResultListener *L) const override {
+assert(V);
+assert(&V->getASTContext() == &Ctx && "different ASTs?");
+auto ConvertibleTo = OpaqueType::fromCompletionResult(
+Ctx, CodeCompletionResult(V, CCP_Declaration));
+
+bool Matched = PreferredType == ConvertibleTo;
+if (L->IsInterested())
+  *L << "Types of source and target "
+ << (Matched ? "matched" : "did not match")
+ << "\n\tTarget type: " << To.getAsString()
+ << "\n\tSource value type: " << V->getType().getAsString();
+return Matched;
+  }
+};
+
+class ExpectedTypeConversionTest : public ASTTest {
+protected:
+  Matcher isConvertibleTo(QualType To) {
+return ::testing::MakeMatcher(new ConvertibleToMatcher(ASTCtx(), To));
+  }
+
+  // A set of DeclNames whose type match each other computed by
+  // OpaqueType::fromCompletionResult.
+  using EquivClass = std::set;
+
+  Matcher>
+  ClassesAre(llvm::ArrayRef Classes) {
+using MapEntry = std::map::value_type;
+
+std::vector> Elements;
+Elements.reserve(Classes.size());
+for (auto &Cls : Classes)
+  Elements.push_back(Field(&MapEntry::second, Cls));
+return UnorderedElementsAreArray(Elements);
+  }
+
+  // Groups \p Decls into equivalence classes based on the result of
+  // 'OpaqueType::fromCompletionResult'.
+  std::map
+  buildEquivClasses(llvm::ArrayRef Decls) {
+std::map Classes;
+for (auto *D : Decls) {
+  auto Type = OpaqueType::fromCompletionResult(
+  ASTCtx(), CodeCompletionResult(D, CCP_Declaration));
+  Classes[Type->rawStr()].insert(D->getNameAsString());
+}
+return Classes;
+  }
+};
+
+TEST_F(ExpectedTypeConversionTest, BasicTypes) {
+  build(R"cpp(
+// ints.
+bool b;
+int i;
+unsigned int ui;
+long long ll;
+
+// floats.
+float f;
+double d;
+
+// pointers
+int* iptr;
+bool* bptr;
+
+// user-defined types.
+struct X {};
+X user_type;
+  )cpp");
+
+  const ValueDecl *Decls[] = {decl("b"),decl("i"),decl("ui"),
+  decl("ll"),   decl("f"),decl("d"),
+  decl("iptr"), decl("bptr"), decl("user_type")};
+  EXPECT_THAT(buildEquivClasses(Decls), ClassesAre({{"b", "i", "ui", "ll"},
+{"f", "d"},
+{"iptr"},
+{"bptr"},
+{"user_type"}}));
+}
+
+TEST_F(ExpectedTypeConversionTest, ReferencesDontMatter) {

[PATCH] D52273: [clangd] Initial implementation of expected types

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov marked 2 inline comments as done.
ilya-biryukov added inline comments.



Comment at: clangd/ExpectedTypes.cpp:12
+
+static llvm::Optional toEquivClass(ASTContext &Ctx, QualType T) {
+  if (T.isNull() || T->isDependentType())

sammccall wrote:
> returning QualType vs Type*? It seems we strip all qualifiers, seems clearest 
> for the return type to reflect that.
Done. That produces a bit more trouble at the callsites, so not sure if it's an 
improvement overall.



Comment at: clangd/ExpectedTypes.cpp:15
+return llvm::None;
+  T = T.getCanonicalType().getNonReferenceType().getUnqualifiedType();
+  if (T->isIntegerType() && !T->isEnumeralType())

sammccall wrote:
> Maybe we want Ctx.getUnqualifiedArrayType here or (more likely?) do 
> array-to-pointer decay?
Added array-to-pointer decays, they should improve ranking when assigning from 
an array to a pointer, which is nice.
Also added a FIXME that we should drop qualifiers from inner types of the 
pointers (since we do this for arrays). I think it's fine to leave it for the 
later improvements.



Comment at: clangd/ExpectedTypes.cpp:16
+  T = T.getCanonicalType().getNonReferenceType().getUnqualifiedType();
+  if (T->isIntegerType() && !T->isEnumeralType())
+return QualType(Ctx.IntTy);

sammccall wrote:
> wow, "enumeral" might be my favorite c++-made-up word, displacing "emplace"...
 ¯\_(ツ)_/¯



Comment at: clangd/ExpectedTypes.cpp:30
+return llvm::None;
+  QualType T = VD->getType().getCanonicalType().getNonReferenceType();
+  if (!T->isFunctionType())

sammccall wrote:
> nit: is canonicalization necessary here? you do it in toEquivClass
> (I guess dropping references is, for the function type check)
It was not important, removed it.



Comment at: clangd/ExpectedTypes.cpp:46
+  llvm::SmallString<128> Out;
+  if (index::generateUSRForType(T, Ctx, Out))
+return llvm::None;

sammccall wrote:
> I think ultimately we may want to replace this with a custom walker:
>  - we may want to ignore attributes (e.g. const) or bail out in some cases
>  - generateUSRForType may not have the exact semantics we want for other 
> random reasons
>  - we can do tricks with hash_combine to avoid actually building huge strings 
> we don't care about
> 
> not something for this patch, but maybe a FIXME?
> 
USRs actually seems like a pretty good fit here. I'm not sure dropping 
attributes for internal types would make a big difference in the scoring and 
not sure how big of a problem the strings are, would be nice to actually learn 
it's a problem (in memory consumption, memory alloc rates, etc) before changing 
this.

It's definitely possible to do that, of course, we have a room to change the 
encoding whenever we want, but would avoid adding a FIXME and committing to 
this approach in the initial patch.



Comment at: clangd/ExpectedTypes.h:10
+// A simplified model of C++ types that can be used to check whether they are
+// convertible between each other without looking at the ASTs.
+// Used for code completion ranking.

ilya-biryukov wrote:
> ioeric wrote:
> > We might want to formalize what "convertible" means here. E.g. does it 
> > cover conversion between base and derived class? Does it cover double <-> 
> > int conversion?
> I want to leave it vague for now. Convertible means whatever we think is good 
> for code completion ranking.
> Formalizing means we'll either dig into the C++ encoding or be imprecise. 
> 
> Happy to add the docs, but they'll probably get outdated on every change. 
> Reading the code is actually simpler to get what's going on at this point.
Added a clarification that we want "convertible for the purpose of code 
completion".



Comment at: clangd/ExpectedTypes.h:29
+namespace clangd {
+/// An opaque representation of a type that can be computed based on clang AST
+/// and compared for equality. The encoding is stable between different ASTs,

ioeric wrote:
> The name seems opaque ;) Why is it `opaque`? 
Removed the "opaque" from the comment, hopefully this causes less confusion.
The idea is that users shouldn't rely on this representation in any other way 
than comparing it for equality.



Comment at: clangd/ExpectedTypes.h:32
+/// this allows the representation to be stored in the index and compared with
+/// types coming from a different AST later.
+class OpaqueType {

sammccall wrote:
> Does this need to be a separate class rather than using `std::string`?
> There are echoes of `SymbolID` here, but there were some factors that don't 
> apply here:
>  - it was fixed-width
>  - memory layout was important as we stored lots of these in memory
>  - we hashed them a lot and wanted a specific hash function
> 
> I suspect at least initially producing a somewhat readable st

[PATCH] D49754: Add -m(no-)spe, and e500 CPU definitions and support to clang

[Justin Hibbits via Phabricator via cfe-commits]

jhibbits added a comment.

Hi Vit,

The register spilling bug is being addressed in https://reviews.llvm.org/D54409 
now.


Repository:
  rC Clang

https://reviews.llvm.org/D49754



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

r346967 - [AST][NFC] Various NFCs in StringLiteral

[Bruno Ricci via cfe-commits]

Author: brunoricci
Date: Thu Nov 15 08:42:14 2018
New Revision: 346967

URL: http://llvm.org/viewvc/llvm-project?rev=346967&view=rev
Log:
[AST][NFC] Various NFCs in StringLiteral

Factored out of D54166
([AST] Store the string data in StringLiteral in a trailing array of chars):

* For-range loops in containsNonAscii and containsNonAsciiOrNull.
* Comments and style fixes.
* int -> unsigned in mapCharByteWidth since TargetInfo::getCharWidth
  and friends return an unsigned, and StringLiteral manipulates and
  stores CharByteWidth as an unsigned.


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

Modified: cfe/trunk/include/clang/AST/Expr.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=346967&r1=346966&r2=346967&view=diff
==
--- cfe/trunk/include/clang/AST/Expr.h (original)
+++ cfe/trunk/include/clang/AST/Expr.h Thu Nov 15 08:42:14 2018
@@ -1552,14 +1552,15 @@ public:
 };
 
 /// StringLiteral - This represents a string literal expression, e.g. "foo"
-/// or L"bar" (wide strings).  The actual string is returned by getBytes()
-/// is NOT null-terminated, and the length of the string is determined by
-/// calling getByteLength().  The C type for a string is always a
-/// ConstantArrayType.  In C++, the char type is const qualified, in C it is
-/// not.
+/// or L"bar" (wide strings). The actual string data can be obtained with
+/// getBytes() and is NOT null-terminated. The length of the string data is
+/// determined by calling getByteLength().
+///
+/// The C type for a string is always a ConstantArrayType. In C++, the char
+/// type is const qualified, in C it is not.
 ///
 /// Note that strings in C can be formed by concatenation of multiple string
-/// literal pptokens in translation phase #6.  This keeps track of the 
locations
+/// literal pptokens in translation phase #6. This keeps track of the locations
 /// of each of these pieces.
 ///
 /// Strings in C can also be truncated and extended by assigning into arrays,
@@ -1569,13 +1570,7 @@ public:
 /// have type "char[2]".
 class StringLiteral : public Expr {
 public:
-  enum StringKind {
-Ascii,
-Wide,
-UTF8,
-UTF16,
-UTF32
-  };
+  enum StringKind { Ascii, Wide, UTF8, UTF16, UTF32 };
 
 private:
   friend class ASTStmtReader;
@@ -1596,7 +1591,8 @@ private:
 Expr(StringLiteralClass, Ty, VK_LValue, OK_Ordinary, false, false, false,
  false) {}
 
-  static int mapCharByteWidth(TargetInfo const &target,StringKind k);
+  /// Map a target and string kind to the appropriate character width.
+  static unsigned mapCharByteWidth(TargetInfo const &Target, StringKind SK);
 
 public:
   /// This is the "fully general" constructor that allows representation of
@@ -1666,17 +1662,15 @@ public:
   bool isPascal() const { return IsPascal; }
 
   bool containsNonAscii() const {
-StringRef Str = getString();
-for (unsigned i = 0, e = Str.size(); i != e; ++i)
-  if (!isASCII(Str[i]))
+for (auto c : getString())
+  if (!isASCII(c))
 return true;
 return false;
   }
 
   bool containsNonAsciiOrNull() const {
-StringRef Str = getString();
-for (unsigned i = 0, e = Str.size(); i != e; ++i)
-  if (!isASCII(Str[i]) || !Str[i])
+for (auto c : getString())
+  if (!isASCII(c) || !c)
 return true;
 return false;
   }

Modified: cfe/trunk/lib/AST/Expr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Expr.cpp?rev=346967&r1=346966&r2=346967&view=diff
==
--- cfe/trunk/lib/AST/Expr.cpp (original)
+++ cfe/trunk/lib/AST/Expr.cpp Thu Nov 15 08:42:14 2018
@@ -887,27 +887,28 @@ double FloatingLiteral::getValueAsApprox
   return V.convertToDouble();
 }
 
-int StringLiteral::mapCharByteWidth(TargetInfo const &target,StringKind k) {
-  int CharByteWidth = 0;
-  switch(k) {
-case Ascii:
-case UTF8:
-  CharByteWidth = target.getCharWidth();
-  break;
-case Wide:
-  CharByteWidth = target.getWCharWidth();
-  break;
-case UTF16:
-  CharByteWidth = target.getChar16Width();
-  break;
-case UTF32:
-  CharByteWidth = target.getChar32Width();
-  break;
+unsigned StringLiteral::mapCharByteWidth(TargetInfo const &Target,
+ StringKind SK) {
+  unsigned CharByteWidth = 0;
+  switch (SK) {
+  case Ascii:
+  case UTF8:
+CharByteWidth = Target.getCharWidth();
+break;
+  case Wide:
+CharByteWidth = Target.getWCharWidth();
+break;
+  case UTF16:
+CharByteWidth = Target.getChar16Width();
+break;
+  case UTF32:
+CharByteWidth = Target.getChar32Width();
+break;
   }
   assert((CharByteWidth & 7) == 0 && "Assumes character size is byte 
multiple");
   CharByteWidth /= 8;
-  assert((CharByteWidth==1 || CharByteWidth==2 || CharByteWidth==4)
- && "character byte widt

[PATCH] D54489: Implement -frecord-command-line (-frecord-gcc-switches)

[Scott Linder via Phabricator via cfe-commits]

scott.linder added a comment.

Looking a bit further, it seems some other object-file formats have conventions 
for naming (e.g. Mach-O sections are `_snake_case`) and so `.LLVM.command.line` 
may not end up being what we want universally.

Additionally it seems things like `.ident` are not supported by all object-file 
formats in LLVM; using Mach-O as an example again, it uses the default of "no 
.ident support", and I don't see the ident in the .S or .o


https://reviews.llvm.org/D54489



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

[PATCH] D53830: [clang-tidy]: Abseil: new check 'abseil-upgrade-duration-conversions'

[Alex Strelnikov via Phabricator via cfe-commits]

astrelni updated this revision to Diff 174218.
astrelni marked an inline comment as done.
astrelni added a comment.

Fix to use `hasAnyName` everywhere


https://reviews.llvm.org/D53830

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp
  clang-tidy/abseil/UpgradeDurationConversionsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-upgrade-duration-conversions.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-upgrade-duration-conversions.cpp

Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -12,6 +12,7 @@
abseil-redundant-strcat-calls
abseil-string-find-startswith
abseil-str-cat-append
+   abseil-upgrade-duration-conversions
android-cloexec-accept
android-cloexec-accept4
android-cloexec-creat
Index: docs/clang-tidy/checks/abseil-upgrade-duration-conversions.rst
===
--- docs/clang-tidy/checks/abseil-upgrade-duration-conversions.rst
+++ docs/clang-tidy/checks/abseil-upgrade-duration-conversions.rst
@@ -0,0 +1,43 @@
+.. title:: clang-tidy - abseil-upgrade-duration-conversions
+
+abseil-upgrade-duration-conversions
+===
+
+Finds calls to ``absl::Duration`` arithmetic operators and factories whose
+argument needs an explicit cast to continue compiling after upcoming API
+changes.
+
+The operators ``*=``, ``/=``, ``*``, and ``/`` for ``absl::Duration`` currently
+accept an argument of class type that is convertible to an arithmetic type. Such
+a call currently converts the value to an ``int64_t``, even in a case such as
+``std::atomic`` that would result in lossy conversion.
+
+Additionally, the ``absl::Duration`` factory functions (``absl::Hours``,
+``absl::Minutes``, etc) currently accept an ``int64_t`` or a floating-point
+type. Similar to the arithmetic operators, calls with an argument of class type
+that is convertible to an arithmetic type go through the ``int64_t`` path.
+
+These operators and factories will be changed to only accept arithmetic types to
+prevent unintended behavior. After these changes are released, passing an
+argument of class type will no longer compile, even if the type is implicitly
+convertible to an arithmetic type.
+
+Here are example fixes created by this check:
+
+.. code-block:: c++
+
+  std::atomic a;
+  absl::Duration d = absl::Milliseconds(a);
+  d *= a;
+
+becomes
+
+.. code-block:: c++
+
+  std::atomic a;
+  absl::Duration d = absl::Milliseconds(static_cast(a));
+  d *= static_cast(a);
+
+Note that this check always adds a cast to ``int64_t`` in order to preserve the
+current behavior of user code. It is possible that this uncovers unintended
+behavior due to types implicitly convertible to a floating-point type.
Index: test/clang-tidy/abseil-upgrade-duration-conversions.cpp
===
--- test/clang-tidy/abseil-upgrade-duration-conversions.cpp
+++ test/clang-tidy/abseil-upgrade-duration-conversions.cpp
@@ -0,0 +1,451 @@
+// RUN: %check_clang_tidy %s abseil-upgrade-duration-conversions %t
+
+using int64_t = long long;
+
+// Partial implementation of relevant APIs from
+// https://github.com/abseil/abseil-cpp/blob/master/absl/time/time.h
+namespace absl {
+
+class Duration {
+public:
+  Duration &operator*=(int64_t r);
+  Duration &operator*=(float r);
+  Duration &operator*=(double r);
+  template  Duration &operator*=(T r);
+
+  Duration &operator/=(int64_t r);
+  Duration &operator/=(float r);
+  Duration &operator/=(double r);
+  template  Duration &operator/=(T r);
+};
+
+template  Duration operator*(Duration lhs, T rhs);
+template  Duration operator*(T lhs, Duration rhs);
+template  Duration operator/(Duration lhs, T rhs);
+
+constexpr Duration Nanoseconds(int64_t n);
+constexpr Duration Microseconds(int64_t n);
+constexpr Duration Milliseconds(int64_t n);
+constexpr Duration Seconds(int64_t n);
+constexpr Duration Minutes(int64_t n);
+constexpr Duration Hours(int64_t n);
+
+template  struct EnableIfFloatImpl {};
+template <> struct EnableIfFloatImpl { typedef int Type; };
+template <> struct EnableIfFloatImpl { typedef int Type; };
+template <> struct EnableIfFloatImpl { typedef int Type; };
+template  using EnableIfFloat = typename EnableIfFloatImpl::Type;
+
+template  = 0> Duration Nanoseconds(T n);
+template  = 0> Duration Microseconds(T n);
+template  = 0> Duration Milliseconds(T n);
+template  = 0> Duration Seconds(T n);
+template  = 0> Duration Minutes(T n);
+template  = 0> Duration Hours(T n);
+
+} // namespace absl
+
+template  struct ConvertibleTo {
+  operator T() const;
+};
+
+template 
+ConvertibleTo operator+(ConvertibleTo, ConvertibleTo);
+
+template 
+ConvertibleTo operator*(ConvertibleTo, ConvertibleTo);
+
+void arithmeticOperatorBas

[PATCH] D53830: [clang-tidy]: Abseil: new check 'abseil-upgrade-duration-conversions'

[Alex Strelnikov via Phabricator via cfe-commits]

astrelni updated this revision to Diff 174226.
astrelni added a comment.

Fix incorrect uploaded diff.


https://reviews.llvm.org/D53830

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp
  clang-tidy/abseil/UpgradeDurationConversionsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-upgrade-duration-conversions.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-upgrade-duration-conversions.cpp

Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -12,6 +12,7 @@
abseil-redundant-strcat-calls
abseil-string-find-startswith
abseil-str-cat-append
+   abseil-upgrade-duration-conversions
android-cloexec-accept
android-cloexec-accept4
android-cloexec-creat
Index: docs/clang-tidy/checks/abseil-upgrade-duration-conversions.rst
===
--- docs/clang-tidy/checks/abseil-upgrade-duration-conversions.rst
+++ docs/clang-tidy/checks/abseil-upgrade-duration-conversions.rst
@@ -0,0 +1,43 @@
+.. title:: clang-tidy - abseil-upgrade-duration-conversions
+
+abseil-upgrade-duration-conversions
+===
+
+Finds calls to ``absl::Duration`` arithmetic operators and factories whose
+argument needs an explicit cast to continue compiling after upcoming API
+changes.
+
+The operators ``*=``, ``/=``, ``*``, and ``/`` for ``absl::Duration`` currently
+accept an argument of class type that is convertible to an arithmetic type. Such
+a call currently converts the value to an ``int64_t``, even in a case such as
+``std::atomic`` that would result in lossy conversion.
+
+Additionally, the ``absl::Duration`` factory functions (``absl::Hours``,
+``absl::Minutes``, etc) currently accept an ``int64_t`` or a floating-point
+type. Similar to the arithmetic operators, calls with an argument of class type
+that is convertible to an arithmetic type go through the ``int64_t`` path.
+
+These operators and factories will be changed to only accept arithmetic types to
+prevent unintended behavior. After these changes are released, passing an
+argument of class type will no longer compile, even if the type is implicitly
+convertible to an arithmetic type.
+
+Here are example fixes created by this check:
+
+.. code-block:: c++
+
+  std::atomic a;
+  absl::Duration d = absl::Milliseconds(a);
+  d *= a;
+
+becomes
+
+.. code-block:: c++
+
+  std::atomic a;
+  absl::Duration d = absl::Milliseconds(static_cast(a));
+  d *= static_cast(a);
+
+Note that this check always adds a cast to ``int64_t`` in order to preserve the
+current behavior of user code. It is possible that this uncovers unintended
+behavior due to types implicitly convertible to a floating-point type.
Index: test/clang-tidy/abseil-upgrade-duration-conversions.cpp
===
--- test/clang-tidy/abseil-upgrade-duration-conversions.cpp
+++ test/clang-tidy/abseil-upgrade-duration-conversions.cpp
@@ -0,0 +1,451 @@
+// RUN: %check_clang_tidy %s abseil-upgrade-duration-conversions %t
+
+using int64_t = long long;
+
+// Partial implementation of relevant APIs from
+// https://github.com/abseil/abseil-cpp/blob/master/absl/time/time.h
+namespace absl {
+
+class Duration {
+public:
+  Duration &operator*=(int64_t r);
+  Duration &operator*=(float r);
+  Duration &operator*=(double r);
+  template  Duration &operator*=(T r);
+
+  Duration &operator/=(int64_t r);
+  Duration &operator/=(float r);
+  Duration &operator/=(double r);
+  template  Duration &operator/=(T r);
+};
+
+template  Duration operator*(Duration lhs, T rhs);
+template  Duration operator*(T lhs, Duration rhs);
+template  Duration operator/(Duration lhs, T rhs);
+
+constexpr Duration Nanoseconds(int64_t n);
+constexpr Duration Microseconds(int64_t n);
+constexpr Duration Milliseconds(int64_t n);
+constexpr Duration Seconds(int64_t n);
+constexpr Duration Minutes(int64_t n);
+constexpr Duration Hours(int64_t n);
+
+template  struct EnableIfFloatImpl {};
+template <> struct EnableIfFloatImpl { typedef int Type; };
+template <> struct EnableIfFloatImpl { typedef int Type; };
+template <> struct EnableIfFloatImpl { typedef int Type; };
+template  using EnableIfFloat = typename EnableIfFloatImpl::Type;
+
+template  = 0> Duration Nanoseconds(T n);
+template  = 0> Duration Microseconds(T n);
+template  = 0> Duration Milliseconds(T n);
+template  = 0> Duration Seconds(T n);
+template  = 0> Duration Minutes(T n);
+template  = 0> Duration Hours(T n);
+
+} // namespace absl
+
+template  struct ConvertibleTo {
+  operator T() const;
+};
+
+template 
+ConvertibleTo operator+(ConvertibleTo, ConvertibleTo);
+
+template 
+ConvertibleTo operator*(ConvertibleTo, ConvertibleTo);
+
+void arithmeticOperatorBasicPositive() {
+  absl::Duration d;
+  d *= Conve

r346969 - [AST] Store the string data in StringLiteral in a trailing array of chars

[Bruno Ricci via cfe-commits]

Author: brunoricci
Date: Thu Nov 15 09:31:16 2018
New Revision: 346969

URL: http://llvm.org/viewvc/llvm-project?rev=346969&view=rev
Log:
[AST] Store the string data in StringLiteral in a trailing array of chars

Use the newly available space in the bit-fields of Stmt and store the
string data in a trailing array of chars after the trailing array
of SourceLocation. This cuts the size of StringLiteral by 2 pointers.

Also refactor slightly StringLiteral::Create and StringLiteral::CreateEmpty
so that StringLiteral::Create is just responsible for the allocation, and the
constructor is responsible for doing all the initialization. This match what
is done for the other classes in general.

This patch should have no other functional changes apart from this.

A concern was raised during review about the interaction between
this patch and serialization abbreviations. I believe however that
there is currently no abbreviation defined for StringLiteral.
The only statements/expressions which have abbreviations are currently
DeclRefExpr, IntegerLiteral, CharacterLiteral and ImplicitCastExpr.

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

Reviewed By: dblaikie, rjmccall


Modified:
cfe/trunk/include/clang/AST/Expr.h
cfe/trunk/include/clang/AST/Stmt.h
cfe/trunk/lib/AST/Expr.cpp
cfe/trunk/lib/Serialization/ASTReaderStmt.cpp
cfe/trunk/lib/Serialization/ASTWriterStmt.cpp

Modified: cfe/trunk/include/clang/AST/Expr.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=346969&r1=346968&r2=346969&view=diff
==
--- cfe/trunk/include/clang/AST/Expr.h (original)
+++ cfe/trunk/include/clang/AST/Expr.h Thu Nov 15 09:31:16 2018
@@ -1568,98 +1568,131 @@ public:
 ///   char X[2] = "foobar";
 /// In this case, getByteLength() will return 6, but the string literal will
 /// have type "char[2]".
-class StringLiteral : public Expr {
+class StringLiteral final
+: public Expr,
+  private llvm::TrailingObjects {
+  friend class ASTStmtReader;
+  friend TrailingObjects;
+
+  /// StringLiteral is followed by several trailing objects. They are in order:
+  ///
+  /// * A single unsigned storing the length in characters of this string. The
+  ///   length in bytes is this length times the width of a single character.
+  ///   Always present and stored as a trailing objects because storing it in
+  ///   StringLiteral would increase the size of StringLiteral by sizeof(void 
*)
+  ///   due to alignment requirements. If you add some data to StringLiteral,
+  ///   consider moving it inside StringLiteral.
+  ///
+  /// * An array of getNumConcatenated() SourceLocation, one for each of the
+  ///   token this string is made of.
+  ///
+  /// * An array of getByteLength() char used to store the string data.
+
 public:
   enum StringKind { Ascii, Wide, UTF8, UTF16, UTF32 };
 
 private:
-  friend class ASTStmtReader;
+  unsigned numTrailingObjects(OverloadToken) const { return 1; }
+  unsigned numTrailingObjects(OverloadToken) const {
+return getNumConcatenated();
+  }
+
+  unsigned numTrailingObjects(OverloadToken) const {
+return getByteLength();
+  }
 
-  union {
-const char *asChar;
-const uint16_t *asUInt16;
-const uint32_t *asUInt32;
-  } StrData;
-  unsigned Length;
-  unsigned CharByteWidth : 4;
-  unsigned Kind : 3;
-  unsigned IsPascal : 1;
-  unsigned NumConcatenated;
-  SourceLocation TokLocs[1];
-
-  StringLiteral(QualType Ty) :
-Expr(StringLiteralClass, Ty, VK_LValue, OK_Ordinary, false, false, false,
- false) {}
+  char *getStrDataAsChar() { return getTrailingObjects(); }
+  const char *getStrDataAsChar() const { return getTrailingObjects(); }
+
+  const uint16_t *getStrDataAsUInt16() const {
+return reinterpret_cast(getTrailingObjects());
+  }
+
+  const uint32_t *getStrDataAsUInt32() const {
+return reinterpret_cast(getTrailingObjects());
+  }
+
+  /// Build a string literal.
+  StringLiteral(const ASTContext &Ctx, StringRef Str, StringKind Kind,
+bool Pascal, QualType Ty, const SourceLocation *Loc,
+unsigned NumConcatenated);
+
+  /// Build an empty string literal.
+  StringLiteral(EmptyShell Empty, unsigned NumConcatenated, unsigned Length,
+unsigned CharByteWidth);
 
   /// Map a target and string kind to the appropriate character width.
   static unsigned mapCharByteWidth(TargetInfo const &Target, StringKind SK);
 
+  /// Set one of the string literal token.
+  void setStrTokenLoc(unsigned TokNum, SourceLocation L) {
+assert(TokNum < getNumConcatenated() && "Invalid tok number");
+getTrailingObjects()[TokNum] = L;
+  }
+
 public:
   /// This is the "fully general" constructor that allows representation of
   /// strings formed from multiple concatenated tokens.
-  static StringLiteral *Create(const ASTContext &C, StringRef Str,
+  static StringLiteral *Create(const ASTContext &Ctx, StringRef Str,
 

[PATCH] D54166: [AST] Store the string data in StringLiteral in a trailing array of chars

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL346969: [AST] Store the string data in StringLiteral in a 
trailing array of chars (authored by brunoricci, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D54166?vs=174032&id=174228#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D54166

Files:
  cfe/trunk/include/clang/AST/Expr.h
  cfe/trunk/include/clang/AST/Stmt.h
  cfe/trunk/lib/AST/Expr.cpp
  cfe/trunk/lib/Serialization/ASTReaderStmt.cpp
  cfe/trunk/lib/Serialization/ASTWriterStmt.cpp

Index: cfe/trunk/include/clang/AST/Expr.h
===
--- cfe/trunk/include/clang/AST/Expr.h
+++ cfe/trunk/include/clang/AST/Expr.h
@@ -1568,98 +1568,131 @@
 ///   char X[2] = "foobar";
 /// In this case, getByteLength() will return 6, but the string literal will
 /// have type "char[2]".
-class StringLiteral : public Expr {
+class StringLiteral final
+: public Expr,
+  private llvm::TrailingObjects {
+  friend class ASTStmtReader;
+  friend TrailingObjects;
+
+  /// StringLiteral is followed by several trailing objects. They are in order:
+  ///
+  /// * A single unsigned storing the length in characters of this string. The
+  ///   length in bytes is this length times the width of a single character.
+  ///   Always present and stored as a trailing objects because storing it in
+  ///   StringLiteral would increase the size of StringLiteral by sizeof(void *)
+  ///   due to alignment requirements. If you add some data to StringLiteral,
+  ///   consider moving it inside StringLiteral.
+  ///
+  /// * An array of getNumConcatenated() SourceLocation, one for each of the
+  ///   token this string is made of.
+  ///
+  /// * An array of getByteLength() char used to store the string data.
+
 public:
   enum StringKind { Ascii, Wide, UTF8, UTF16, UTF32 };
 
 private:
-  friend class ASTStmtReader;
+  unsigned numTrailingObjects(OverloadToken) const { return 1; }
+  unsigned numTrailingObjects(OverloadToken) const {
+return getNumConcatenated();
+  }
 
-  union {
-const char *asChar;
-const uint16_t *asUInt16;
-const uint32_t *asUInt32;
-  } StrData;
-  unsigned Length;
-  unsigned CharByteWidth : 4;
-  unsigned Kind : 3;
-  unsigned IsPascal : 1;
-  unsigned NumConcatenated;
-  SourceLocation TokLocs[1];
-
-  StringLiteral(QualType Ty) :
-Expr(StringLiteralClass, Ty, VK_LValue, OK_Ordinary, false, false, false,
- false) {}
+  unsigned numTrailingObjects(OverloadToken) const {
+return getByteLength();
+  }
+
+  char *getStrDataAsChar() { return getTrailingObjects(); }
+  const char *getStrDataAsChar() const { return getTrailingObjects(); }
+
+  const uint16_t *getStrDataAsUInt16() const {
+return reinterpret_cast(getTrailingObjects());
+  }
+
+  const uint32_t *getStrDataAsUInt32() const {
+return reinterpret_cast(getTrailingObjects());
+  }
+
+  /// Build a string literal.
+  StringLiteral(const ASTContext &Ctx, StringRef Str, StringKind Kind,
+bool Pascal, QualType Ty, const SourceLocation *Loc,
+unsigned NumConcatenated);
+
+  /// Build an empty string literal.
+  StringLiteral(EmptyShell Empty, unsigned NumConcatenated, unsigned Length,
+unsigned CharByteWidth);
 
   /// Map a target and string kind to the appropriate character width.
   static unsigned mapCharByteWidth(TargetInfo const &Target, StringKind SK);
 
+  /// Set one of the string literal token.
+  void setStrTokenLoc(unsigned TokNum, SourceLocation L) {
+assert(TokNum < getNumConcatenated() && "Invalid tok number");
+getTrailingObjects()[TokNum] = L;
+  }
+
 public:
   /// This is the "fully general" constructor that allows representation of
   /// strings formed from multiple concatenated tokens.
-  static StringLiteral *Create(const ASTContext &C, StringRef Str,
+  static StringLiteral *Create(const ASTContext &Ctx, StringRef Str,
StringKind Kind, bool Pascal, QualType Ty,
-   const SourceLocation *Loc, unsigned NumStrs);
+   const SourceLocation *Loc,
+   unsigned NumConcatenated);
 
   /// Simple constructor for string literals made from one token.
-  static StringLiteral *Create(const ASTContext &C, StringRef Str,
+  static StringLiteral *Create(const ASTContext &Ctx, StringRef Str,
StringKind Kind, bool Pascal, QualType Ty,
SourceLocation Loc) {
-return Create(C, Str, Kind, Pascal, Ty, &Loc, 1);
+return Create(Ctx, Str, Kind, Pascal, Ty, &Loc, 1);
   }
 
   /// Construct an empty string literal.
-  static StringLiteral *CreateEmpty(const ASTContext &C, unsigned NumStrs);
+  static StringLiteral *CreateEmpty(const ASTContext &Ctx,
+unsigned NumConcatenated, unsigned Le

[PATCH] D54438: [analyzer][WIP] Reimplement dependencies between checkers

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

Szelethus added a comment.
Herald added a subscriber: gamesh411.

In https://reviews.llvm.org/D54438#1297602, @NoQ wrote:

> Hmm, makes sense. Maybe `static bool BlahBlahChecker::isApplicable(const 
> LangOpts &LO)` or something like that.
>
> Btw, maybe instead of default constructor and `->setup(Args)` method we could 
> stuff all of the `setup(Args)`'s arguments into the one-and-only constructor 
> for the checker?


Neat. Make all checkers have only a single constructor, expecting a `const 
CheckerManager &`! Since `getCurrentName` and the like will be thing of the 
past, for once, maybe we could also initialize checker options in constructors 
too. Hopefully.


Repository:
  rC Clang

https://reviews.llvm.org/D54438



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

[PATCH] D54428: [clangd] XPC transport layer, framework, test-client

[Jan Korous via Phabricator via cfe-commits]

jkorous planned changes to this revision.
jkorous added a comment.

In https://reviews.llvm.org/D54428#1297147, @sammccall wrote:

> A question about the high-level build target setup (I don't know much about 
> XPC services/frameworks, bear with me...):
>
> This is set up so that the clangd binary (ClangdMain) can run unmodified as 
> an XPC service, all flags and options are still respected etc.
>  At the same time, the framework plist seems like it is designed to invoke 
> the clangd binary in a very specific configuration (no flags will be plumbed 
> through).
>
> Is it actually important that the `clangd` binary itself can be used with 
> XPC, rather than defining another binary that spawns a 
> `ClangdServer+XPCTransport` with the right config? If you strip out all of 
> `ClangdMain` that's not related to flag parsing and options, there's almost 
> nothing left...


I don't really expect you to be familiar with XPC - feel free to ask about 
anything and I'll do my best explaining.

It's more like that configuration isn't implemented in this initial patch but 
as we can't use command line options (launchd doesn't pass these to XPC 
service) we'll have to duplicate these as environment variables. We discusse 
internally and we prefer to have just single binary as that would make 
integration and testing easier for us.




Comment at: tool/ClangdMain.cpp:30
+#ifdef CLANGD_BUILD_XPC
+#include "xpc/XPCTransport.h"
+#endif

sammccall wrote:
> WDYT about putting `newXPCTransport()` in `Transport.h` behind an ifdef?
> 
> That will be consistent with JSON, allow the `XPCTransport.h` to be removed 
> entirely, and remove one #ifdef here.
> 
> I find the #ifdefs easier to understand if they surround functional code, 
> rather than #includes - might be just me.
Ok, sounds reasonable.



Comment at: tool/ClangdMain.cpp:328
+#ifdef CLANGD_BUILD_XPC
+if (getenv("CLANGD_AS_XPC_SERVICE"))
+  return newXPCransport();

sammccall wrote:
> I'd consider putting this outside the #ifdef, so we can print an error and 
> exit if it's requested but not built.
> 
> In fact, can this just be a regular flag instead? `-transport={xpc|json}`
Unfortunately it's not possible to have launchd pass flags when spawning XPC 
services (otherwise it would be my first choice too).



Comment at: tool/ClangdMain.cpp:329
+if (getenv("CLANGD_AS_XPC_SERVICE"))
+  return newXPCransport();
+#endif

sammccall wrote:
> no support for `-input-mirror` or pretty-printing in the logs - deliberate?
I was thinking this could be added later.



Comment at: xpc/CMakeLists.txt:21
+
+add_clang_library(clangdXpcJsonConversions
+  Conversion.cpp

sammccall wrote:
> why is conversions a separate library from transport?
> 
> No problem with fine-grained targets per se, but it's inconsistent with much 
> of the rest of clang-tools-extra.
Guilty of YAGNI violation here - was thinking about eventual use by XPC 
clients. I'll stick to consistency for now and would separate it if needed.



Comment at: xpc/Conversion.cpp:16
+
+using namespace clang;
+using namespace clangd;

sammccall wrote:
> nit:
> ```
> using namespace llvm;
> namespace clang {
> namespace clangd {
> ```
> (we're finally consistent about this)
Ok. Thanks.



Comment at: xpc/Conversion.cpp:22
+xpc_object_t clangd::jsonToXpc(const json::Value &json) {
+  const char *const key = "LSP";
+  std::string payload_string;

sammccall wrote:
> Key, PayloadString, etc
Thanks. Somehow I was always working on projects with different naming 
conventions and still struggle with this.



Comment at: xpc/Conversion.cpp:23
+  const char *const key = "LSP";
+  std::string payload_string;
+  raw_string_ostream payload_stream(payload_string);

sammccall wrote:
> nit: you can #include ScopedPrinter, and then this is just 
> llvm::to_string(json)
I'll take a look at ScopedPrinter. Thanks.



Comment at: xpc/Conversion.cpp:28
+  xpc_object_t payload_obj = xpc_string_create(payload_string.c_str());
+  return xpc_dictionary_create(&key, &payload_obj, 1);
+}

sammccall wrote:
> ah, this encoding is a little sad vs the "native" encoding of object -> 
> xpc_dictionary, array -> xpc_array etc which looked promising...
> 
> Totally your call, but out of curiosity - why the change?
I know. At first we didn't even think of anything else than 1:1 mapping between 
JSON and XPC dictionary as it's just natural. But later we learned about 
performance issues with construction of XPC dictionaries with lots of elements 
which would be a problem for code completion.



Comment at: xpc/Conversion.h:19
+
+xpc_object_t jsonToXpc(const llvm::json::Value &json);
+llvm::json::Value xpcToJson(const xpc_object_t &json);

sammccall wrote:
> param nam

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

In https://reviews.llvm.org/D53738#1299557, @ebevhan wrote:

> I'd be interested in seeing tests for two saturating unsigned _Fract with and 
> without padding.
>
> If the padding case emits a uadd_sat, that seems wrong; uadd_sat doesn't 
> saturate on the padding bit, but will saturate the whole number, which can 
> result in invalid representation both with or without saturation taking 
> effect.
>
> Common semantics for unsigned types with padding might need a bit of trickery 
> to get it to do the right thing.


Good case to bring up. For addition, I think we just need to add an extra 
condition that checks for unsigned padding in the result. Added this test also.




Comment at: clang/test/Frontend/fixed_point_add.c:36
+
+  // To smaller scale and same width
+  // CHECK:  [[SA:%[0-9]+]] = load i16, i16* %sa, align 2

ebevhan wrote:
> What do these comments refer to? The scale is the same for both operands here.
This was meant to be a addition with a short _Accum and _Fract, but 
accidentally typed this instead. Added that test after this one.


Repository:
  rC Clang

https://reviews.llvm.org/D53738



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

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

[Leonard Chan via Phabricator via cfe-commits]

leonardchan updated this revision to Diff 174235.
leonardchan marked 13 inline comments as done.

Repository:
  rC Clang

https://reviews.llvm.org/D53738

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/FixedPoint.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Basic/FixedPoint.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Frontend/fixed_point_add.c
  clang/test/Frontend/fixed_point_add_ast.c
  clang/test/Frontend/fixed_point_conversions.c

Index: clang/test/Frontend/fixed_point_conversions.c
===
--- clang/test/Frontend/fixed_point_conversions.c
+++ clang/test/Frontend/fixed_point_conversions.c
@@ -214,19 +214,17 @@
   // Only get overflow checking if signed fract to unsigned accum
   sat_ua = sat_sf;
   // DEFAULT:  [[FRACT:%[0-9a-z]+]] = load i8, i8* %sat_sf, align 1
-  // DEFAULT-NEXT: [[FRACT_EXT:%[0-9a-z]+]] = sext i8 [[FRACT]] to i17
-  // DEFAULT-NEXT: [[ACCUM:%[0-9a-z]+]] = shl i17 [[FRACT_EXT]], 9
-  // DEFAULT-NEXT: [[IS_NEG:%[0-9a-z]+]] = icmp slt i17 [[ACCUM]], 0
-  // DEFAULT-NEXT: [[RESULT:%[0-9a-z]+]] = select i1 [[IS_NEG]], i17 0, i17 [[ACCUM]]
-  // DEFAULT-NEXT: [[RESULT_EXT:%[0-9a-z]+]] = sext i17 [[RESULT]] to i32
-  // DEFAULT-NEXT: store i32 [[RESULT_EXT]], i32* %sat_ua, align 4
+  // DEFAULT-NEXT: [[FRACT_EXT:%[0-9a-z]+]] = sext i8 [[FRACT]] to i32
+  // DEFAULT-NEXT: [[ACCUM:%[0-9a-z]+]] = shl i32 [[FRACT_EXT]], 9
+  // DEFAULT-NEXT: [[IS_NEG:%[0-9a-z]+]] = icmp slt i32 [[ACCUM]], 0
+  // DEFAULT-NEXT: [[RESULT:%[0-9a-z]+]] = select i1 [[IS_NEG]], i32 0, i32 [[ACCUM]]
+  // DEFAULT-NEXT: store i32 [[RESULT]], i32* %sat_ua, align 4
   // SAME:  [[FRACT:%[0-9a-z]+]] = load i8, i8* %sat_sf, align 1
-  // SAME-NEXT: [[FRACT_EXT:%[0-9a-z]+]] = sext i8 [[FRACT]] to i16
-  // SAME-NEXT: [[ACCUM:%[0-9a-z]+]] = shl i16 [[FRACT_EXT]], 8
-  // SAME-NEXT: [[IS_NEG:%[0-9a-z]+]] = icmp slt i16 [[ACCUM]], 0
-  // SAME-NEXT: [[RESULT:%[0-9a-z]+]] = select i1 [[IS_NEG]], i16 0, i16 [[ACCUM]]
-  // SAME-NEXT: [[RESULT_EXT:%[0-9a-z]+]] = sext i16 [[RESULT]] to i32
-  // SAME-NEXT: store i32 [[RESULT_EXT]], i32* %sat_ua, align 4
+  // SAME-NEXT: [[FRACT_EXT:%[0-9a-z]+]] = sext i8 [[FRACT]] to i32
+  // SAME-NEXT: [[ACCUM:%[0-9a-z]+]] = shl i32 [[FRACT_EXT]], 8
+  // SAME-NEXT: [[IS_NEG:%[0-9a-z]+]] = icmp slt i32 [[ACCUM]], 0
+  // SAME-NEXT: [[RESULT:%[0-9a-z]+]] = select i1 [[IS_NEG]], i32 0, i32 [[ACCUM]]
+  // SAME-NEXT: store i32 [[RESULT]], i32* %sat_ua, align 4
 }
 
 void TestFixedPointCastBetFractAccum() {
Index: clang/test/Frontend/fixed_point_add_ast.c
===
--- /dev/null
+++ clang/test/Frontend/fixed_point_add_ast.c
@@ -0,0 +1,337 @@
+// RUN: %clang_cc1 -x c -ffixed-point -ast-dump %s | FileCheck %s --strict-whitespace
+
+void SignedAdditions() {
+  // CHECK-LABEL: SignedAdditions
+  short _Accum sa;
+  _Accum a;
+  long _Accum la;
+  unsigned short _Accum usa;
+  unsigned _Accum ua;
+  unsigned long _Accum ula;
+
+  short _Fract sf;
+  _Fract f;
+  long _Fract lf;
+  unsigned short _Fract usf;
+  unsigned _Fract uf;
+  unsigned long _Fract ulf;
+
+  // CHECK-NOT:  FixedPointCast
+  // CHECK:  `-BinaryOperator {{.*}} 'short _Accum' '+'
+  // CHECK-NEXT:   |-ImplicitCastExpr {{.*}} 'short _Accum' 
+  // CHECK-NEXT:   | `-DeclRefExpr {{.*}} 'short _Accum' {{.*}} 'sa' 'short _Accum'
+  // CHECK-NEXT:   `-ImplicitCastExpr {{.*}} 'short _Accum' 
+  // CHECK-NEXT: `-DeclRefExpr {{.*}} 'short _Accum' {{.*}} 'sa' 'short _Accum'
+  sa = sa + sa;
+
+  // CHECK-NOT:  FixedPointCast
+  // CHECK:  `-BinaryOperator {{.*}} '_Accum' '+'
+  // CHECK-NEXT:   |-ImplicitCastExpr {{.*}} 'short _Accum' 
+  // CHECK-NEXT:   | `-DeclRefExpr {{.*}} 'short _Accum' {{.*}} 'sa' 'short _Accum'
+  // CHECK-NEXT:   `-ImplicitCastExpr {{.*}} '_Accum' 
+  // CHECK-NEXT: `-DeclRefExpr {{.*}} '_Accum' {{.*}} 'a' '_Accum'
+  a = sa + a;
+
+  // CHECK-NOT:  FixedPointCast
+  // CHECK:  `-BinaryOperator {{.*}} 'long _Accum' '+'
+  // CHECK-NEXT:   |-ImplicitCastExpr {{.*}} 'short _Accum' 
+  // CHECK-NEXT:   | `-DeclRefExpr {{.*}} 'short _Accum' {{.*}} 'sa' 'short _Accum'
+  // CHECK-NEXT:   `-ImplicitCastExpr {{.*}} 'long _Accum' 
+  // CHECK-NEXT: `-DeclRefExpr {{.*}} 'long _Accum' {{.*}} 'la' 'long _Accum'
+  la = sa + la;
+
+  // CHECK-NOT:  FixedPointCast
+  // CHECK:   `-ImplicitCastExpr {{.*}} 'long _Accum' 
+  // CHECK-NEXT:`-BinaryOperator {{.*}} '_Accum' '+'
+  // CHECK-NEXT:  |-ImplicitCastExpr {{.*}} 'short _Accum' 
+  // CHECK-NEXT:  | `-DeclRefExpr {{.*}} 'short _Accum' {{.*}} 'sa' 'short _Accum'
+  // CHECK-NEXT:  `-ImplicitCastExpr {{.*}} '_Accum' 
+  // CHECK-NEXT:`-DeclRefExpr {{.*}} '_Accum' {{.*}} 'a' '_Accum'
+  la = sa + a;
+
+  // CHECK-NOT:  FixedPointCast
+  // CHECK:  `-BinaryOperator {{.*}} 'short _Accum' '+'
+  // CHECK-NE

[PATCH] D40988: Clang-format: add finer-grained options for putting all arguments on one line

[Russell McClellan via Phabricator via cfe-commits]

russellmcc added a comment.

bump!  Thanks for your consideration.


https://reviews.llvm.org/D40988



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

[PATCH] D54324: [AST] Store the value of CharacterLiteral in the bit-fields of Stmt if possible

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added inline comments.



Comment at: include/clang/AST/Expr.h:1407
 public:
-  enum CharacterKind {
-Ascii,
-Wide,
-UTF8,
-UTF16,
-UTF32
-  };
+  enum CharacterKind { Ascii, Wide, UTF8, UTF16, UTF32 };
 

riccibruno wrote:
> shafik wrote:
> > Minor comment, does it make sense to covert this to a scoped enum since it 
> > looks like it is being strictly used as a set of values.
> Does it really add anything ?
> It means that instead of writing `CharacterLiteral::UTF32`
> you have to write `CharacterLiteral::CharacterKind::UTF32`
> 
> Seems a bit verbose. But I don't have any strong opinion on this.
It adds the protection against unintended conversions to and from the scoped 
enum, which in general is a good thing. The other benefits is not having the 
enumerators leak out into the surrounding scope but is limited in this case.

It does add verbosity though.


Repository:
  rC Clang

https://reviews.llvm.org/D54324



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

[PATCH] D53702: [ASTImporter] Set redecl chain of functions before any other import

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.

Did you ever resolve the issue of libcxx tests not running 
https://reviews.llvm.org/D53697


Repository:
  rC Clang

https://reviews.llvm.org/D53702



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

[PATCH] D54589: [clang][UBSan] Sanitization for alignment assumptions.

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri created this revision.
lebedev.ri added reviewers: ABataev, craig.topper, vsk, rsmith, rnk, 
Sanitizers, erichkeane, filcab, rjmccall.
lebedev.ri added a project: Sanitizers.
lebedev.ri added a dependency: D54588: [llvm][IRBuilder] Introspection for 
CreateAlignmentAssumption*() functions.

UB isn't nice. It's cool and powerful, but not nice.
Having a way to detect it is nice though.
P1007R3: std::assume_aligned  / 
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p1007r2.pdf says:

  We propose to add this functionality via a library function instead of a core 
language attribute.
  ...
  If the pointer passed in is not aligned to at least N bytes, calling 
assume_aligned results in undefined behaviour.

This differential teaches clang to sanitize all the various variants of this 
assume-aligned attribute.

Requires https://reviews.llvm.org/D54588 for LLVM IRBuilder changes.


Repository:
  rC Clang

https://reviews.llvm.org/D54589

Files:
  docs/ReleaseNotes.rst
  docs/UndefinedBehaviorSanitizer.rst
  lib/CodeGen/CGBuiltin.cpp
  lib/CodeGen/CGCall.cpp
  lib/CodeGen/CGExprScalar.cpp
  lib/CodeGen/CGStmtOpenMP.cpp
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenFunction.h
  test/CodeGen/catch-alignment-assumption-attribute-align_value-on-lvalue.cpp
  test/CodeGen/catch-alignment-assumption-attribute-align_value-on-paramvar.cpp
  
test/CodeGen/catch-alignment-assumption-attribute-alloc_align-on-function-variable.cpp
  test/CodeGen/catch-alignment-assumption-attribute-alloc_align-on-function.cpp
  
test/CodeGen/catch-alignment-assumption-attribute-assume_aligned-on-function-two-params.cpp
  
test/CodeGen/catch-alignment-assumption-attribute-assume_aligned-on-function.cpp
  test/CodeGen/catch-alignment-assumption-blacklist.c
  
test/CodeGen/catch-alignment-assumption-builtin_assume_aligned-three-params-variable.cpp
  
test/CodeGen/catch-alignment-assumption-builtin_assume_aligned-three-params.cpp
  test/CodeGen/catch-alignment-assumption-builtin_assume_aligned-two-params.cpp
  test/CodeGen/catch-alignment-assumption-openmp.cpp

Index: test/CodeGen/catch-alignment-assumption-openmp.cpp
===
--- /dev/null
+++ test/CodeGen/catch-alignment-assumption-openmp.cpp
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 -fopenmp-simd -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s --check-prefixes=CHECK,CHECK-NOSANITIZE
+// RUN: %clang_cc1 -fopenmp-simd -fsanitize=alignment -fno-sanitize-recover=alignment -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s -implicit-check-not="call void @__ubsan_handle_alignment_assumption" --check-prefixes=CHECK,CHECK-SANITIZE,CHECK-SANITIZE-ANYRECOVER,CHECK-SANITIZE-NORECOVER,CHECK-SANITIZE-UNREACHABLE
+// RUN: %clang_cc1 -fopenmp-simd -fsanitize=alignment -fsanitize-recover=alignment -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s -implicit-check-not="call void @__ubsan_handle_alignment_assumption" --check-prefixes=CHECK,CHECK-SANITIZE,CHECK-SANITIZE-ANYRECOVER,CHECK-SANITIZE-RECOVER
+// RUN: %clang_cc1 -fopenmp-simd -fsanitize=alignment -fsanitize-trap=alignment -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s -implicit-check-not="call void @__ubsan_handle_alignment_assumption" --check-prefixes=CHECK,CHECK-SANITIZE,CHECK-SANITIZE-TRAP,CHECK-SANITIZE-UNREACHABLE
+
+// CHECK-SANITIZE-ANYRECOVER: @[[CHAR:.*]] = {{.*}} c"'char *'\00" }
+// CHECK-SANITIZE-ANYRECOVER: @[[LINE_100_ALIGNMENT_ASSUMPTION:.*]] = {{.*}}, i32 100, i32 30 }, {{.*}}* @[[CHAR]] }
+
+void func(char *data) {
+  // CHECK: define void @{{.*}}(i8* %[[DATA:.*]])
+  // CHECK-NEXT: [[ENTRY:.*]]:
+  // CHECK-NEXT:   %[[DATA_ADDR:.*]] = alloca i8*, align 8
+  // CHECK:   store i8* %[[DATA]], i8** %[[DATA_ADDR]], align 8
+  // CHECK:   %[[DATA_RELOADED:.*]] = load i8*, i8** %[[DATA_ADDR]], align 8
+  // CHECK-NEXT:   %[[PTRINT:.*]] = ptrtoint i8* %[[DATA_RELOADED]] to i64
+  // CHECK-NEXT:   %[[MASKEDPTR:.*]] = and i64 %[[PTRINT]], 1073741823
+  // CHECK-NEXT:   %[[MASKCOND:.*]] = icmp eq i64 %[[MASKEDPTR]], 0
+  // CHECK-SANITIZE-NEXT:   %[[PTRINT_DUP:.*]] = ptrtoint i8* %[[DATA_RELOADED]] to i64, !nosanitize
+  // CHECK-SANITIZE-NEXT:   br i1 %[[MASKCOND]], label %[[CONT:.*]], label %[[HANDLER_ALIGNMENT_ASSUMPTION:[^,]+]],{{.*}} !nosanitize
+  // CHECK-SANITIZE:  [[HANDLER_ALIGNMENT_ASSUMPTION]]:
+  // CHECK-SANITIZE-NORECOVER-NEXT: call void @__ubsan_handle_alignment_assumption_abort(i8* bitcast ({ {{{.*}}}, {{{.*}}}, {{{.*}}}* }* @[[LINE_100_ALIGNMENT_ASSUMPTION]] to i8*), i64 %[[PTRINT_DUP]], i64 1073741824, i64 0, i64 %[[PTRINT]]){{.*}}, !nosanitize
+  // CHECK-SANITIZE-RECOVER-NEXT:   call void @__ubsan_handle_alignment_assumption(i8* bitcast ({ {{{.*}}}, {{{.*}}}, {{{.*}}}* }* @[[LINE_100_ALIGNMENT_ASSUMPTION]] to i8*), i64 %[[PTRINT_DUP]], i64 1073741824, i64 0, i64 %[[PTRINT]]){{.*}}, !nosanitize
+  // CHECK-SANITIZE-TRAP-NEXT:  

[PATCH] D51223: Update tests for new YAMLIO polymorphic traits

[Scott Linder via Phabricator via cfe-commits]

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

r346884


https://reviews.llvm.org/D51223



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

[PATCH] D51223: Update tests for new YAMLIO polymorphic traits

[Scott Linder via Phabricator via cfe-commits]

scott.linder closed this revision.
scott.linder added a comment.

r346884


https://reviews.llvm.org/D51223



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

[PATCH] D54576: [clang] - Simplify tools::SplitDebugName.

[David Blaikie via Phabricator via cfe-commits]

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

Agreed - looks like this went in untested in r175813 & has never been used.


https://reviews.llvm.org/D54576



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

[PATCH] D54592: [CStringChecker] evaluate explicit_bzero

[David CARLIER via Phabricator via cfe-commits]

devnexen created this revision.
devnexen added reviewers: george.karpenkov, dergachev.a.
devnexen created this object with visibility "All Users".
Herald added a subscriber: cfe-commits.

- explicit_bzero has limited scope/usage only for security/crypto purposes but 
is non-optimisable version of memset/0 and bzero.
- explicit_memset has similar signature and semantics as memset but is also a 
non-optimisable version.


Repository:
  rC Clang

https://reviews.llvm.org/D54592

Files:
  lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  test/Analysis/string.c

Index: test/Analysis/string.c
===
--- test/Analysis/string.c
+++ test/Analysis/string.c
@@ -1184,11 +1184,13 @@
 }
 
 //===--===
-// memset()
+// memset() / explicit_bzero()
 //===--===
 
 void *memset(void *dest, int ch, size_t count);
 
+void explicit_bzero(void *dest, size_t count);
+
 void *malloc(size_t size);
 void free(void *);
 
@@ -1383,6 +1385,32 @@
   clang_analyzer_eval(array[4] == 0); // expected-warning{{TRUE}}
 }
 
+void explicit_bzero1_null() {
+  char *a = NULL;
+
+  explicit_bzero(a, 10); // expected-warning{{Null pointer argument in call to memory clearance function}}
+}
+
+void explicit_bzero2_clear_mypassword() {
+  char passwd[7] = "passwd";
+
+  explicit_bzero(passwd, sizeof(passwd)); // no-warning
+
+  clang_analyzer_eval(strlen(passwd) == 0); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(passwd[0] == '\0'); // expected-warning{{UNKNOWN}}
+}
+
+#ifdef SUPPRESS_OUT_OF_BOUND
+void explicit_bzero3_out_ofbound() {
+  char *privkey = (char *)malloc(6);
+  const char newprivkey[10] = "mysafekey";
+
+  strcpy(privkey, "random");
+  explicit_bzero(privkey, sizeof(newprivkey));
+  clang_analyzer_eval(privkey[0] == '\0'); // expected-warning{{UNKNOWN}}
+  free(privkey);
+}
+#endif
 //===--===
 // FIXMEs
 //===--===
Index: lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -124,6 +124,7 @@
   void evalStdCopyBackward(CheckerContext &C, const CallExpr *CE) const;
   void evalStdCopyCommon(CheckerContext &C, const CallExpr *CE) const;
   void evalMemset(CheckerContext &C, const CallExpr *CE) const;
+  void evalExplicitBzero(CheckerContext &C, const CallExpr *CE) const;
 
   // Utility methods
   std::pair
@@ -2191,6 +2192,44 @@
   C.addTransition(State);
 }
 
+void CStringChecker::evalExplicitBzero(CheckerContext &C, const CallExpr *CE) const {
+  if (CE->getNumArgs() != 2)
+return;
+
+  CurrentFunctionDescription = "memory clearance function";
+
+  const Expr *Mem = CE->getArg(0);
+  const Expr *Size = CE->getArg(1);
+  ProgramStateRef State = C.getState();
+  
+  // See if the size argument is zero.
+  const LocationContext *LCtx = C.getLocationContext();
+  SVal SizeVal = State->getSVal(Size, LCtx);
+  QualType SizeTy = Size->getType();
+
+  ProgramStateRef StateZeroSize, StateNonZeroSize;
+  std::tie(StateZeroSize, StateNonZeroSize) =
+assumeZero(C, State, SizeVal, SizeTy);
+
+  // Get the value of the memory area.
+  SVal MemVal = State->getSVal(Mem, LCtx);
+  
+  // If the size is zero, there won't be any actual memory access,
+  // In this case we just return.
+  if (StateZeroSize && !StateNonZeroSize)
+return;
+
+  // Ensure the memory area is not null.
+  // If it is NULL there will be a NULL pointer dereference.
+  State = checkNonNull(C, StateNonZeroSize, Mem, MemVal);
+  if (!State)
+return;
+
+  State = CheckBufferAccess(C, State, Size, Mem);
+  if (!State)
+return;
+}
+
 static bool isCPPStdLibraryFunction(const FunctionDecl *FD, StringRef Name) {
   IdentifierInfo *II = FD->getIdentifier();
   if (!II)
@@ -2224,7 +2263,8 @@
 evalFunction =  &CStringChecker::evalMemcmp;
   else if (C.isCLibraryFunction(FDecl, "memmove"))
 evalFunction =  &CStringChecker::evalMemmove;
-  else if (C.isCLibraryFunction(FDecl, "memset"))
+  else if (C.isCLibraryFunction(FDecl, "memset") || 
+C.isCLibraryFunction(FDecl, "explicit_memset"))
 evalFunction =  &CStringChecker::evalMemset;
   else if (C.isCLibraryFunction(FDecl, "strcpy"))
 evalFunction =  &CStringChecker::evalStrcpy;
@@ -2262,6 +2302,8 @@
 evalFunction =  &CStringChecker::evalStdCopy;
   else if (isCPPStdLibraryFunction(FDecl, "copy_backward"))
 evalFunction =  &CStringChecker::evalStdCopyBackward;
+  else if (C.isCLibraryFunction(FDecl, "explicit_bzero"))
+evalFunction =  &CStringChecker::evalExplicitBzero;
 
   // If the callee isn't a string function, let another checker handle it.
   if (!evalFunction)

[PATCH] D52578: Thread safety analysis: Allow scoped releasing of capabilities

[Delesley Hutchins via Phabricator via cfe-commits]

delesley added inline comments.



Comment at: lib/Analysis/ThreadSafety.cpp:951
+}
   } else {
+// We're removing the underlying mutex. Warn on double unlocking.

aaronpuchert wrote:
> aaronpuchert wrote:
> > delesley wrote:
> > > aaronpuchert wrote:
> > > > delesley wrote:
> > > > > I find this very confusing.  A lock here unlocks the underlying 
> > > > > mutex, and vice versa.  At the very least, some methods need to be 
> > > > > renamed, or maybe we can have separate classes for ScopedLockable and 
> > > > > ScopedUnlockable. 
> > > > I agree that it's confusing, but it follows what I think was the idea 
> > > > behind scoped capabilities: that they are also capabilities that can be 
> > > > acquired/released. Now since the scoped capability releases a mutex on 
> > > > construction (i.e. when it is acquired), it has to acquire the mutex 
> > > > when released. So the way it handles the released mutexes mirrors what 
> > > > happens on the scoped capability itself.
> > > > 
> > > > It's definitely not very intuitive, but I feel it's the most consistent 
> > > > variant with what we have already.
> > > > 
> > > > The nice thing about this is that it works pretty well with the 
> > > > existing semantics and allows constructs such as `MutexLockUnlock` (see 
> > > > the tests) that unlocks one mutex and locks another. Not sure if anyone 
> > > > needs this, but why not?
> > > A scoped_lockable object is not a capability, it is an object that 
> > > manages capabilities.  IMHO, conflating those two concepts is a bad idea, 
> > > and recipe for confusion.  You would not, for example, use a 
> > > scoped_lockable object in a guarded_by attribute.  
> > > 
> > > Unfortunately, the existing code tends to conflate "capabilities" and 
> > > "facts", which is my fault.  A scoped_lockable object is a valid fact -- 
> > > the fact records whether the object is in scope -- it's just not a valid 
> > > capability. 
> > > 
> > > Simply renaming the methods from handleLock/Unlock to  addFact/removeFact 
> > > would be a nice first step in making the code clearer, and would 
> > > distinguish between facts that refer to capabilities, and facts that 
> > > refer to scoped objects.  
> > > 
> > > 
> > > 
> > > 
> > Makes sense to me. I'll see if I can refactor some of the code to clarify 
> > this.
> > 
> > I think that we should also separate between unlocking the underlying 
> > mutexes and destructing the scoped_lockable, which is currently handled via 
> > the extra parameter `FullyRemove` of `handleUnlock`.
> Scoped capabilities are not the same as capabilities, but they are also more 
> than just facts. (Except if you follow 
> [Wittgenstein](https://en.wikipedia.org/wiki/Tractatus_Logico-Philosophicus#Proposition_1),
>  perhaps.) They can be released, and reacquired, which makes them more than 
> mere immutable facts. I think we can consider them as proxy capabilities. 
> They cannot directly protect any resource, but we can acquire and release 
> (actual) capabilities through them. That's why it doesn't sound crazy to me 
> to say that releasing a scoped capability might acquire an underlying mutex, 
> if that was released on construction of the scoped capability.
The notion of "proxy" is how scoped objects have always been treated in the 
past, but that's mostly a historical accident, rather than a well-reasoned 
design choice.  As a bit of history, the first version of thread safety 
analysis only tracked mutexes.  Thus, the original implementation pretended 
that a scoped object was a mutex, because that's the only thing that the 
analysis could deal with.

As the analysis grew more complex, I switched to the current system based on 
"facts".  There are a number of facts that are potentially useful in static 
analysis, such as whether one expression aliases another, and most of them 
don't look at all like capabilities.  IMHO, knowing whether an object is within 
scope falls into this class.  The basic feature of facts is that they are 
control flow dependent -- they can be added to sets, removed from sets, and 
removed from intersections at joint points in the CFG.  Renaming the methods to 
reflect this design will make future extensions easier to understand.

The only real argument for treating scoped lockable objects as proxies, which 
can be "locked" and "unlocked", is that you can (in a future patch) reuse the 
existing acquire_capability and release_capability attributes to support 
releasing and then re-acquiring the proxy.  It's a bit counter-intuitive, but 
the alternative is adding new attributes, which is also bad.

However, even if you reuse the existing attributes, when it comes time to 
define a ScopedUnlockable class, you *still* shouldn't provide methods named 
"lock" and "unlock", where "lock"  releases the underlying mutex, and "unlock" 
reacquires it.  That's bad API design which will be endlessly confusing to 
users.

Thus, I would still 

[PATCH] D54441: [OPENMP] Support relational-op !- (not-equal) as one of the canonical forms of random access iterator

[Kelvin Li via Phabricator via cfe-commits]

kkwli0 added a comment.

Please do a rebase.  The test case teams_distribute_simd_loop_messages.cpp 
needs to update too.




Comment at: clang/lib/Sema/SemaOpenMP.cpp:3707
   ///   UB  >= Var
-  bool TestIsLessOp = false;
+  /// This will has no value when the condition is !=
+  llvm::Optional TestIsLessOp;

has -> have


Repository:
  rC Clang

https://reviews.llvm.org/D54441



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

[PATCH] D53764: [OpenCL] Enable address spaces for references in C++

[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.


https://reviews.llvm.org/D53764



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

[PATCH] D54489: Implement -frecord-command-line (-frecord-gcc-switches)

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: docs/ClangCommandLineReference.rst:797
+
+Generate a section .LLVM.command.line containing the clang driver command line.
+

scott.linder wrote:
> rjmccall wrote:
> > 1. Is this section always called `.LLVM.command.line`, or does it differ by 
> > target object-file format?
> > 2. For that matter, is this actually supported by all object-file formats?
> > 3. Please describe the format of the section.  Is it just a flat C string?  
> > Is it nul-terminated?  Are different options combined with spaces, and if 
> > so, does that mean that interior spaces in command line arguments are 
> > impossible to distinguish?  If the latter, I assume that this is required 
> > behavior for backward compatibility; please at least apologize for that in 
> > the documentation. :)
> For (1) I think the answer should be that we emit `.LLVM.command.line` (or 
> whatever name we land on) for every object-file format with the concept of 
> names for sections.
> 
> For (2), I have only implemented this for ELF so far in LLVM. I don't know 
> how reasonable that is, and if it isn't I can look at adding it to other 
> common object-file formats that LLVM supports.
> 
> For (3), my current proposal for the format is the same as the `.comment` 
> section for idents: null-surrounded strings. Interior spaces are escaped, in 
> the same manner as for the -g variant. There may still be more thought to put 
> into the format, as the GCC version null-terminates each individual option; 
> the reason I avoided this is that during object linking the options of many 
> command-lines simply get mixed together, which seems less than useful.
> 
> As an example, for ELF `clang -frecord-command-line -S "foo .c"` produces the 
> ASM:
> 
> ```
> .section.LLVM.command.line,"MS",@progbits,1
> .zero   1
> .ascii  "/usr/local/bin/clang-8 -frecord-command-line -S foo\\ .c"
> .zero   1
> ```
> 
> And for multiple command-lines in a single object (e.g. linking three objects 
> with recorded command-lines) this would be:
> 
> ```
> .section.LLVM.command.line,"MS",@progbits,1
> .zero   1
> .ascii  "/usr/local/bin/clang-8 -frecord-command-line -c foo\\ .c"
> .zero   1
> .ascii  "/usr/local/bin/clang-8 -frecord-command-line -some -unique -options 
> -c bar.c"
> .zero   1
> .ascii  "/usr/local/bin/clang-8 -frecord-command-line -something -else -c 
> baz.c"
> .zero   1
> ```
> 
> I will try to capture more of this in the documentation.
Okay.  I don't particularly need you to implement this on additional targets, 
but yes, please capture all this layout information in the documentation and 
make sure that the driver enforces any meaningful restrictions about which 
targets support it.


https://reviews.llvm.org/D54489



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

[PATCH] D54598: [CMake] Explicitly list Linux targets for Fuchsia toolchain

[Petr Hosek via Phabricator via cfe-commits]

phosek created this revision.
phosek added reviewers: mcgrathr, jakehehrlich, juliehockett.
Herald added subscribers: cfe-commits, mgorny.

Not all Linux targets use the ${arch}-linux-gnu spelling, so
specify the list of Linux explicitly.


Repository:
  rC Clang

https://reviews.llvm.org/D54598

Files:
  clang/cmake/caches/Fuchsia-stage2.cmake


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -33,41 +33,41 @@
   list(APPEND RUNTIME_TARGETS "default")
 endif()
 
-foreach(target i386;x86_64;armhf;aarch64)
+foreach(target 
i386-linux-gnu;x86_64-linux-gnu;arm-linux-gnueabi;aarch64-linux-gnu)
   if(LINUX_${target}_SYSROOT)
 # Set the per-target builtins options.
-list(APPEND BUILTIN_TARGETS "${target}-linux-gnu")
-set(BUILTINS_${target}-linux-gnu_CMAKE_SYSTEM_NAME Linux CACHE STRING "")
-set(BUILTINS_${target}-linux-gnu_CMAKE_BUILD_TYPE Release CACHE STRING "")
-set(BUILTINS_${target}-linux-gnu_CMAKE_SYSROOT ${LINUX_${target}_SYSROOT} 
CACHE STRING "")
-set(BUILTINS_${target}-linux-gnu_CMAKE_SHARED_LINKER_FLAGS "-fuse-ld=lld" 
CACHE STRING "")
-set(BUILTINS_${target}-linux-gnu_CMAKE_MODULE_LINKER_FLAGS "-fuse-ld=lld" 
CACHE STRING "")
-set(BUILTINS_${target}-linux-gnu_CMAKE_EXE_LINKER_FLAG "-fuse-ld=lld" 
CACHE STRING "")
+list(APPEND BUILTIN_TARGETS "${target}")
+set(BUILTINS_${target}_CMAKE_SYSTEM_NAME Linux CACHE STRING "")
+set(BUILTINS_${target}_CMAKE_BUILD_TYPE Release CACHE STRING "")
+set(BUILTINS_${target}_CMAKE_SYSROOT ${LINUX_${target}_SYSROOT} CACHE 
STRING "")
+set(BUILTINS_${target}_CMAKE_SHARED_LINKER_FLAGS "-fuse-ld=lld" CACHE 
STRING "")
+set(BUILTINS_${target}_CMAKE_MODULE_LINKER_FLAGS "-fuse-ld=lld" CACHE 
STRING "")
+set(BUILTINS_${target}_CMAKE_EXE_LINKER_FLAG "-fuse-ld=lld" CACHE STRING 
"")
 
 # Set the per-target runtimes options.
-list(APPEND RUNTIME_TARGETS "${target}-linux-gnu")
-set(RUNTIMES_${target}-linux-gnu_CMAKE_SYSTEM_NAME Linux CACHE STRING "")
-set(RUNTIMES_${target}-linux-gnu_CMAKE_BUILD_TYPE Release CACHE STRING "")
-set(RUNTIMES_${target}-linux-gnu_CMAKE_SYSROOT ${LINUX_${target}_SYSROOT} 
CACHE STRING "")
-set(RUNTIMES_${target}-linux-gnu_CMAKE_SHARED_LINKER_FLAGS "-fuse-ld=lld" 
CACHE STRING "")
-set(RUNTIMES_${target}-linux-gnu_CMAKE_MODULE_LINKER_FLAGS "-fuse-ld=lld" 
CACHE STRING "")
-set(RUNTIMES_${target}-linux-gnu_CMAKE_EXE_LINKER_FLAGS "-fuse-ld=lld" 
CACHE STRING "")
-set(RUNTIMES_${target}-linux-gnu_LLVM_ENABLE_ASSERTIONS ON CACHE BOOL "")
-set(RUNTIMES_${target}-linux-gnu_LIBUNWIND_ENABLE_SHARED OFF CACHE BOOL "")
-set(RUNTIMES_${target}-linux-gnu_LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL 
"")
-set(RUNTIMES_${target}-linux-gnu_LIBUNWIND_INSTALL_LIBRARY OFF CACHE BOOL 
"")
-set(RUNTIMES_${target}-linux-gnu_LIBCXXABI_USE_COMPILER_RT ON CACHE BOOL 
"")
-set(RUNTIMES_${target}-linux-gnu_LIBCXXABI_ENABLE_SHARED OFF CACHE BOOL "")
-set(RUNTIMES_${target}-linux-gnu_LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL 
"")
-set(RUNTIMES_${target}-linux-gnu_LIBCXXABI_ENABLE_STATIC_UNWINDER ON CACHE 
BOOL "")
-set(RUNTIMES_${target}-linux-gnu_LIBCXXABI_INSTALL_LIBRARY OFF CACHE BOOL 
"")
-set(RUNTIMES_${target}-linux-gnu_LIBCXX_USE_COMPILER_RT ON CACHE BOOL "")
-set(RUNTIMES_${target}-linux-gnu_LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
-set(RUNTIMES_${target}-linux-gnu_LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE 
BOOL "")
-set(RUNTIMES_${target}-linux-gnu_LIBCXX_ABI_VERSION 2 CACHE STRING "")
-set(RUNTIMES_${target}-linux-gnu_SANITIZER_CXX_ABI "libc++" CACHE STRING 
"")
-set(RUNTIMES_${target}-linux-gnu_SANITIZER_CXX_ABI_INTREE ON CACHE BOOL "")
-set(RUNTIMES_${target}-linux-gnu_COMPILER_RT_USE_BUILTINS_LIBRARY ON CACHE 
BOOL "")
+list(APPEND RUNTIME_TARGETS "${target}")
+set(RUNTIMES_${target}_CMAKE_SYSTEM_NAME Linux CACHE STRING "")
+set(RUNTIMES_${target}_CMAKE_BUILD_TYPE Release CACHE STRING "")
+set(RUNTIMES_${target}_CMAKE_SYSROOT ${LINUX_${target}_SYSROOT} CACHE 
STRING "")
+set(RUNTIMES_${target}_CMAKE_SHARED_LINKER_FLAGS "-fuse-ld=lld" CACHE 
STRING "")
+set(RUNTIMES_${target}_CMAKE_MODULE_LINKER_FLAGS "-fuse-ld=lld" CACHE 
STRING "")
+set(RUNTIMES_${target}_CMAKE_EXE_LINKER_FLAGS "-fuse-ld=lld" CACHE STRING 
"")
+set(RUNTIMES_${target}_LLVM_ENABLE_ASSERTIONS ON CACHE BOOL "")
+set(RUNTIMES_${target}_LIBUNWIND_ENABLE_SHARED OFF CACHE BOOL "")
+set(RUNTIMES_${target}_LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL "")
+set(RUNTIMES_${target}_LIBUNWIND_INSTALL_LIBRARY OFF CACHE BOOL "")
+set(RUNTIMES_${target}_LIBCXXABI_USE_COMPILER_RT ON CACHE BOOL "")
+set(RUNTIMES_${target}_LIBCXXABI_ENABLE_SHARED OFF CACHE BOOL "")
+set(RUNTIMES_${target}_LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL "")
+set(RUNTIMES_${target}_LIBCXXABI

[PATCH] D54245: [VFS] Implement `RedirectingFileSystem::getRealPath`.

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno accepted this revision.
bruno added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: jkorous.

LGTM


https://reviews.llvm.org/D54245



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

r346996 - Fix parens warning in assert in ASTMatchFinder

[Erich Keane via cfe-commits]

Author: erichkeane
Date: Thu Nov 15 13:35:35 2018
New Revision: 346996

URL: http://llvm.org/viewvc/llvm-project?rev=346996&view=rev
Log:
Fix parens warning in assert in ASTMatchFinder

Change-Id: Ie34f9c6846b98fba87449e73299519fc2346bac1

Modified:
cfe/trunk/lib/ASTMatchers/ASTMatchFinder.cpp

Modified: cfe/trunk/lib/ASTMatchers/ASTMatchFinder.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ASTMatchers/ASTMatchFinder.cpp?rev=346996&r1=346995&r2=346996&view=diff
==
--- cfe/trunk/lib/ASTMatchers/ASTMatchFinder.cpp (original)
+++ cfe/trunk/lib/ASTMatchers/ASTMatchFinder.cpp Thu Nov 15 13:35:35 2018
@@ -676,13 +676,13 @@ private:
   //  c) there is a bug in the AST, and the node is not reachable
   // Usually the traversal scope is the whole AST, which precludes b.
   // Bugs are common enough that it's worthwhile asserting when we can.
-  assert(Node.get() ||
- /* Traversal scope is limited if none of the bounds are the TU */
- llvm::none_of(ActiveASTContext->getTraversalScope(),
-   [](Decl *D) {
- return D->getKind() == Decl::TranslationUnit;
-   }) &&
- "Found node that is not in the complete parent map!");
+  assert((Node.get() ||
+  /* Traversal scope is limited if none of the bounds are the TU */
+  llvm::none_of(ActiveASTContext->getTraversalScope(),
+[](Decl *D) {
+  return D->getKind() == Decl::TranslationUnit;
+})) &&
+ "Found node that is not in the complete parent map!");
   return false;
 }
 if (Parents.size() == 1) {


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

[PATCH] D52674: [AST] Add Obj-C discriminator to MS ABI RTTI

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai planned changes to this revision.
smeenai added a comment.

In https://reviews.llvm.org/D52674#1298115, @rjmccall wrote:

> In https://reviews.llvm.org/D52674#1297893, @smeenai wrote:
>
> > In https://reviews.llvm.org/D52674#1297879, @rjmccall wrote:
> >
> > > I'm not worried about the mangler being re-used for multiple 
> > > declarations, I'm worried about a global flag changing how we mangle all 
> > > components of a type when we only mean to change it at the top level.
> >
> >
> > Hmm, but don't we want it to affect all components? For example, consider 
> > something like:
> >
> >   @interface I
> >   @end
> >  
> >   template  class C {};
> >  
> >   void f();
> >   void g() {
> > try {
> >   f();
> > } catch (C *) {
> > }
> >   }
> >
> >
> > I would say that we want the RTTI for `C *` to have the discriminator 
> > for `I`.
>
>
> Why?  IIUC, you're adding the discriminator to distinguish between two 
> different RTTI objects.  It's not like there are two different types.  You 
> should mangle `C` normally here.


You're right – I wasn't thinking this through correctly. Back to the drawing 
board.


Repository:
  rC Clang

https://reviews.llvm.org/D52674



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

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

[Leonard Chan via Phabricator via cfe-commits]

leonardchan updated this revision to Diff 174279.

Repository:
  rC Clang

https://reviews.llvm.org/D53738

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/FixedPoint.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Basic/FixedPoint.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Frontend/fixed_point_add.c
  clang/test/Frontend/fixed_point_add_ast.c
  clang/test/Frontend/fixed_point_conversions.c

Index: clang/test/Frontend/fixed_point_conversions.c
===
--- clang/test/Frontend/fixed_point_conversions.c
+++ clang/test/Frontend/fixed_point_conversions.c
@@ -214,19 +214,17 @@
   // Only get overflow checking if signed fract to unsigned accum
   sat_ua = sat_sf;
   // DEFAULT:  [[FRACT:%[0-9a-z]+]] = load i8, i8* %sat_sf, align 1
-  // DEFAULT-NEXT: [[FRACT_EXT:%[0-9a-z]+]] = sext i8 [[FRACT]] to i17
-  // DEFAULT-NEXT: [[ACCUM:%[0-9a-z]+]] = shl i17 [[FRACT_EXT]], 9
-  // DEFAULT-NEXT: [[IS_NEG:%[0-9a-z]+]] = icmp slt i17 [[ACCUM]], 0
-  // DEFAULT-NEXT: [[RESULT:%[0-9a-z]+]] = select i1 [[IS_NEG]], i17 0, i17 [[ACCUM]]
-  // DEFAULT-NEXT: [[RESULT_EXT:%[0-9a-z]+]] = sext i17 [[RESULT]] to i32
-  // DEFAULT-NEXT: store i32 [[RESULT_EXT]], i32* %sat_ua, align 4
+  // DEFAULT-NEXT: [[FRACT_EXT:%[0-9a-z]+]] = sext i8 [[FRACT]] to i32
+  // DEFAULT-NEXT: [[ACCUM:%[0-9a-z]+]] = shl i32 [[FRACT_EXT]], 9
+  // DEFAULT-NEXT: [[IS_NEG:%[0-9a-z]+]] = icmp slt i32 [[ACCUM]], 0
+  // DEFAULT-NEXT: [[RESULT:%[0-9a-z]+]] = select i1 [[IS_NEG]], i32 0, i32 [[ACCUM]]
+  // DEFAULT-NEXT: store i32 [[RESULT]], i32* %sat_ua, align 4
   // SAME:  [[FRACT:%[0-9a-z]+]] = load i8, i8* %sat_sf, align 1
-  // SAME-NEXT: [[FRACT_EXT:%[0-9a-z]+]] = sext i8 [[FRACT]] to i16
-  // SAME-NEXT: [[ACCUM:%[0-9a-z]+]] = shl i16 [[FRACT_EXT]], 8
-  // SAME-NEXT: [[IS_NEG:%[0-9a-z]+]] = icmp slt i16 [[ACCUM]], 0
-  // SAME-NEXT: [[RESULT:%[0-9a-z]+]] = select i1 [[IS_NEG]], i16 0, i16 [[ACCUM]]
-  // SAME-NEXT: [[RESULT_EXT:%[0-9a-z]+]] = sext i16 [[RESULT]] to i32
-  // SAME-NEXT: store i32 [[RESULT_EXT]], i32* %sat_ua, align 4
+  // SAME-NEXT: [[FRACT_EXT:%[0-9a-z]+]] = sext i8 [[FRACT]] to i32
+  // SAME-NEXT: [[ACCUM:%[0-9a-z]+]] = shl i32 [[FRACT_EXT]], 8
+  // SAME-NEXT: [[IS_NEG:%[0-9a-z]+]] = icmp slt i32 [[ACCUM]], 0
+  // SAME-NEXT: [[RESULT:%[0-9a-z]+]] = select i1 [[IS_NEG]], i32 0, i32 [[ACCUM]]
+  // SAME-NEXT: store i32 [[RESULT]], i32* %sat_ua, align 4
 }
 
 void TestFixedPointCastBetFractAccum() {
Index: clang/test/Frontend/fixed_point_add_ast.c
===
--- /dev/null
+++ clang/test/Frontend/fixed_point_add_ast.c
@@ -0,0 +1,337 @@
+// RUN: %clang_cc1 -x c -ffixed-point -ast-dump %s | FileCheck %s --strict-whitespace
+
+void SignedAdditions() {
+  // CHECK-LABEL: SignedAdditions
+  short _Accum sa;
+  _Accum a;
+  long _Accum la;
+  unsigned short _Accum usa;
+  unsigned _Accum ua;
+  unsigned long _Accum ula;
+
+  short _Fract sf;
+  _Fract f;
+  long _Fract lf;
+  unsigned short _Fract usf;
+  unsigned _Fract uf;
+  unsigned long _Fract ulf;
+
+  // CHECK-NOT:  FixedPointCast
+  // CHECK:  `-BinaryOperator {{.*}} 'short _Accum' '+'
+  // CHECK-NEXT:   |-ImplicitCastExpr {{.*}} 'short _Accum' 
+  // CHECK-NEXT:   | `-DeclRefExpr {{.*}} 'short _Accum' {{.*}} 'sa' 'short _Accum'
+  // CHECK-NEXT:   `-ImplicitCastExpr {{.*}} 'short _Accum' 
+  // CHECK-NEXT: `-DeclRefExpr {{.*}} 'short _Accum' {{.*}} 'sa' 'short _Accum'
+  sa = sa + sa;
+
+  // CHECK-NOT:  FixedPointCast
+  // CHECK:  `-BinaryOperator {{.*}} '_Accum' '+'
+  // CHECK-NEXT:   |-ImplicitCastExpr {{.*}} 'short _Accum' 
+  // CHECK-NEXT:   | `-DeclRefExpr {{.*}} 'short _Accum' {{.*}} 'sa' 'short _Accum'
+  // CHECK-NEXT:   `-ImplicitCastExpr {{.*}} '_Accum' 
+  // CHECK-NEXT: `-DeclRefExpr {{.*}} '_Accum' {{.*}} 'a' '_Accum'
+  a = sa + a;
+
+  // CHECK-NOT:  FixedPointCast
+  // CHECK:  `-BinaryOperator {{.*}} 'long _Accum' '+'
+  // CHECK-NEXT:   |-ImplicitCastExpr {{.*}} 'short _Accum' 
+  // CHECK-NEXT:   | `-DeclRefExpr {{.*}} 'short _Accum' {{.*}} 'sa' 'short _Accum'
+  // CHECK-NEXT:   `-ImplicitCastExpr {{.*}} 'long _Accum' 
+  // CHECK-NEXT: `-DeclRefExpr {{.*}} 'long _Accum' {{.*}} 'la' 'long _Accum'
+  la = sa + la;
+
+  // CHECK-NOT:  FixedPointCast
+  // CHECK:   `-ImplicitCastExpr {{.*}} 'long _Accum' 
+  // CHECK-NEXT:`-BinaryOperator {{.*}} '_Accum' '+'
+  // CHECK-NEXT:  |-ImplicitCastExpr {{.*}} 'short _Accum' 
+  // CHECK-NEXT:  | `-DeclRefExpr {{.*}} 'short _Accum' {{.*}} 'sa' 'short _Accum'
+  // CHECK-NEXT:  `-ImplicitCastExpr {{.*}} '_Accum' 
+  // CHECK-NEXT:`-DeclRefExpr {{.*}} '_Accum' {{.*}} 'a' '_Accum'
+  la = sa + a;
+
+  // CHECK-NOT:  FixedPointCast
+  // CHECK:  `-BinaryOperator {{.*}} 'short _Accum' '+'
+  // CHECK-NEXT:   |-ImplicitCastExpr {{.*}} 'short _Accum' 

[PATCH] D53329: Generate DIFile with main program if source is not available

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a subscriber: scott.linder.
dblaikie added a comment.

Thanks!

So I can see where/how this fails now - the LLVM backend seems to require that 
if any file has embedded source, they all do. Would you be able to/would you 
mind adding a debug info verifier check for this? That'd help make this fail 
sooner. (@scott.linder - perhaps you'd be able to/interested in doing this - 
looks like you wrote the initial implementation?)

Beyond that, then the question is whether this fix is the right way to satisfy 
that requirement (I'm assuming the requirement is reasonable - though I've not 
looked at the embedded source support & have no idea if there's a way/it's 
reasonable to support some embedded source and some not). My big question & 
I've tried to do some debugging to better understand this - but don't have the 
time to dive much further into it (& haven't really figured it out as yet): Why 
is this source not accessible through this API at this point? is the fallback 
to the main file always accurate?

(as far as I got was figuring out that SourceManager::getBufferData had an SLoc 
with isFile == true for the header, but isFile was false for the SLocs related 
to the .cpp file. Can't say I know why that is/what that means & would like to 
understand that (or someone else more familiar with this stuff who already 
knows can review this code) before approving this patch - if you could help 
explain this, that'd be great)


Repository:
  rC Clang

https://reviews.llvm.org/D53329



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

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

> Good case to bring up. For addition, I think we just need to add an extra 
> condition that checks for unsigned padding in the result. Added this test 
> also.

Actually this was wrong. Updated and instead dictate how the appropriate number 
of bits to `getCommonSemantics`. The saturation intrinsics handle unsigned 
padding types correctly now and only add the extra padding bit to the common 
width if both operands have unsigned padding and the result is not saturated 
(to prevent an unnecessary ext + trunc on non saturating operations).


Repository:
  rC Clang

https://reviews.llvm.org/D53738



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

r346997 - [CMake] Explicitly list Linux targets for Fuchsia toolchain

[Petr Hosek via cfe-commits]

Author: phosek
Date: Thu Nov 15 13:55:59 2018
New Revision: 346997

URL: http://llvm.org/viewvc/llvm-project?rev=346997&view=rev
Log:
[CMake] Explicitly list Linux targets for Fuchsia toolchain

Not all Linux targets use the ${arch}-linux-gnu spelling, so instead
specify the list of Linux explicitly.

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

Modified:
cfe/trunk/cmake/caches/Fuchsia-stage2.cmake

Modified: cfe/trunk/cmake/caches/Fuchsia-stage2.cmake
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/cmake/caches/Fuchsia-stage2.cmake?rev=346997&r1=346996&r2=346997&view=diff
==
--- cfe/trunk/cmake/caches/Fuchsia-stage2.cmake (original)
+++ cfe/trunk/cmake/caches/Fuchsia-stage2.cmake Thu Nov 15 13:55:59 2018
@@ -33,41 +33,41 @@ if(APPLE)
   list(APPEND RUNTIME_TARGETS "default")
 endif()
 
-foreach(target i386;x86_64;armhf;aarch64)
+foreach(target 
i386-linux-gnu;x86_64-linux-gnu;arm-linux-gnueabi;aarch64-linux-gnu)
   if(LINUX_${target}_SYSROOT)
 # Set the per-target builtins options.
-list(APPEND BUILTIN_TARGETS "${target}-linux-gnu")
-set(BUILTINS_${target}-linux-gnu_CMAKE_SYSTEM_NAME Linux CACHE STRING "")
-set(BUILTINS_${target}-linux-gnu_CMAKE_BUILD_TYPE Release CACHE STRING "")
-set(BUILTINS_${target}-linux-gnu_CMAKE_SYSROOT ${LINUX_${target}_SYSROOT} 
CACHE STRING "")
-set(BUILTINS_${target}-linux-gnu_CMAKE_SHARED_LINKER_FLAGS "-fuse-ld=lld" 
CACHE STRING "")
-set(BUILTINS_${target}-linux-gnu_CMAKE_MODULE_LINKER_FLAGS "-fuse-ld=lld" 
CACHE STRING "")
-set(BUILTINS_${target}-linux-gnu_CMAKE_EXE_LINKER_FLAG "-fuse-ld=lld" 
CACHE STRING "")
+list(APPEND BUILTIN_TARGETS "${target}")
+set(BUILTINS_${target}_CMAKE_SYSTEM_NAME Linux CACHE STRING "")
+set(BUILTINS_${target}_CMAKE_BUILD_TYPE Release CACHE STRING "")
+set(BUILTINS_${target}_CMAKE_SYSROOT ${LINUX_${target}_SYSROOT} CACHE 
STRING "")
+set(BUILTINS_${target}_CMAKE_SHARED_LINKER_FLAGS "-fuse-ld=lld" CACHE 
STRING "")
+set(BUILTINS_${target}_CMAKE_MODULE_LINKER_FLAGS "-fuse-ld=lld" CACHE 
STRING "")
+set(BUILTINS_${target}_CMAKE_EXE_LINKER_FLAG "-fuse-ld=lld" CACHE STRING 
"")
 
 # Set the per-target runtimes options.
-list(APPEND RUNTIME_TARGETS "${target}-linux-gnu")
-set(RUNTIMES_${target}-linux-gnu_CMAKE_SYSTEM_NAME Linux CACHE STRING "")
-set(RUNTIMES_${target}-linux-gnu_CMAKE_BUILD_TYPE Release CACHE STRING "")
-set(RUNTIMES_${target}-linux-gnu_CMAKE_SYSROOT ${LINUX_${target}_SYSROOT} 
CACHE STRING "")
-set(RUNTIMES_${target}-linux-gnu_CMAKE_SHARED_LINKER_FLAGS "-fuse-ld=lld" 
CACHE STRING "")
-set(RUNTIMES_${target}-linux-gnu_CMAKE_MODULE_LINKER_FLAGS "-fuse-ld=lld" 
CACHE STRING "")
-set(RUNTIMES_${target}-linux-gnu_CMAKE_EXE_LINKER_FLAGS "-fuse-ld=lld" 
CACHE STRING "")
-set(RUNTIMES_${target}-linux-gnu_LLVM_ENABLE_ASSERTIONS ON CACHE BOOL "")
-set(RUNTIMES_${target}-linux-gnu_LIBUNWIND_ENABLE_SHARED OFF CACHE BOOL "")
-set(RUNTIMES_${target}-linux-gnu_LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL 
"")
-set(RUNTIMES_${target}-linux-gnu_LIBUNWIND_INSTALL_LIBRARY OFF CACHE BOOL 
"")
-set(RUNTIMES_${target}-linux-gnu_LIBCXXABI_USE_COMPILER_RT ON CACHE BOOL 
"")
-set(RUNTIMES_${target}-linux-gnu_LIBCXXABI_ENABLE_SHARED OFF CACHE BOOL "")
-set(RUNTIMES_${target}-linux-gnu_LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL 
"")
-set(RUNTIMES_${target}-linux-gnu_LIBCXXABI_ENABLE_STATIC_UNWINDER ON CACHE 
BOOL "")
-set(RUNTIMES_${target}-linux-gnu_LIBCXXABI_INSTALL_LIBRARY OFF CACHE BOOL 
"")
-set(RUNTIMES_${target}-linux-gnu_LIBCXX_USE_COMPILER_RT ON CACHE BOOL "")
-set(RUNTIMES_${target}-linux-gnu_LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
-set(RUNTIMES_${target}-linux-gnu_LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE 
BOOL "")
-set(RUNTIMES_${target}-linux-gnu_LIBCXX_ABI_VERSION 2 CACHE STRING "")
-set(RUNTIMES_${target}-linux-gnu_SANITIZER_CXX_ABI "libc++" CACHE STRING 
"")
-set(RUNTIMES_${target}-linux-gnu_SANITIZER_CXX_ABI_INTREE ON CACHE BOOL "")
-set(RUNTIMES_${target}-linux-gnu_COMPILER_RT_USE_BUILTINS_LIBRARY ON CACHE 
BOOL "")
+list(APPEND RUNTIME_TARGETS "${target}")
+set(RUNTIMES_${target}_CMAKE_SYSTEM_NAME Linux CACHE STRING "")
+set(RUNTIMES_${target}_CMAKE_BUILD_TYPE Release CACHE STRING "")
+set(RUNTIMES_${target}_CMAKE_SYSROOT ${LINUX_${target}_SYSROOT} CACHE 
STRING "")
+set(RUNTIMES_${target}_CMAKE_SHARED_LINKER_FLAGS "-fuse-ld=lld" CACHE 
STRING "")
+set(RUNTIMES_${target}_CMAKE_MODULE_LINKER_FLAGS "-fuse-ld=lld" CACHE 
STRING "")
+set(RUNTIMES_${target}_CMAKE_EXE_LINKER_FLAGS "-fuse-ld=lld" CACHE STRING 
"")
+set(RUNTIMES_${target}_LLVM_ENABLE_ASSERTIONS ON CACHE BOOL "")
+set(RUNTIMES_${target}_LIBUNWIND_ENABLE_SHARED OFF CACHE BOOL "")
+set(RUNTIMES_${target}_LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL "")
+set(RUNTIMES_${target}_LIBUNWIND_INSTALL_LIBRARY OFF C