[PATCH] D55139: [clangd] Avoid memory-mapping files on Windows

[Nikolai Kosjar via Phabricator via cfe-commits]

nik added a comment.

Please fix this also for libclang clients. I think it's safe to assume that 
files might be edited once CXTranslationUnit_PrecompiledPreamble or 
CXTranslationUnit_CacheCompletionResults is set as flag - that's what 
clang_defaultEditingTranslationUnitOptions() returns.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55139



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

[PATCH] D55139: [clangd] Avoid memory-mapping files on Windows

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

LGTM




Comment at: clangd/FSProvider.cpp:33
+  return File;
+return std::unique_ptr(
+new VolatileFile(std::move(std::move(*File;

make_unique?



Comment at: clangd/FSProvider.cpp:34
+return std::unique_ptr(
+new VolatileFile(std::move(std::move(*File;
+  }

can we move once ?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55139



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

[PATCH] D54995: [MemoryBuffer] Add the setter to be able to force disabled mmap

[Ivan Donchevskii via Phabricator via cfe-commits]

yvvan updated this revision to Diff 176318.
yvvan retitled this revision from "[MemoryBuffer] By default assume that all 
files are volatile to prevent unintended file locks" to "[MemoryBuffer] Add the 
setter to be able to force disabled mmap".
yvvan edited the summary of this revision.
yvvan added a comment.

Instead of forcing some default value let's give the client an ability to force 
disabled mmap if one desires.


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

https://reviews.llvm.org/D54995

Files:
  include/llvm/Support/MemoryBuffer.h
  lib/Support/MemoryBuffer.cpp


Index: lib/Support/MemoryBuffer.cpp
===
--- lib/Support/MemoryBuffer.cpp
+++ lib/Support/MemoryBuffer.cpp
@@ -39,6 +39,12 @@
 // MemoryBuffer implementation itself.
 
//===--===//
 
+static bool MemoryMappingEnabled = true;
+
+void MemoryBuffer::enableMemoryMapping(bool enable) {
+  MemoryMappingEnabled = enable;
+}
+
 MemoryBuffer::~MemoryBuffer() { }
 
 /// init - Initialize this MemoryBuffer as a reference to externally allocated
@@ -312,6 +318,9 @@
   bool RequiresNullTerminator,
   int PageSize,
   bool IsVolatile) {
+  if (!MemoryMappingEnabled)
+return false;
+
   // mmap may leave the buffer without null terminator if the file size changed
   // by the time the last page is mapped in, so avoid it if the file size is
   // likely to change.
Index: include/llvm/Support/MemoryBuffer.h
===
--- include/llvm/Support/MemoryBuffer.h
+++ include/llvm/Support/MemoryBuffer.h
@@ -132,6 +132,8 @@
   getFileSlice(const Twine &Filename, uint64_t MapSize, uint64_t Offset,
bool IsVolatile = false);
 
+  static void enableMemoryMapping(bool enable);
+
   
//======//
   // Provided for performance analysis.
   
//======//


Index: lib/Support/MemoryBuffer.cpp
===
--- lib/Support/MemoryBuffer.cpp
+++ lib/Support/MemoryBuffer.cpp
@@ -39,6 +39,12 @@
 // MemoryBuffer implementation itself.
 //===--===//
 
+static bool MemoryMappingEnabled = true;
+
+void MemoryBuffer::enableMemoryMapping(bool enable) {
+  MemoryMappingEnabled = enable;
+}
+
 MemoryBuffer::~MemoryBuffer() { }
 
 /// init - Initialize this MemoryBuffer as a reference to externally allocated
@@ -312,6 +318,9 @@
   bool RequiresNullTerminator,
   int PageSize,
   bool IsVolatile) {
+  if (!MemoryMappingEnabled)
+return false;
+
   // mmap may leave the buffer without null terminator if the file size changed
   // by the time the last page is mapped in, so avoid it if the file size is
   // likely to change.
Index: include/llvm/Support/MemoryBuffer.h
===
--- include/llvm/Support/MemoryBuffer.h
+++ include/llvm/Support/MemoryBuffer.h
@@ -132,6 +132,8 @@
   getFileSlice(const Twine &Filename, uint64_t MapSize, uint64_t Offset,
bool IsVolatile = false);
 
+  static void enableMemoryMapping(bool enable);
+
   //======//
   // Provided for performance analysis.
   //======//
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54995: [MemoryBuffer] Add the setter to be able to force disabled mmap

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: lib/Support/MemoryBuffer.cpp:42
 
+static bool MemoryMappingEnabled = true;
+

Such global flags are a bad idea in general, and really not great in LLVM's 
case.
The editor would set it for "it's" llvm, but that will also affect the LLVM 
that is used by e.g. mesa.



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

https://reviews.llvm.org/D54995



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

[PATCH] D54995: [MemoryBuffer] Add the setter to be able to force disabled mmap

[Ivan Donchevskii via Phabricator via cfe-commits]

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



Comment at: lib/Support/MemoryBuffer.cpp:42
 
+static bool MemoryMappingEnabled = true;
+

lebedev.ri wrote:
> Such global flags are a bad idea in general, and really not great in LLVM's 
> case.
> The editor would set it for "it's" llvm, but that will also affect the LLVM 
> that is used by e.g. mesa.
> 
Oh no, don't mention mesa. The proper client should never share it's LLVM layer 
with mesa. We already got issues with versions incompatibility and the only 
good solution is to link llvm statically inside client. Otherwise mesa causes 
the mess anyways.

Also I expect this setter to be used only on Windows.

Of course there's another solution is that Ilya proposed but it's only for 
clangd there and is bigger in code size. And it can still allow bugs if 
somebody uses MemoryBuffer not through the FileSystem class.


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

https://reviews.llvm.org/D54995



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

[PATCH] D55124: [CodeComplete] Cleanup access checking in code completion

[Kadir Cetinkaya via Phabricator via cfe-commits]

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



Comment at: lib/Parse/ParseExprCXX.cpp:248
   // seen a leading '::' or part of a nested-name-specifier.
+  ParsedType ObjectTypeForCompletion = ObjectType;
   ObjectType = nullptr;

What about first checking for whether we are on code completion point and then 
assigning nullptr to objecttype?

Also it is not in the scope of this patch but I was wondering why we only 
trigger completion if we've seen a scope specifier, do you have any idea?


Repository:
  rC Clang

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

https://reviews.llvm.org/D55124



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

[PATCH] D54903: [Sema] Improve static_assert diagnostics.

[Clement Courbet via Phabricator via cfe-commits]

courbet marked 4 inline comments as done.
courbet added inline comments.



Comment at: lib/AST/NestedNameSpecifier.cpp:308-310
+if (ResolveTemplateArguments && getAsRecordDecl()) {
+  if (const auto *Record =
+  dyn_cast(getAsRecordDecl())) {

aaron.ballman wrote:
> I'd remove the `getAsRecordDecl()` from the first `if` and instead use 
> `dyn_cast_or_null` in the second `if`. Probably something like:
> ```
> const auto *Record = 
> dyn_cast_or_null(getAsRecordDecl());
> if (ResolveTemplateArguments && Record) {
> }
> ```
SG, thanks for the pointer to `dyn_cast_or_null`.



Comment at: lib/Sema/SemaTemplate.cpp:3071
+  printTemplateArgumentList(OS, IV->getTemplateArgs().asArray(), Policy);
+}
+return;

aaron.ballman wrote:
> Quuxplusone wrote:
> > Checking my understanding: Am I correct that this code currently does not 
> > pretty-print
> > 
> > static_assert(std::is_same(), "");
> > 
> > (creating an object of the trait type and then using its constexpr 
> > `operator bool` to convert it to bool)? This is a rare idiom and doesn't 
> > need to be supported AFAIC.
> I'm fine worrying about that situation for a follow-up patch if it isn't 
> currently supported.
This is not supported indeed. There are a bunch of other expr types that we 
could support.
This is the most frequent one (11.2% of our codebase), the next one I plan to 
address (in a follow-up patch) is 
`static_assert(!std;:type_trait::value)` with 6.6%.
The one you're mentioning here accounts for 2.8%.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54903



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

[PATCH] D54903: [Sema] Improve static_assert diagnostics.

[Clement Courbet via Phabricator via cfe-commits]

courbet updated this revision to Diff 176320.
courbet added a comment.

- Fix spurious formating changes
- Add tests
- Improve readability.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54903

Files:
  include/clang/AST/NestedNameSpecifier.h
  lib/AST/NestedNameSpecifier.cpp
  lib/Sema/SemaTemplate.cpp
  test/SemaCXX/static-assert-cxx17.cpp
  test/SemaCXX/static-assert.cpp

Index: test/SemaCXX/static-assert.cpp
===
--- test/SemaCXX/static-assert.cpp
+++ test/SemaCXX/static-assert.cpp
@@ -68,3 +68,100 @@
 };
 
 static_assert(first_trait::value && second_trait::value, "message"); // expected-error{{static_assert failed due to requirement 'second_trait::value' "message"}}
+
+namespace std {
+
+template 
+struct integral_constant {
+  static const Tp value = v;
+  typedef Tp value_type;
+  typedef integral_constant type;
+};
+
+template 
+const Tp integral_constant::value;
+
+typedef integral_constant true_type;
+typedef integral_constant false_type;
+
+template 
+struct is_const : public false_type {};
+template 
+struct is_const : public true_type {};
+
+// We do not define is_same in terms of integral_constant to check that both implementations are supported.
+template 
+struct is_same {
+  static const bool value = false;
+};
+
+template 
+struct is_same {
+  static const bool value = true;
+};
+
+} // namespace std
+
+struct ExampleTypes {
+  using T = int;
+  using U = float;
+};
+
+static_assert(std::is_same::value, "message");
+// expected-error@-1{{static_assert failed due to requirement 'std::is_same::value' "message"}}
+static_assert(std::is_const::value, "message");
+// expected-error@-1{{static_assert failed due to requirement 'std::is_const::value' "message"}}
+
+struct BI_tag {};
+struct RAI_tag : BI_tag {};
+struct MyIterator {
+  using tag = BI_tag;
+};
+struct MyContainer {
+  using iterator = MyIterator;
+};
+template 
+void foo() {
+  static_assert(std::is_same::value, "message");
+  // expected-error@-1{{static_assert failed due to requirement 'std::is_same::value' "message"}}
+}
+template void foo();
+// expected-note@-1{{in instantiation of function template specialization 'foo' requested here}}
+
+namespace ns {
+template 
+struct NestedTemplates1 {
+  struct NestedTemplates2 {
+template 
+struct NestedTemplates3 : public std::is_same {};
+  };
+};
+} // namespace ns
+
+template 
+void foo2() {
+  static_assert(::ns::NestedTemplates1::NestedTemplates2::template NestedTemplates3::value, "message");
+  // expected-error@-1{{static_assert failed due to requirement '::ns::NestedTemplates1::NestedTemplates2::NestedTemplates3::value' "message"}}
+}
+template void foo2();
+// expected-note@-1{{in instantiation of function template specialization 'foo2' requested here}}
+
+template 
+void foo3(T t) {
+  static_assert(std::is_const::value, "message");
+  // expected-error-re@-1{{static_assert failed due to requirement 'std::is_const<(lambda at {{.*}}static-assert.cpp:{{[0-9]*}}:{{[0-9]*}})>::value' "message"}}
+  static_assert(std::is_const::value, "message");
+  // expected-error-re@-1{{static_assert failed due to requirement 'std::is_const<(lambda at {{.*}}static-assert.cpp:{{[0-9]*}}:{{[0-9]*}})>::value' "message"}}
+}
+void callFoo3() {
+  foo3([]() {});
+  // expected-note@-1{{in instantiation of function template specialization 'foo3<(lambda at }}
+}
+
+template 
+void foo4(T t) {
+  static_assert(std::is_const::value, "message");
+  // expected-error@-1{{type 'int' cannot be used prior to '::' because it has no members}}
+}
+void callFoo4() { foo4(42); }
+// expected-note@-1{{in instantiation of function template specialization 'foo4' requested here}}
Index: test/SemaCXX/static-assert-cxx17.cpp
===
--- /dev/null
+++ test/SemaCXX/static-assert-cxx17.cpp
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++1z -triple=x86_64-linux-gnu
+
+template 
+struct S1 {
+  static constexpr const bool value = false;
+};
+
+template 
+inline constexpr bool global_inline_var = S1::value;
+
+template 
+struct S2 {
+  template 
+  static inline constexpr bool var = global_inline_var;
+};
+
+template 
+void foo() {
+  static_assert(S1::value);
+  // expected-error@-1{{static_assert failed due to requirement 'S1::value'}}
+}
+template void foo();
+// expected-note@-1{{in instantiation of function template specialization 'foo' requested here}}
+
+template 
+void foo2() {
+  static_assert(global_inline_var);
+  // expected-error@-1{{static_assert failed due to requirement 'global_inline_var'}}
+}
+template void foo2();
+// expected-note@-1{{in instantiation of function template specialization 'foo2' requested here}}
+
+template 
+void foo3() {
+  static_assert(T::template var);
+  // expected-error@-1{{static_assert failed due to requirement 'S2::var'}}
+}
+template void foo3, int, float>(

[PATCH] D55189: Extract TextNodeDumper class

[Stephen Kelly via Phabricator via cfe-commits]

steveire created this revision.
steveire added a reviewer: aaron.ballman.
Herald added a subscriber: cfe-commits.

Start by moving some utilities to it. It will eventually house dumping
of individual nodes (after indentation etc has already been accounted
for).


Repository:
  rC Clang

https://reviews.llvm.org/D55189

Files:
  include/clang/AST/ASTTextNodeDumper.h
  lib/AST/ASTDumper.cpp

Index: lib/AST/ASTDumper.cpp
===
--- lib/AST/ASTDumper.cpp
+++ lib/AST/ASTDumper.cpp
@@ -14,6 +14,7 @@
 
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTDumperUtils.h"
+#include "clang/AST/ASTTextNodeDumper.h"
 #include "clang/AST/Attr.h"
 #include "clang/AST/CommentVisitor.h"
 #include "clang/AST/DeclCXX.h"
@@ -45,6 +46,7 @@
 public TypeVisitor {
 
 TextChildDumper ChildDumper;
+TextNodeDumper NodeDumper;
 
 raw_ostream &OS;
 const CommandTraits *Traits;
@@ -57,11 +59,6 @@
 /// not already been loaded.
 bool Deserialize = false;
 
-/// Keep track of the last location we print out so that we can
-/// print out deltas from then on out.
-const char *LastLocFilename = "";
-unsigned LastLocLine = ~0U;
-
 const bool ShowColors;
 
 /// Dump a child of the current node.
@@ -81,8 +78,9 @@
 ASTDumper(raw_ostream &OS, const CommandTraits *Traits,
   const SourceManager *SM, bool ShowColors,
   const PrintingPolicy &PrintPolicy)
-: ChildDumper(OS, ShowColors), OS(OS), Traits(Traits), SM(SM),
-  PrintPolicy(PrintPolicy), ShowColors(ShowColors) {}
+: ChildDumper(OS, ShowColors),
+  NodeDumper(OS, ShowColors, SM, PrintPolicy), OS(OS), Traits(Traits),
+  SM(SM), PrintPolicy(PrintPolicy), ShowColors(ShowColors) {}
 
 void setDeserialize(bool D) { Deserialize = D; }
 
@@ -91,23 +89,17 @@
 void dumpFullComment(const FullComment *C);
 
 // Utilities
-void dumpPointer(const void *Ptr);
-void dumpSourceRange(SourceRange R);
-void dumpLocation(SourceLocation Loc);
-void dumpBareType(QualType T, bool Desugar = true);
-void dumpType(QualType T);
+void dumpType(QualType T) { NodeDumper.dumpType(T); }
 void dumpTypeAsChild(QualType T);
 void dumpTypeAsChild(const Type *T);
-void dumpBareDeclRef(const Decl *Node);
 void dumpDeclRef(const Decl *Node, const char *Label = nullptr);
-void dumpName(const NamedDecl *D);
+void dumpBareDeclRef(const Decl *Node) { NodeDumper.dumpBareDeclRef(Node); }
 bool hasNodes(const DeclContext *DC);
 void dumpDeclContext(const DeclContext *DC);
 void dumpLookups(const DeclContext *DC, bool DumpDecls);
 void dumpAttr(const Attr *A);
 
 // C++ Utilities
-void dumpAccessSpecifier(AccessSpecifier AS);
 void dumpCXXCtorInitializer(const CXXCtorInitializer *Init);
 void dumpTemplateParameters(const TemplateParameterList *TPL);
 void dumpTemplateArgumentListInfo(const TemplateArgumentListInfo &TALI);
@@ -156,20 +148,20 @@
 }
 void VisitVariableArrayType(const VariableArrayType *T) {
   OS << " ";
-  dumpSourceRange(T->getBracketsRange());
+  NodeDumper.dumpSourceRange(T->getBracketsRange());
   VisitArrayType(T);
   dumpStmt(T->getSizeExpr());
 }
 void VisitDependentSizedArrayType(const DependentSizedArrayType *T) {
   VisitArrayType(T);
   OS << " ";
-  dumpSourceRange(T->getBracketsRange());
+  NodeDumper.dumpSourceRange(T->getBracketsRange());
   dumpStmt(T->getSizeExpr());
 }
 void VisitDependentSizedExtVectorType(
 const DependentSizedExtVectorType *T) {
   OS << " ";
-  dumpLocation(T->getAttributeLoc());
+  NodeDumper.dumpLocation(T->getAttributeLoc());
   dumpTypeAsChild(T->getElementType());
   dumpStmt(T->getSizeExpr());
 }
@@ -421,7 +413,6 @@
 void VisitMaterializeTemporaryExpr(const MaterializeTemporaryExpr *Node);
 void VisitExprWithCleanups(const ExprWithCleanups *Node);
 void VisitUnresolvedLookupExpr(const UnresolvedLookupExpr *Node);
-void dumpCXXTemporary(const CXXTemporary *Temporary);
 void VisitLambdaExpr(const LambdaExpr *Node) {
   VisitExpr(Node);
   dumpDecl(Node->getLambdaClass());
@@ -475,77 +466,6 @@
 //  Utilities
 //===--===//
 
-void ASTDumper::dumpPointer(const void *Ptr) {
-  ColorScope Color(OS, ShowColors, AddressColor);
-  OS << ' ' << Ptr;
-}
-
-void ASTDumper::dumpLocation(SourceLocation Loc) {
-  if (!SM)
-return;
-
-  ColorScope Color(OS, ShowColors, LocationColor);
-  SourceLocation SpellingLoc = SM->getSpellingLoc(Loc);
-
-  // The general format we print out is filename:line:col, but we drop pieces
-  // that haven't changed since the last loc printed.
-  PresumedLoc PLoc = SM->getPresumedLoc(SpellingLoc);
-
-  if (PLoc.isInvalid()) {
-OS << "";
-return;
-  }
-
-  if (strcmp(PLoc.getFilename()

[PATCH] D55188: Extract TextChildDumper class

[Stephen Kelly via Phabricator via cfe-commits]

steveire created this revision.
steveire added a reviewer: aaron.ballman.
Herald added a subscriber: cfe-commits.

Repository:
  rC Clang

https://reviews.llvm.org/D55188

Files:
  include/clang/AST/ASTDumperUtils.h
  lib/AST/ASTDumper.cpp

Index: lib/AST/ASTDumper.cpp
===
--- lib/AST/ASTDumper.cpp
+++ lib/AST/ASTDumper.cpp
@@ -13,6 +13,7 @@
 //===--===//
 
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/ASTDumperUtils.h"
 #include "clang/AST/Attr.h"
 #include "clang/AST/CommentVisitor.h"
 #include "clang/AST/DeclCXX.h"
@@ -29,84 +30,13 @@
 #include "llvm/Support/raw_ostream.h"
 using namespace clang;
 using namespace clang::comments;
+using namespace clang::ast_dumper;
 
 //===--===//
 // ASTDumper Visitor
 //===--===//
 
 namespace  {
-  // Colors used for various parts of the AST dump
-  // Do not use bold yellow for any text.  It is hard to read on white screens.
-
-  struct TerminalColor {
-raw_ostream::Colors Color;
-bool Bold;
-  };
-
-  // Red   - CastColor
-  // Green - TypeColor
-  // Bold Green- DeclKindNameColor, UndeserializedColor
-  // Yellow- AddressColor, LocationColor
-  // Blue  - CommentColor, NullColor, IndentColor
-  // Bold Blue - AttrColor
-  // Bold Magenta  - StmtColor
-  // Cyan  - ValueKindColor, ObjectKindColor
-  // Bold Cyan - ValueColor, DeclNameColor
-
-  // Decl kind names (VarDecl, FunctionDecl, etc)
-  static const TerminalColor DeclKindNameColor = { raw_ostream::GREEN, true };
-  // Attr names (CleanupAttr, GuardedByAttr, etc)
-  static const TerminalColor AttrColor = { raw_ostream::BLUE, true };
-  // Statement names (DeclStmt, ImplicitCastExpr, etc)
-  static const TerminalColor StmtColor = { raw_ostream::MAGENTA, true };
-  // Comment names (FullComment, ParagraphComment, TextComment, etc)
-  static const TerminalColor CommentColor = { raw_ostream::BLUE, false };
-
-  // Type names (int, float, etc, plus user defined types)
-  static const TerminalColor TypeColor = { raw_ostream::GREEN, false };
-
-  // Pointer address
-  static const TerminalColor AddressColor = { raw_ostream::YELLOW, false };
-  // Source locations
-  static const TerminalColor LocationColor = { raw_ostream::YELLOW, false };
-
-  // lvalue/xvalue
-  static const TerminalColor ValueKindColor = { raw_ostream::CYAN, false };
-  // bitfield/objcproperty/objcsubscript/vectorcomponent
-  static const TerminalColor ObjectKindColor = { raw_ostream::CYAN, false };
-
-  // Null statements
-  static const TerminalColor NullColor = { raw_ostream::BLUE, false };
-
-  // Undeserialized entities
-  static const TerminalColor UndeserializedColor = { raw_ostream::GREEN, true };
-
-  // CastKind from CastExpr's
-  static const TerminalColor CastColor = { raw_ostream::RED, false };
-
-  // Value of the statement
-  static const TerminalColor ValueColor = { raw_ostream::CYAN, true };
-  // Decl names
-  static const TerminalColor DeclNameColor = { raw_ostream::CYAN, true };
-
-  // Indents ( `, -. | )
-  static const TerminalColor IndentColor = { raw_ostream::BLUE, false };
-
-  class ColorScope {
-raw_ostream &OS;
-const bool ShowColors;
-
-  public:
-ColorScope(raw_ostream &OS, bool ShowColors, TerminalColor Color)
-: OS(OS), ShowColors(ShowColors) {
-  if (ShowColors)
-OS.changeColor(Color.Color, Color.Bold);
-}
-~ColorScope() {
-  if (ShowColors)
-OS.resetColor();
-}
-  };
 
   class ASTDumper
   : public ConstDeclVisitor,
@@ -114,6 +44,8 @@
 public ConstCommentVisitor,
 public TypeVisitor {
 
+TextChildDumper ChildDumper;
+
 raw_ostream &OS;
 const CommandTraits *Traits;
 const SourceManager *SM;
@@ -121,22 +53,10 @@
 /// The policy to use for printing; can be defaulted.
 PrintingPolicy PrintPolicy;
 
-/// Pending[i] is an action to dump an entity at level i.
-llvm::SmallVector, 32> Pending;
-
 /// Indicates whether we should trigger deserialization of nodes that had
 /// not already been loaded.
 bool Deserialize = false;
 
-/// Indicates whether we're at the top level.
-bool TopLevel = true;
-
-/// Indicates if we're handling the first child after entering a new depth.
-bool FirstChild = true;
-
-/// Prefix for currently-being-dumped entity.
-std::string Prefix;
-
 /// Keep track of the last location we print out so that we can
 /// print out deltas from then on out.
 const char *LastLocFilename = "";
@@ -146,65 +66,7 @@
 
 /// Dump a child of the current node.
 template void dumpChild(Fn doDumpChild) {
-  // If we're at the top level, there's nothing interesting to do; just
-  // run the dumper.
-  if (TopLe

[PATCH] D55062: [clangd] Partition include graph on auto-index.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 176323.
kadircet marked 5 inline comments as done.
kadircet added a comment.

- Address comments


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55062

Files:
  clangd/index/Background.cpp
  unittests/clangd/BackgroundIndexTests.cpp

Index: unittests/clangd/BackgroundIndexTests.cpp
===
--- unittests/clangd/BackgroundIndexTests.cpp
+++ unittests/clangd/BackgroundIndexTests.cpp
@@ -93,7 +93,7 @@
   Cmd.Filename = testPath("root/A.cc");
   Cmd.Directory = testPath("root");
   Cmd.CommandLine = {"clang++", "-DA=1", testPath("root/A.cc")};
-  CDB.setCompileCommand(testPath("root"), Cmd);
+  CDB.setCompileCommand(testPath("root/A.cc"), Cmd);
 
   ASSERT_TRUE(Idx.blockUntilIdleForTest());
   EXPECT_THAT(
@@ -103,7 +103,7 @@
 
   Cmd.Filename = testPath("root/B.cc");
   Cmd.CommandLine = {"clang++", Cmd.Filename};
-  CDB.setCompileCommand(testPath("root"), Cmd);
+  CDB.setCompileCommand(testPath("root/A.cc"), Cmd);
 
   ASSERT_TRUE(Idx.blockUntilIdleForTest());
   // B_CC is dropped as we don't collect symbols from A.h in this compilation.
@@ -143,7 +143,7 @@
 OverlayCDB CDB(/*Base=*/nullptr);
 BackgroundIndex Idx(Context::empty(), "", FS, CDB,
 [&](llvm::StringRef) { return &MSS; });
-CDB.setCompileCommand(testPath("root"), Cmd);
+CDB.setCompileCommand(testPath("root/A.cc"), Cmd);
 ASSERT_TRUE(Idx.blockUntilIdleForTest());
   }
   EXPECT_EQ(CacheHits, 0U);
@@ -165,5 +165,48 @@
   EXPECT_THAT(*ShardSource->Refs, RefsAre({FileURI("unittest:///root/A.cc")}));
 }
 
+TEST_F(BackgroundIndexTest, DirectIncludesTest) {
+  MockFSProvider FS;
+  FS.Files[testPath("root/B.h")] = "";
+  FS.Files[testPath("root/A.h")] = R"cpp(
+  #include "B.h"
+  void common();
+  void f_b();
+  class A_CC {};
+  )cpp";
+  std::string A_CC = "#include \"A.h\"\nvoid g() { (void)common; }";
+  FS.Files[testPath("root/A.cc")] = A_CC;
+
+  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")};
+  {
+OverlayCDB CDB(/*Base=*/nullptr);
+BackgroundIndex Idx(Context::empty(), "", FS, CDB,
+[&](llvm::StringRef) { return &MSS; });
+CDB.setCompileCommand(testPath("root/A.cc"), Cmd);
+ASSERT_TRUE(Idx.blockUntilIdleForTest());
+  }
+
+  auto ShardSource = MSS.loadShard(testPath("root/A.cc"));
+  EXPECT_TRUE(ShardSource->Sources);
+  EXPECT_EQ(ShardSource->Sources->size(), 2U); // A.cc, A.h
+  EXPECT_THAT(
+  ShardSource->Sources->lookup("unittest:///root/A.cc").DirectIncludes,
+  UnorderedElementsAre("unittest:///root/A.h"));
+
+  auto ShardHeader = MSS.loadShard(testPath("root/A.h"));
+  EXPECT_TRUE(ShardHeader->Sources);
+  EXPECT_EQ(ShardHeader->Sources->size(), 2U); // A.h B.h
+  EXPECT_THAT(
+  ShardHeader->Sources->lookup("unittest:///root/A.h").DirectIncludes,
+  UnorderedElementsAre("unittest:///root/B.h"));
+}
+
 } // namespace clangd
 } // namespace clang
Index: clangd/index/Background.cpp
===
--- clangd/index/Background.cpp
+++ clangd/index/Background.cpp
@@ -35,6 +35,59 @@
 using namespace llvm;
 namespace clang {
 namespace clangd {
+namespace {
+// Resolves URI to file paths with cache.
+class URIToFileCache {
+public:
+  URIToFileCache(llvm::StringRef HintPath) : HintPath(HintPath) {}
+
+  llvm::StringRef resolve(llvm::StringRef FileURI) {
+auto I = URIToPathCache.try_emplace(FileURI);
+if (I.second) {
+  auto U = URI::parse(FileURI);
+  if (!U) {
+elog("Failed to parse URI {0}: {1}", FileURI, U.takeError());
+assert(false && "Failed to parse URI");
+return "";
+  }
+  auto Path = URI::resolve(*U, HintPath);
+  if (!Path) {
+elog("Failed to resolve URI {0}: {1}", FileURI, Path.takeError());
+assert(false && "Failed to resolve URI");
+return "";
+  }
+  I.first->second = *Path;
+}
+return I.first->second;
+  }
+
+private:
+  std::string HintPath;
+  llvm::StringMap URIToPathCache;
+};
+
+// We keep only the node "Path" and its edges.
+IncludeGraph getSubGraph(const URI &U, const IncludeGraph &FullGraph) {
+  IncludeGraph IG;
+
+  std::string FileURI = U.toString();
+  auto Entry = IG.try_emplace(FileURI).first;
+  auto &Node = Entry->getValue();
+  Node = FullGraph.lookup(Entry->getKey());
+  Node.URI = Entry->getKey();
+
+  // Since the strings in direct includes are references to the keys of the
+  // fullgraph, we need to create entries in our new sub-graph for those
+  // includes and change references to point to the new graph's keys.
+  for (auto &Include : Node.DirectInclud

[PATCH] D55062: [clangd] Partition include graph on auto-index.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clangd/index/Background.cpp:260
+std::unique_ptr IG =
+Index.Sources ? llvm::make_unique(getSubGraph(
+URI::create(Path), Index.Sources.getValue()))

ilya-biryukov wrote:
> What are the cases when it happens in practice? Do we ever run the indexer 
> without producing the include graph?
You are right that's not the case any more.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55062



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

[PATCH] D55190: Move dump of individual comment nodes to NodeDumper

[Stephen Kelly via Phabricator via cfe-commits]

steveire created this revision.
steveire added a reviewer: aaron.ballman.
Herald added a subscriber: cfe-commits.

Repository:
  rC Clang

https://reviews.llvm.org/D55190

Files:
  include/clang/AST/ASTTextNodeDumper.h
  lib/AST/ASTDumper.cpp

Index: lib/AST/ASTDumper.cpp
===
--- lib/AST/ASTDumper.cpp
+++ lib/AST/ASTDumper.cpp
@@ -49,7 +49,6 @@
 TextNodeDumper NodeDumper;
 
 raw_ostream &OS;
-const CommandTraits *Traits;
 const SourceManager *SM;
 
 /// The policy to use for printing; can be defaulted.
@@ -79,8 +78,8 @@
   const SourceManager *SM, bool ShowColors,
   const PrintingPolicy &PrintPolicy)
 : ChildDumper(OS, ShowColors),
-  NodeDumper(OS, ShowColors, SM, PrintPolicy), OS(OS), Traits(Traits),
-  SM(SM), PrintPolicy(PrintPolicy), ShowColors(ShowColors) {}
+  NodeDumper(OS, ShowColors, SM, PrintPolicy, Traits), OS(OS), SM(SM),
+  PrintPolicy(PrintPolicy), ShowColors(ShowColors) {}
 
 void setDeserialize(bool D) { Deserialize = D; }
 
@@ -436,29 +435,6 @@
 // Comments.
 const char *getCommandName(unsigned CommandID);
 void dumpComment(const Comment *C, const FullComment *FC);
-
-// Inline comments.
-void visitTextComment(const TextComment *C, const FullComment *FC);
-void visitInlineCommandComment(const InlineCommandComment *C,
-   const FullComment *FC);
-void visitHTMLStartTagComment(const HTMLStartTagComment *C,
-  const FullComment *FC);
-void visitHTMLEndTagComment(const HTMLEndTagComment *C,
-const FullComment *FC);
-
-// Block comments.
-void visitBlockCommandComment(const BlockCommandComment *C,
-  const FullComment *FC);
-void visitParamCommandComment(const ParamCommandComment *C,
-  const FullComment *FC);
-void visitTParamCommandComment(const TParamCommandComment *C,
-   const FullComment *FC);
-void visitVerbatimBlockComment(const VerbatimBlockComment *C,
-   const FullComment *FC);
-void visitVerbatimBlockLineComment(const VerbatimBlockLineComment *C,
-   const FullComment *FC);
-void visitVerbatimLineComment(const VerbatimLineComment *C,
-  const FullComment *FC);
   };
 }
 
@@ -2359,15 +2335,6 @@
 // Comments
 //===--===//
 
-const char *ASTDumper::getCommandName(unsigned CommandID) {
-  if (Traits)
-return Traits->getCommandInfo(CommandID)->Name;
-  const CommandInfo *Info = CommandTraits::getBuiltinCommandInfo(CommandID);
-  if (Info)
-return Info->Name;
-  return "";
-}
-
 void ASTDumper::dumpFullComment(const FullComment *C) {
   if (!C)
 return;
@@ -2376,18 +2343,10 @@
 
 void ASTDumper::dumpComment(const Comment *C, const FullComment *FC) {
   dumpChild([=] {
+NodeDumper.visit(C, FC);
 if (!C) {
-  ColorScope Color(OS, ShowColors, NullColor);
-  OS << "<<>>";
   return;
 }
-
-{
-  ColorScope Color(OS, ShowColors, CommentColor);
-  OS << C->getCommentKindName();
-}
-NodeDumper.dumpPointer(C);
-NodeDumper.dumpSourceRange(C->getSourceRange());
 ConstCommentVisitor::visit(C, FC);
 for (Comment::child_iterator I = C->child_begin(), E = C->child_end();
  I != E; ++I)
@@ -2395,114 +2354,6 @@
   });
 }
 
-void ASTDumper::visitTextComment(const TextComment *C, const FullComment *) {
-  OS << " Text=\"" << C->getText() << "\"";
-}
-
-void ASTDumper::visitInlineCommandComment(const InlineCommandComment *C,
-  const FullComment *) {
-  OS << " Name=\"" << getCommandName(C->getCommandID()) << "\"";
-  switch (C->getRenderKind()) {
-  case InlineCommandComment::RenderNormal:
-OS << " RenderNormal";
-break;
-  case InlineCommandComment::RenderBold:
-OS << " RenderBold";
-break;
-  case InlineCommandComment::RenderMonospaced:
-OS << " RenderMonospaced";
-break;
-  case InlineCommandComment::RenderEmphasized:
-OS << " RenderEmphasized";
-break;
-  }
-
-  for (unsigned i = 0, e = C->getNumArgs(); i != e; ++i)
-OS << " Arg[" << i << "]=\"" << C->getArgText(i) << "\"";
-}
-
-void ASTDumper::visitHTMLStartTagComment(const HTMLStartTagComment *C,
- const FullComment *) {
-  OS << " Name=\"" << C->getTagName() << "\"";
-  if (C->getNumAttrs() != 0) {
-OS << " Attrs: ";
-for (unsigned i = 0, e = C->getNumAttrs(); i != e; ++i) {
-  const HTMLStartTagComment::Attribute &Attr = C->getAttr(i);
-  OS << " \"" << Attr.Name << "=\"" << Attr.Value << "\"";
-}
-  }
-  if (C->isSelfClosing())
-OS << " SelfClosing";
-}
-
-void ASTDumper::v

[PATCH] D55191: [clangd] Refine the way of checking a declaration is referenced by the written code.

[Haojian Wu via Phabricator via cfe-commits]

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

The previous solution (checking the AST) is not a reliable way to
determine whether a declaration is explicitly referenced by the source
code, we are still missing a few cases.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D55191

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

Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -1225,6 +1225,53 @@
   }
 }
 
+TEST(FindReferences, ExplicitSymbols) {
+  const char *Tests[] = {
+  R"cpp(
+  struct Foo { Foo* [self]() const; };
+  void f() {
+if (Foo* T = foo.[^self]()) {} // Foo member call expr.
+  }
+  )cpp",
+
+  R"cpp(
+  struct Foo { Foo(int); };
+  Foo f() {
+int [b];
+return [^b]; // Foo constructor expr.
+  }
+  )cpp",
+
+  R"cpp(
+  struct Foo {};
+  void g(Foo);
+  Foo [f]();
+  void call() {
+g([^f]());  // Foo constructor expr.
+  }
+  )cpp",
+
+  R"cpp(
+  void [foo](int);
+  void [foo](double);
+
+  namespace ns {
+  using [fo^o];
+  }
+  )cpp",
+  };
+  for (const char *Test : Tests) {
+Annotations T(Test);
+auto AST = TestTU::withCode(T.code()).build();
+std::vector> ExpectedLocations;
+for (const auto &R : T.ranges())
+  ExpectedLocations.push_back(RangeIs(R));
+EXPECT_THAT(findReferences(AST, T.point()),
+ElementsAreArray(ExpectedLocations))
+<< Test;
+  }
+}
+
 TEST(FindReferences, NeedsIndex) {
   const char *Header = "int foo();";
   Annotations Main("int main() { [[f^oo]](); }");
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -139,30 +139,30 @@
   SourceLocation Loc,
   index::IndexDataConsumer::ASTNodeInfo ASTNode) override {
 if (Loc == SearchedLocation) {
-  // Check whether the E has an implicit AST node (e.g. ImplicitCastExpr).
-  auto hasImplicitExpr = [](const Expr *E) {
-if (!E || E->child_begin() == E->child_end())
+  // Check whether the give ND is explicitly referenced by the written
+  // identifier at Loc in the source code.
+  auto IsWrittenInCode = [this](const NamedDecl *ND, SourceLocation Loc) {
+if (!ND)
   return false;
-// Use the first child is good enough for most cases -- normally the
-// expression returned by handleDeclOccurence contains exactly one
-// child expression.
-const auto *FirstChild = *E->child_begin();
-return isa(FirstChild) ||
-   isa(FirstChild) ||
-   isa(FirstChild) ||
-   isa(FirstChild);
+auto &SM = AST.getSourceManager();
+auto FileIDAndOffset = SM.getDecomposedLoc(Loc);
+auto Code = SM.getBufferData(FileIDAndOffset.first);
+auto IdentifierLength =
+clang::Lexer::MeasureTokenLength(Loc, SM, AST.getLangOpts());
+StringRef WrittenName =
+Code.substr(FileIDAndOffset.second, IdentifierLength);
+return WrittenName == ND->getDeclName().getAsString();
   };
-
-  bool IsExplicit = !hasImplicitExpr(ASTNode.OrigE);
+  bool WrittenInCode = IsWrittenInCode(dyn_cast(D), Loc);
   // Find and add definition declarations (for GoToDefinition).
   // We don't use parameter `D`, as Parameter `D` is the canonical
   // declaration, which is the first declaration of a redeclarable
   // declaration, and it could be a forward declaration.
   if (const auto *Def = getDefinition(D)) {
-Decls[Def] |= IsExplicit;
+Decls[Def] |= WrittenInCode;
   } else {
 // Couldn't find a definition, fall back to use `D`.
-Decls[D] |= IsExplicit;
+Decls[D] |= WrittenInCode;
   }
 }
 return true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55124: [CodeComplete] Cleanup access checking in code completion

[Ilya Biryukov via Phabricator via cfe-commits]

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

- Do not introduce a new local var, reuse existing


Repository:
  rC Clang

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

https://reviews.llvm.org/D55124

Files:
  include/clang/Sema/Sema.h
  lib/Parse/ParseExprCXX.cpp
  lib/Sema/CodeCompleteConsumer.cpp
  lib/Sema/SemaAccess.cpp
  lib/Sema/SemaCodeComplete.cpp
  test/CodeCompletion/accessibility-crash.cpp
  test/CodeCompletion/accessibility.cpp

Index: test/CodeCompletion/accessibility.cpp
===
--- /dev/null
+++ test/CodeCompletion/accessibility.cpp
@@ -0,0 +1,73 @@
+class X {
+public:
+ int pub;
+protected:
+ int prot;
+private:
+ int priv;
+};
+
+class Unrelated {
+public:
+  static int pub;
+protected:
+  static int prot;
+private:
+  static int priv;
+};
+
+class Y : public X {
+  int test() {
+this->pub = 10;
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:21:11 %s -o - \
+// RUN: | FileCheck -check-prefix=THIS %s
+// THIS: priv (InBase,Inaccessible)
+// THIS: prot (InBase)
+// THIS: pub (InBase)
+//
+// Also check implicit 'this->', i.e. complete at the start of the line.
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:21:1 %s -o - \
+// RUN: | FileCheck -check-prefix=THIS %s
+
+X().pub + 10;
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:32:9 %s -o - \
+// RUN: | FileCheck -check-prefix=X-OBJ %s
+// X-OBJ: priv (Inaccessible)
+// X-OBJ: prot (Inaccessible)
+// X-OBJ: pub : [#int#]pub
+
+Y().pub + 10;
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:39:9 %s -o - \
+// RUN: | FileCheck -check-prefix=Y-OBJ %s
+// Y-OBJ: priv (InBase,Inaccessible)
+// Y-OBJ: prot (InBase)
+// Y-OBJ: pub (InBase)
+
+this->X::pub = 10;
+X::pub = 10;
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:46:14 %s -o - \
+// RUN: | FileCheck -check-prefix=THIS-BASE %s
+//
+// THIS-BASE: priv (Inaccessible)
+// THIS-BASE: prot : [#int#]prot
+// THIS-BASE: pub : [#int#]pub
+//
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:47:8 %s -o - \
+// RUN: | FileCheck -check-prefix=THIS-BASE %s
+
+
+this->Unrelated::pub = 10; // a check we don't crash in this cases.
+Y().Unrelated::pub = 10; // a check we don't crash in this cases.
+Unrelated::pub = 10;
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:59:22 %s -o - \
+// RUN: | FileCheck -check-prefix=UNRELATED %s
+// UNRELATED: priv (Inaccessible)
+// UNRELATED: prot (Inaccessible)
+// UNRELATED: pub : [#int#]pub
+//
+// RUN: not %clang_cc1 -fsyntax-only -code-completion-at=%s:60:20 %s -o - \
+// RUN: | FileCheck -check-prefix=UNRELATED %s
+// RUN: not %clang_cc1 -fsyntax-only -code-completion-at=%s:61:16 %s -o - \
+// RUN: | FileCheck -check-prefix=UNRELATED %s
+  }
+};
Index: test/CodeCompletion/accessibility-crash.cpp
===
--- /dev/null
+++ test/CodeCompletion/accessibility-crash.cpp
@@ -0,0 +1,23 @@
+class X {
+public:
+ int pub;
+protected:
+ int prot;
+private:
+ int priv;
+};
+
+class Y : public X {
+  int test() {
+[]() {
+
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:13:1 %s -o - \
+  // RUN: | FileCheck %s
+  // CHECK: priv (InBase,Inaccessible)
+  // CHECK: prot (InBase)
+  // CHECK: pub (InBase)
+};
+  }
+};
+
+
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -1278,38 +1278,53 @@
 }
 
 namespace {
+
 /// Visible declaration consumer that adds a code-completion result
 /// for each visible declaration.
 class CodeCompletionDeclConsumer : public VisibleDeclConsumer {
   ResultBuilder &Results;
-  DeclContext *CurContext;
+  DeclContext *InitialLookupCtx;
+  // NamingClass and BaseType are used for access-checking. See
+  // Sema::IsSimplyAccessible for details.
+  CXXRecordDecl *NamingClass;
+  QualType BaseType;
   std::vector FixIts;
-  // This is set to the record where the search starts, if this is a record
-  // member completion.
-  RecordDecl *MemberCompletionRecord = nullptr;
 
 public:
   CodeCompletionDeclConsumer(
-  ResultBuilder &Results, DeclContext *CurContext,
-  std::vector FixIts = std::vector(),
-  RecordDecl *MemberCompletionRecord = nullptr)
-  : Results(Results), CurContext(CurContext), FixIts(std::move(FixIts)),
-MemberCompletionRecord(MemberCompletionRecord) {}
+  ResultBuilder &Results, DeclContext *InitialLookupCtx,
+  QualType BaseType = QualType(),
+  std::vector FixIts = std::vector())
+  : Results(Results), InitialLookupCtx(InitialLookupCtx),
+FixIts(std::move(Fix

[PATCH] D55124: [CodeComplete] Cleanup access checking in code completion

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: lib/Parse/ParseExprCXX.cpp:248
   // seen a leading '::' or part of a nested-name-specifier.
+  ParsedType ObjectTypeForCompletion = ObjectType;
   ObjectType = nullptr;

kadircet wrote:
> What about first checking for whether we are on code completion point and 
> then assigning nullptr to objecttype?
> 
> Also it is not in the scope of this patch but I was wondering why we only 
> trigger completion if we've seen a scope specifier, do you have any idea?
Done. This got me thinking that we probably won't pass the object type on more 
than 1 qualifiers, e.g. `X().Foo::Bar::baz().`  I'll test and address this in a 
separate patch, though.

> Also it is not in the scope of this patch but I was wondering why we only 
> trigger completion if we've seen a scope specifier, do you have any idea?
I haven't looked closely into the code, but I suspect other cases are handled 
elsewhere, e.g. the case with no qualifiers is handled when parsing member 
expressions, etc.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55124



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

[PATCH] D55136: [OpenCL][Sema] Improve BuildResolvedCallExpr handling of builtins

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: lib/Sema/SemaExpr.cpp:5556
 // Extract the return type from the (builtin) function pointer type.
-auto FnPtrTy = Context.getPointerType(FDecl->getType());
+// FIXME Several builtins still have setType in 
Sema::CheckBuiltinFunctionCall.
+//   One should review their definitions in Builtins.def to ensure they

riccibruno wrote:
> This goes over 80 cols.
I think the format is normally
  // FIXME: text...
  // more text...
No extra spaces on the next line. :)



Repository:
  rC Clang

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

https://reviews.llvm.org/D55136



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

[PATCH] D54945: This commit adds a chapter about clang-tidy integrations

[Marina Kalashina via Phabricator via cfe-commits]

MarinaKalashina updated this revision to Diff 176328.
MarinaKalashina added a comment.

Fixes:

- empty line before 'Standalone tool'
- table columns with '+/-' aligned
- line width limited to 80 (except for the table)

Additions:

- clang-tidy-vs plugin
- Clangd in the intro, the table, and CLion's paragraph


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

https://reviews.llvm.org/D54945

Files:
  docs/clang-tidy/index.rst

Index: docs/clang-tidy/index.rst
===
--- docs/clang-tidy/index.rst
+++ docs/clang-tidy/index.rst
@@ -21,9 +21,6 @@
 Using clang-tidy
 
 
-Standalone tool

-
 :program:`clang-tidy` is a `LibTooling`_-based tool, and it's easier to work
 with if you set up a compile command database for your project (for an example
 of how to do this see `How To Setup Tooling For LLVM`_). You can also specify
@@ -258,125 +255,6 @@
   value:   'some value'
   ...
 
-
-Clang-tidy integrated
--
-
-.. _Clangd: https://clang.llvm.org/extra/clangd.html
-
-Apart from being a standalone tool, :program:`clang-tidy` is integrated into
-various IDEs, code analyzers, and editors. Besides, it is currently being
-integrated into Clangd_. The following table shows the most
-well-known :program:`clang-tidy` integrations in detail.
-
-+--++-+--+-+--+
-|  |Feature   |
-+==++=+==+=+==+
-|  **Tool**| On-the-fly inspection  | Check list configuration (GUI)  | Options to checks (GUI)  | Configuration via ``.clang-tidy`` files | Custom clang-tidy binary |
-+--++-+--+-+--+
-|A.L.E. for Vim| \+\|   \-\   |   \-\| \-\ |   \+\|
-+--++-+--+-+--+
-|Clang Power Tools for Visual Studio   | \-\|   \+\   |   \-\| \+\ |   \-\|
-+--++-+--+-+--+
-|Clangd| \+\|   \-\   |   \-\| \-\ |   \-\|
-+--++-+--+-+--+
-|CLion IDE | \+\|   \+\   |   \+\| \+\ |   \+\|
-+--++-+--+-+--+
-|CodeChecker   | \-\|   \-\   |   \-\| \-\ |   \+\|
-+--++-+--+-+--+
-|CPPCheck  | \-\|   \-\   |   \-\| \-\ |   \-\|
-+--++-+--+-+--+
-|CPPDepend | \-\|   \-\   |   \-\| \-\ |   \-\|
-+--+--

[PATCH] D55121: Make several Python scripts portable across Python2 and Python 3

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 176330.
serge-sans-paille edited the summary of this revision.

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

https://reviews.llvm.org/D55121

Files:
  bindings/python/clang/cindex.py
  docs/tools/dump_format_style.py
  tools/scan-view/share/Reporter.py
  tools/scan-view/share/ScanView.py
  utils/ABITest/ABITestGen.py
  utils/ABITest/TypeGen.py
  utils/analyzer/CmpRuns.py
  utils/modfuzz.py
  utils/token-delta.py

Index: utils/token-delta.py
===
--- utils/token-delta.py
+++ utils/token-delta.py
@@ -94,7 +94,7 @@
 
 ###
 
-class Token:
+class Token(object):
 def __init__(self, type, data, flags, file, line, column):
 self.type   = type
 self.data   = data
Index: utils/modfuzz.py
===
--- utils/modfuzz.py
+++ utils/modfuzz.py
@@ -12,7 +12,7 @@
 clang = sys.argv[1]
 none_opts = 0.3
 
-class Decl:
+class Decl(object):
   def __init__(self, text, depends=[], provides=[], conflicts=[]):
 self.text = text
 self.depends = depends
@@ -39,7 +39,7 @@
   Decl('X %(name)s;\n', depends=['X']),
 ]
 
-class FS:
+class FS(object):
   def __init__(self):
 self.fs = {}
 self.prevfs = {}
@@ -62,7 +62,7 @@
 
 fs = FS()
 
-class CodeModel:
+class CodeModel(object):
   def __init__(self):
 self.source = ''
 self.modules = {}
Index: utils/analyzer/CmpRuns.py
===
--- utils/analyzer/CmpRuns.py
+++ utils/analyzer/CmpRuns.py
@@ -38,7 +38,7 @@
 
 STATS_REGEXP = re.compile(r"Statistics: (\{.+\})", re.MULTILINE | re.DOTALL)
 
-class Colors:
+class Colors(object):
 """
 Color for terminal highlight.
 """
@@ -50,14 +50,14 @@
 # path - the analysis output directory
 # root - the name of the root directory, which will be disregarded when
 # determining the source file name
-class SingleRunInfo:
+class SingleRunInfo(object):
 def __init__(self, path, root="", verboseLog=None):
 self.path = path
 self.root = root.rstrip("/\\")
 self.verboseLog = verboseLog
 
 
-class AnalysisDiagnostic:
+class AnalysisDiagnostic(object):
 def __init__(self, data, report, htmlReport):
 self._data = data
 self._loc = self._data['location']
@@ -117,14 +117,14 @@
 return self._data
 
 
-class AnalysisReport:
+class AnalysisReport(object):
 def __init__(self, run, files):
 self.run = run
 self.files = files
 self.diagnostics = []
 
 
-class AnalysisRun:
+class AnalysisRun(object):
 def __init__(self, info):
 self.path = info.path
 self.root = info.root
Index: utils/ABITest/TypeGen.py
===
--- utils/ABITest/TypeGen.py
+++ utils/ABITest/TypeGen.py
@@ -17,7 +17,7 @@
 ###
 # Actual type types
 
-class Type:
+class Type(object):
 def isBitField(self):
 return False
 
Index: utils/ABITest/ABITestGen.py
===
--- utils/ABITest/ABITestGen.py
+++ utils/ABITest/ABITestGen.py
@@ -10,7 +10,7 @@
 
 
 
-class TypePrinter:
+class TypePrinter(object):
 def __init__(self, output, outputHeader=None, 
  outputTests=None, outputDriver=None,
  headerName=None, info=None):
Index: tools/scan-view/share/ScanView.py
===
--- tools/scan-view/share/ScanView.py
+++ tools/scan-view/share/ScanView.py
@@ -422,7 +422,7 @@
 return self.send_string(res, 'text/plain')
 
 def get_report_context(self, report):
-class Context:
+class Context(object):
 pass
 if report is None or report == 'None':
 data = self.load_crashes()
Index: tools/scan-view/share/Reporter.py
===
--- tools/scan-view/share/Reporter.py
+++ tools/scan-view/share/Reporter.py
@@ -16,7 +16,7 @@
 
 # Collect information about a bug.
 
-class BugReport:
+class BugReport(object):
 def __init__(self, title, description, files):
 self.title = title
 self.description = description
@@ -37,7 +37,7 @@
 # ReporterParameter
 #======#
 
-class ReporterParameter:
+class ReporterParameter(object):
   def __init__(self, n):
 self.name = n
   def getName(self):
@@ -75,7 +75,7 @@
 # Reporters
 #======#
 
-class EmailReporter:
+class EmailReporter(object):
 def getName(self):
 return 'Email'
 
@@ -143,7 +143,7 @@
 
 return "Message sent!"
 
-class BugzillaReporter:
+class BugzillaReporter(object):
 def getName(self):
 return 'Bugzilla'
 
@@ -174,7 +174,7 @@
 else:
   return '7'

[PATCH] D55128: [CMake] Store path to vendor-specific headers in clang-headers target property

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL348116: [CMake] Store path to vendor-specific headers in 
clang-headers target property (authored by stefan.graenitz, committed by ).

Repository:
  rL LLVM

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

https://reviews.llvm.org/D55128

Files:
  cfe/trunk/lib/Headers/CMakeLists.txt


Index: cfe/trunk/lib/Headers/CMakeLists.txt
===
--- cfe/trunk/lib/Headers/CMakeLists.txt
+++ cfe/trunk/lib/Headers/CMakeLists.txt
@@ -144,7 +144,7 @@
   list(APPEND out_files ${dst})
 endforeach( f )
 
-add_custom_command(OUTPUT ${output_dir}/arm_neon.h 
+add_custom_command(OUTPUT ${output_dir}/arm_neon.h
   DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/arm_neon.h
   COMMAND ${CMAKE_COMMAND} -E copy_if_different 
${CMAKE_CURRENT_BINARY_DIR}/arm_neon.h ${output_dir}/arm_neon.h
   COMMENT "Copying clang's arm_neon.h...")
@@ -156,7 +156,9 @@
 list(APPEND out_files ${output_dir}/arm_fp16.h)
 
 add_custom_target(clang-headers ALL DEPENDS ${out_files})
-set_target_properties(clang-headers PROPERTIES FOLDER "Misc")
+set_target_properties(clang-headers PROPERTIES
+  FOLDER "Misc"
+  RUNTIME_OUTPUT_DIRECTORY "${output_dir}")
 
 install(
   FILES ${files} ${CMAKE_CURRENT_BINARY_DIR}/arm_neon.h


Index: cfe/trunk/lib/Headers/CMakeLists.txt
===
--- cfe/trunk/lib/Headers/CMakeLists.txt
+++ cfe/trunk/lib/Headers/CMakeLists.txt
@@ -144,7 +144,7 @@
   list(APPEND out_files ${dst})
 endforeach( f )
 
-add_custom_command(OUTPUT ${output_dir}/arm_neon.h 
+add_custom_command(OUTPUT ${output_dir}/arm_neon.h
   DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/arm_neon.h
   COMMAND ${CMAKE_COMMAND} -E copy_if_different ${CMAKE_CURRENT_BINARY_DIR}/arm_neon.h ${output_dir}/arm_neon.h
   COMMENT "Copying clang's arm_neon.h...")
@@ -156,7 +156,9 @@
 list(APPEND out_files ${output_dir}/arm_fp16.h)
 
 add_custom_target(clang-headers ALL DEPENDS ${out_files})
-set_target_properties(clang-headers PROPERTIES FOLDER "Misc")
+set_target_properties(clang-headers PROPERTIES
+  FOLDER "Misc"
+  RUNTIME_OUTPUT_DIRECTORY "${output_dir}")
 
 install(
   FILES ${files} ${CMAKE_CURRENT_BINARY_DIR}/arm_neon.h
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55139: [clangd] Avoid memory-mapping files on Windows

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov planned changes to this revision.
ilya-biryukov added a comment.

We need to keep mmapping the PCH files.

In D55139#1315944 , @malaperle wrote:

> Hi Ilya. Does this apply to compile_commands.json too? I've seen that problem 
> for that file as well. If not, I understand it can be another patch, just 
> curious.


Yeah, this applies to all files, including `compiler_commands.json`.
I've discussed this with a few smart folks and they pointed out doing it for 
**all** files is not ok, most problematic are PCH files, which are (1) huge, 
(2) produced by clangd itself and never modified after being produced. 
(1) means it's very wasteful to fully load them into memory, (2) means it can 
easily be avoided. That complicated things, though, and the current approach to 
fix this in libclang D54995  (passing an 
explicit flag when we know the file should be memory-mapped) starts to look 
more attractive.
I'll patch it up in this patch by looking at file name and avoiding that for 
PCH files. There are still PCM files, which are very similar, my hopes are 
they're not widely used on Windows these days so we should be fine for now, but 
that would definitely bite us in the future.

> Please fix this also for libclang clients. I think it's safe to assume that 
> files might be edited once CXTranslationUnit_PrecompiledPreamble or 
> CXTranslationUnit_CacheCompletionResults is set as flag - that's what 
> clang_defaultEditingTranslationUnitOptions() returns.

libclang clients should be covered by D54995  
and previous changes around it. As I mentioned in the previous comment, passing 
a flag when file cannot be memory-mapped seems like a proper solution anyway.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55139



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

[PATCH] D55206: [clangd] Get rid of AST matchers in CodeComplete, NFC

[Haojian Wu via Phabricator via cfe-commits]

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

The isIndexedForCodeCompletion is called in the code patch of
SymbolCollector.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D55206

Files:
  clangd/CodeComplete.cpp


Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -37,7 +37,6 @@
 #include "index/Index.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
-#include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Format/Format.h"
@@ -1645,14 +1644,24 @@
 }
 
 bool isIndexedForCodeCompletion(const NamedDecl &ND, ASTContext &ASTCtx) {
-  using namespace clang::ast_matchers;
-  auto InTopLevelScope = hasDeclContext(
-  anyOf(namespaceDecl(), translationUnitDecl(), linkageSpecDecl()));
-  return !match(decl(anyOf(InTopLevelScope,
-   hasDeclContext(
-   enumDecl(InTopLevelScope, 
unless(isScoped()),
-ND, ASTCtx)
-  .empty();
+  auto InTopLevelScope = [](const NamedDecl &ND) {
+switch (ND.getDeclContext()->getDeclKind()) {
+case Decl::TranslationUnit:
+case Decl::Namespace:
+case Decl::LinkageSpec:
+  return true;
+default:
+  return false;
+};
+return false;
+  };
+  if (InTopLevelScope(ND))
+return true;
+
+  if (const auto *EnumDecl = dyn_cast(ND.getDeclContext()))
+return InTopLevelScope(*EnumDecl) && !EnumDecl->isScoped();
+
+  return false;
 }
 
 CompletionItem CodeCompletion::render(const CodeCompleteOptions &Opts) const {


Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -37,7 +37,6 @@
 #include "index/Index.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
-#include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Format/Format.h"
@@ -1645,14 +1644,24 @@
 }
 
 bool isIndexedForCodeCompletion(const NamedDecl &ND, ASTContext &ASTCtx) {
-  using namespace clang::ast_matchers;
-  auto InTopLevelScope = hasDeclContext(
-  anyOf(namespaceDecl(), translationUnitDecl(), linkageSpecDecl()));
-  return !match(decl(anyOf(InTopLevelScope,
-   hasDeclContext(
-   enumDecl(InTopLevelScope, unless(isScoped()),
-ND, ASTCtx)
-  .empty();
+  auto InTopLevelScope = [](const NamedDecl &ND) {
+switch (ND.getDeclContext()->getDeclKind()) {
+case Decl::TranslationUnit:
+case Decl::Namespace:
+case Decl::LinkageSpec:
+  return true;
+default:
+  return false;
+};
+return false;
+  };
+  if (InTopLevelScope(ND))
+return true;
+
+  if (const auto *EnumDecl = dyn_cast(ND.getDeclContext()))
+return InTopLevelScope(*EnumDecl) && !EnumDecl->isScoped();
+
+  return false;
 }
 
 CompletionItem CodeCompletion::render(const CodeCompleteOptions &Opts) const {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55127: [OpenCL] Diagnose conflicting address spaces between template definition and its instantiation

[Anastasia Stulova via Phabricator via cfe-commits]

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



Comment at: lib/Sema/SemaType.cpp:7232
+  if (D.getContext() == DeclaratorContext::TemplateArgContext)
+// Do not deduce address space for non-pointee type in template arg.
+;

rjmccall wrote:
> I don't understand what you're trying to do here.  Template arguments may 
> need to be directly qualified with an address-space sometimes, and if you 
> prevent that unconditionally you're going to break all sorts of things, like 
> partially-specializing a template based on the presence of an address-space 
> qualifier.
I want to prevent deduction of address spaces for template arguments (when they 
are not specified explicitly).

Without this change, this example won't compile:
  template 
  void foo() {
static __global T i;
  }
  foo(); // error because int here is deduced to __private int (so i will 
have conflicting addr space quals)
But I think it's perfectly reasonable to compile this example because the addr 
space qual of `i` is specified to be `__global`.

Basically I am just trying to fix OpenCL C deduction rules that didn't account 
for the logic of templates.



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

https://reviews.llvm.org/D55127



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

r348120 - [OpenCL][Sema] Improve BuildResolvedCallExpr handling of builtins

[Marco Antognini via cfe-commits]

Author: mantognini
Date: Mon Dec  3 02:58:56 2018
New Revision: 348120

URL: http://llvm.org/viewvc/llvm-project?rev=348120&view=rev
Log:
[OpenCL][Sema] Improve BuildResolvedCallExpr handling of builtins

Summary:
This is a follow-up on https://reviews.llvm.org/D52879, addressing a few issues.

This:
 - adds a FIXME for later improvement for specific builtins: I previously have 
only checked OpenCL ones and ensured tests cover those.
 - fixed the CallExpr type.



Reviewers: riccibruno

Reviewed By: riccibruno

Subscribers: yaxunl, Anastasia, kristina, svenvh, cfe-commits

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

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

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=348120&r1=348119&r2=348120&view=diff
==
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Mon Dec  3 02:58:56 2018
@@ -5562,17 +5562,20 @@ Sema::BuildResolvedCallExpr(Expr *Fn, Na
   // We special-case function promotion here because we only allow promoting
   // builtin functions to function pointers in the callee of a call.
   ExprResult Result;
-  QualType ReturnTy;
+  QualType ResultTy;
   if (BuiltinID &&
   Fn->getType()->isSpecificBuiltinType(BuiltinType::BuiltinFn)) {
 // Extract the return type from the (builtin) function pointer type.
-auto FnPtrTy = Context.getPointerType(FDecl->getType());
+// FIXME Several builtins still have setType in
+//   Sema::CheckBuiltinFunctionCall. One should review their
+//   definitions in Builtins.def to ensure they are correct before
+//   removing setType calls.
+QualType FnPtrTy = Context.getPointerType(FDecl->getType());
 Result = ImpCastExprToType(Fn, FnPtrTy, CK_BuiltinFnToFnPtr).get();
-auto FnTy = FnPtrTy->getPointeeType()->castAs();
-ReturnTy = FnTy->getReturnType();
+ResultTy = FDecl->getCallResultType();
   } else {
 Result = CallExprUnaryConversions(Fn);
-ReturnTy = Context.BoolTy;
+ResultTy = Context.BoolTy;
   }
   if (Result.isInvalid())
 return ExprError();
@@ -5584,10 +5587,10 @@ Sema::BuildResolvedCallExpr(Expr *Fn, Na
   if (Config)
 TheCall =
 new (Context) CUDAKernelCallExpr(Context, Fn, cast(Config),
- Args, ReturnTy, VK_RValue, RParenLoc);
+ Args, ResultTy, VK_RValue, RParenLoc);
   else
 TheCall = new (Context)
-CallExpr(Context, Fn, Args, ReturnTy, VK_RValue, RParenLoc);
+CallExpr(Context, Fn, Args, ResultTy, VK_RValue, RParenLoc);
 
   if (!getLangOpts().CPlusPlus) {
 // C cannot always handle TypoExpr nodes in builtin calls and direct


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

[PATCH] D55136: [OpenCL][Sema] Improve BuildResolvedCallExpr handling of builtins

[Marco Antognini via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL348120: [OpenCL][Sema] Improve BuildResolvedCallExpr 
handling of builtins (authored by mantognini, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D55136?vs=176157&id=176353#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D55136

Files:
  cfe/trunk/lib/Sema/SemaExpr.cpp


Index: cfe/trunk/lib/Sema/SemaExpr.cpp
===
--- cfe/trunk/lib/Sema/SemaExpr.cpp
+++ cfe/trunk/lib/Sema/SemaExpr.cpp
@@ -5562,17 +5562,20 @@
   // We special-case function promotion here because we only allow promoting
   // builtin functions to function pointers in the callee of a call.
   ExprResult Result;
-  QualType ReturnTy;
+  QualType ResultTy;
   if (BuiltinID &&
   Fn->getType()->isSpecificBuiltinType(BuiltinType::BuiltinFn)) {
 // Extract the return type from the (builtin) function pointer type.
-auto FnPtrTy = Context.getPointerType(FDecl->getType());
+// FIXME Several builtins still have setType in
+//   Sema::CheckBuiltinFunctionCall. One should review their
+//   definitions in Builtins.def to ensure they are correct before
+//   removing setType calls.
+QualType FnPtrTy = Context.getPointerType(FDecl->getType());
 Result = ImpCastExprToType(Fn, FnPtrTy, CK_BuiltinFnToFnPtr).get();
-auto FnTy = FnPtrTy->getPointeeType()->castAs();
-ReturnTy = FnTy->getReturnType();
+ResultTy = FDecl->getCallResultType();
   } else {
 Result = CallExprUnaryConversions(Fn);
-ReturnTy = Context.BoolTy;
+ResultTy = Context.BoolTy;
   }
   if (Result.isInvalid())
 return ExprError();
@@ -5584,10 +5587,10 @@
   if (Config)
 TheCall =
 new (Context) CUDAKernelCallExpr(Context, Fn, cast(Config),
- Args, ReturnTy, VK_RValue, RParenLoc);
+ Args, ResultTy, VK_RValue, RParenLoc);
   else
 TheCall = new (Context)
-CallExpr(Context, Fn, Args, ReturnTy, VK_RValue, RParenLoc);
+CallExpr(Context, Fn, Args, ResultTy, VK_RValue, RParenLoc);
 
   if (!getLangOpts().CPlusPlus) {
 // C cannot always handle TypoExpr nodes in builtin calls and direct


Index: cfe/trunk/lib/Sema/SemaExpr.cpp
===
--- cfe/trunk/lib/Sema/SemaExpr.cpp
+++ cfe/trunk/lib/Sema/SemaExpr.cpp
@@ -5562,17 +5562,20 @@
   // We special-case function promotion here because we only allow promoting
   // builtin functions to function pointers in the callee of a call.
   ExprResult Result;
-  QualType ReturnTy;
+  QualType ResultTy;
   if (BuiltinID &&
   Fn->getType()->isSpecificBuiltinType(BuiltinType::BuiltinFn)) {
 // Extract the return type from the (builtin) function pointer type.
-auto FnPtrTy = Context.getPointerType(FDecl->getType());
+// FIXME Several builtins still have setType in
+//   Sema::CheckBuiltinFunctionCall. One should review their
+//   definitions in Builtins.def to ensure they are correct before
+//   removing setType calls.
+QualType FnPtrTy = Context.getPointerType(FDecl->getType());
 Result = ImpCastExprToType(Fn, FnPtrTy, CK_BuiltinFnToFnPtr).get();
-auto FnTy = FnPtrTy->getPointeeType()->castAs();
-ReturnTy = FnTy->getReturnType();
+ResultTy = FDecl->getCallResultType();
   } else {
 Result = CallExprUnaryConversions(Fn);
-ReturnTy = Context.BoolTy;
+ResultTy = Context.BoolTy;
   }
   if (Result.isInvalid())
 return ExprError();
@@ -5584,10 +5587,10 @@
   if (Config)
 TheCall =
 new (Context) CUDAKernelCallExpr(Context, Fn, cast(Config),
- Args, ReturnTy, VK_RValue, RParenLoc);
+ Args, ResultTy, VK_RValue, RParenLoc);
   else
 TheCall = new (Context)
-CallExpr(Context, Fn, Args, ReturnTy, VK_RValue, RParenLoc);
+CallExpr(Context, Fn, Args, ResultTy, VK_RValue, RParenLoc);
 
   if (!getLangOpts().CPlusPlus) {
 // C cannot always handle TypoExpr nodes in builtin calls and direct
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55206: [clangd] Get rid of AST matchers in CodeComplete, NFC

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clangd/CodeComplete.cpp:1654
+default:
+  return false;
+};

nit: maybe just `break` in here


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55206



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

[PATCH] D55206: [clangd] Get rid of AST matchers in CodeComplete, NFC

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

LG


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55206



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

[PATCH] D54995: [MemoryBuffer] Add the setter to be able to force disabled mmap

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

It seems there are still cases where memory-mapping should be preferred, even 
on Windows, specifically:

- PCH files produced by libclang: they are huge, loading them in memory is 
wasteful and they can clearly be used
- (maybe?) PCM (precompiled module) files: they are huge, however can be 
produced by the user's buildsystem. Locking them might be bad if we don't 
control the buildsystem, but the memory savings are attractive.
- other binary input files?

At least the PCM files seem to be low-hanging fruits, we should try to take 
advantage of them.




Comment at: lib/Support/MemoryBuffer.cpp:42
 
+static bool MemoryMappingEnabled = true;
+

yvvan wrote:
> lebedev.ri wrote:
> > Such global flags are a bad idea in general, and really not great in LLVM's 
> > case.
> > The editor would set it for "it's" llvm, but that will also affect the LLVM 
> > that is used by e.g. mesa.
> > 
> Oh no, don't mention mesa. The proper client should never share it's LLVM 
> layer with mesa. We already got issues with versions incompatibility and the 
> only good solution is to link llvm statically inside client. Otherwise mesa 
> causes the mess anyways.
> 
> Also I expect this setter to be used only on Windows.
> 
> Of course there's another solution is that Ilya proposed but it's only for 
> clangd there and is bigger in code size. And it can still allow bugs if 
> somebody uses MemoryBuffer not through the FileSystem class.
+1, please don't add global flags, there are lots of reasons to avoid them.
Moreover, adding this without removing the current approach (isVolatile) is 
twice as bad - we now have many ways to indicate the files cannot be 
memory-mapped.


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

https://reviews.llvm.org/D54995



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

[PATCH] D55062: [clangd] Partition include graph on auto-index.

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

- Rebase


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55062

Files:
  clangd/index/Background.cpp
  unittests/clangd/BackgroundIndexTests.cpp

Index: unittests/clangd/BackgroundIndexTests.cpp
===
--- unittests/clangd/BackgroundIndexTests.cpp
+++ unittests/clangd/BackgroundIndexTests.cpp
@@ -93,7 +93,7 @@
   Cmd.Filename = testPath("root/A.cc");
   Cmd.Directory = testPath("root");
   Cmd.CommandLine = {"clang++", "-DA=1", testPath("root/A.cc")};
-  CDB.setCompileCommand(testPath("root"), Cmd);
+  CDB.setCompileCommand(testPath("root/A.cc"), Cmd);
 
   ASSERT_TRUE(Idx.blockUntilIdleForTest());
   EXPECT_THAT(
@@ -103,7 +103,7 @@
 
   Cmd.Filename = testPath("root/B.cc");
   Cmd.CommandLine = {"clang++", Cmd.Filename};
-  CDB.setCompileCommand(testPath("root"), Cmd);
+  CDB.setCompileCommand(testPath("root/A.cc"), Cmd);
 
   ASSERT_TRUE(Idx.blockUntilIdleForTest());
   // B_CC is dropped as we don't collect symbols from A.h in this compilation.
@@ -143,7 +143,7 @@
 OverlayCDB CDB(/*Base=*/nullptr);
 BackgroundIndex Idx(Context::empty(), "", FS, CDB,
 [&](llvm::StringRef) { return &MSS; });
-CDB.setCompileCommand(testPath("root"), Cmd);
+CDB.setCompileCommand(testPath("root/A.cc"), Cmd);
 ASSERT_TRUE(Idx.blockUntilIdleForTest());
   }
   EXPECT_EQ(CacheHits, 0U);
@@ -165,5 +165,48 @@
   EXPECT_THAT(*ShardSource->Refs, RefsAre({FileURI("unittest:///root/A.cc")}));
 }
 
+TEST_F(BackgroundIndexTest, DirectIncludesTest) {
+  MockFSProvider FS;
+  FS.Files[testPath("root/B.h")] = "";
+  FS.Files[testPath("root/A.h")] = R"cpp(
+  #include "B.h"
+  void common();
+  void f_b();
+  class A_CC {};
+  )cpp";
+  std::string A_CC = "#include \"A.h\"\nvoid g() { (void)common; }";
+  FS.Files[testPath("root/A.cc")] = A_CC;
+
+  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")};
+  {
+OverlayCDB CDB(/*Base=*/nullptr);
+BackgroundIndex Idx(Context::empty(), "", FS, CDB,
+[&](llvm::StringRef) { return &MSS; });
+CDB.setCompileCommand(testPath("root/A.cc"), Cmd);
+ASSERT_TRUE(Idx.blockUntilIdleForTest());
+  }
+
+  auto ShardSource = MSS.loadShard(testPath("root/A.cc"));
+  EXPECT_TRUE(ShardSource->Sources);
+  EXPECT_EQ(ShardSource->Sources->size(), 2U); // A.cc, A.h
+  EXPECT_THAT(
+  ShardSource->Sources->lookup("unittest:///root/A.cc").DirectIncludes,
+  UnorderedElementsAre("unittest:///root/A.h"));
+
+  auto ShardHeader = MSS.loadShard(testPath("root/A.h"));
+  EXPECT_TRUE(ShardHeader->Sources);
+  EXPECT_EQ(ShardHeader->Sources->size(), 2U); // A.h B.h
+  EXPECT_THAT(
+  ShardHeader->Sources->lookup("unittest:///root/A.h").DirectIncludes,
+  UnorderedElementsAre("unittest:///root/B.h"));
+}
+
 } // namespace clangd
 } // namespace clang
Index: clangd/index/Background.cpp
===
--- clangd/index/Background.cpp
+++ clangd/index/Background.cpp
@@ -35,6 +35,59 @@
 using namespace llvm;
 namespace clang {
 namespace clangd {
+namespace {
+// Resolves URI to file paths with cache.
+class URIToFileCache {
+public:
+  URIToFileCache(llvm::StringRef HintPath) : HintPath(HintPath) {}
+
+  llvm::StringRef resolve(llvm::StringRef FileURI) {
+auto I = URIToPathCache.try_emplace(FileURI);
+if (I.second) {
+  auto U = URI::parse(FileURI);
+  if (!U) {
+elog("Failed to parse URI {0}: {1}", FileURI, U.takeError());
+assert(false && "Failed to parse URI");
+return "";
+  }
+  auto Path = URI::resolve(*U, HintPath);
+  if (!Path) {
+elog("Failed to resolve URI {0}: {1}", FileURI, Path.takeError());
+assert(false && "Failed to resolve URI");
+return "";
+  }
+  I.first->second = *Path;
+}
+return I.first->second;
+  }
+
+private:
+  std::string HintPath;
+  llvm::StringMap URIToPathCache;
+};
+
+// We keep only the node "Path" and its edges.
+IncludeGraph getSubGraph(const URI &U, const IncludeGraph &FullGraph) {
+  IncludeGraph IG;
+
+  std::string FileURI = U.toString();
+  auto Entry = IG.try_emplace(FileURI).first;
+  auto &Node = Entry->getValue();
+  Node = FullGraph.lookup(Entry->getKey());
+  Node.URI = Entry->getKey();
+
+  // Since the strings in direct includes are references to the keys of the
+  // fullgraph, we need to create entries in our new sub-graph for those
+  // includes and change references to point to the new graph's keys.
+  for (auto &Include : Node.DirectIncludes) {
+auto I = IG.try_emplace(Include).first;
+ 

r348123 - [clang] Do not read from 'test/SemaCXX/Inputs' inside 'test/AST'

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Mon Dec  3 03:26:35 2018
New Revision: 348123

URL: http://llvm.org/viewvc/llvm-project?rev=348123&view=rev
Log:
[clang] Do not read from 'test/SemaCXX/Inputs' inside 'test/AST'

Our integrate relies on test inputs being taken from the same diretory as the
test itself.

Added:
cfe/trunk/test/AST/Inputs/std-coroutine.h
Modified:
cfe/trunk/test/AST/coroutine-source-location-crash.cpp

Added: cfe/trunk/test/AST/Inputs/std-coroutine.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/AST/Inputs/std-coroutine.h?rev=348123&view=auto
==
--- cfe/trunk/test/AST/Inputs/std-coroutine.h (added)
+++ cfe/trunk/test/AST/Inputs/std-coroutine.h Mon Dec  3 03:26:35 2018
@@ -0,0 +1,37 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin9 %s -std=c++14 -fcoroutines-ts 
-fsyntax-only -Wignored-qualifiers -Wno-error=return-type -verify -fblocks 
-Wno-unreachable-code -Wno-unused-value
+#ifndef STD_COROUTINE_H
+#define STD_COROUTINE_H
+
+namespace std {
+namespace experimental {
+
+template 
+struct coroutine_traits { using promise_type = typename Ret::promise_type; };
+
+template 
+struct coroutine_handle {
+  static coroutine_handle from_address(void *);
+};
+template <>
+struct coroutine_handle {
+  template 
+  coroutine_handle(coroutine_handle);
+  static coroutine_handle from_address(void *);
+};
+
+struct suspend_always {
+  bool await_ready() { return false; }
+  void await_suspend(coroutine_handle<>) {}
+  void await_resume() {}
+};
+
+struct suspend_never {
+  bool await_ready() { return true; }
+  void await_suspend(coroutine_handle<>) {}
+  void await_resume() {}
+};
+
+} // namespace experimental
+} // namespace std
+
+#endif // STD_COROUTINE_H

Modified: cfe/trunk/test/AST/coroutine-source-location-crash.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/AST/coroutine-source-location-crash.cpp?rev=348123&r1=348122&r2=348123&view=diff
==
--- cfe/trunk/test/AST/coroutine-source-location-crash.cpp (original)
+++ cfe/trunk/test/AST/coroutine-source-location-crash.cpp Mon Dec  3 03:26:35 
2018
@@ -1,6 +1,6 @@
 // RUN: %clang_cc1 -triple x86_64-apple-darwin9 %s -std=c++14 -fcoroutines-ts \
 // RUN:-fsyntax-only -ast-dump | FileCheck %s
-#include "../SemaCXX/Inputs/std-coroutine.h"
+#include "Inputs/std-coroutine.h"
 
 using namespace std::experimental;
 


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

r348124 - [Analysis] Properly prepare test env in test/Analysis/undef-call.c

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Mon Dec  3 03:28:17 2018
New Revision: 348124

URL: http://llvm.org/viewvc/llvm-project?rev=348124&view=rev
Log:
[Analysis] Properly prepare test env in test/Analysis/undef-call.c

The test expectes the '%T/ctudir' to be present, but does not create it.

Modified:
cfe/trunk/test/Analysis/undef-call.c

Modified: cfe/trunk/test/Analysis/undef-call.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/undef-call.c?rev=348124&r1=348123&r2=348124&view=diff
==
--- cfe/trunk/test/Analysis/undef-call.c (original)
+++ cfe/trunk/test/Analysis/undef-call.c Mon Dec  3 03:28:17 2018
@@ -1,3 +1,5 @@
+// RUN: rm -rf %T/ctudir
+// RUN: mkdir %T/ctudir
 // RUN: %clang_cc1 -fsyntax-only -analyze 
-analyzer-checker=debug.ExprInspection -analyzer-config 
experimental-enable-naive-ctu-analysis=true -analyzer-config ctu-dir=%T/ctudir 
-verify %s
 // expected-no-diagnostics
 


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

r348125 - [Analyzer] Actually check for -model-path being a directory

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Mon Dec  3 03:34:08 2018
New Revision: 348125

URL: http://llvm.org/viewvc/llvm-project?rev=348125&view=rev
Log:
[Analyzer] Actually check for -model-path being a directory

The original patch (r348038) clearly contained a typo and checked
for '-ctu-dir' twice.

Modified:
cfe/trunk/lib/Frontend/CompilerInvocation.cpp

Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=348125&r1=348124&r2=348125&view=diff
==
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Mon Dec  3 03:34:08 2018
@@ -456,12 +456,13 @@ static void parseAnalyzerConfigs(Analyze
 return;
 
   if (!AnOpts.CTUDir.empty() && !llvm::sys::fs::is_directory(AnOpts.CTUDir))
-Diags->Report(diag::err_analyzer_config_invalid_input)
-  << "ctu-dir" << "a filename";
+Diags->Report(diag::err_analyzer_config_invalid_input) << "ctu-dir"
+   << "a filename";
 
-  if (!AnOpts.CTUDir.empty() && !llvm::sys::fs::is_directory(AnOpts.CTUDir))
-Diags->Report(diag::err_analyzer_config_invalid_input)
-  << "model-path" << "a filename";
+  if (!AnOpts.ModelPath.empty() &&
+  !llvm::sys::fs::is_directory(AnOpts.ModelPath))
+Diags->Report(diag::err_analyzer_config_invalid_input) << "model-path"
+   << "a filename";
 }
 
 static bool ParseMigratorArgs(MigratorOptions &Opts, ArgList &Args) {


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

[PATCH] D54995: [MemoryBuffer] Add the setter to be able to force disabled mmap

[Ivan Donchevskii via Phabricator via cfe-commits]

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

Ok, no global option.
Why not placing your VolatileFSProvider in clang so that libclang could you it 
too?


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

https://reviews.llvm.org/D54995



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

Re: r348038 - [analyzer] Emit an error for invalid -analyzer-config inputs

[Ilya Biryukov via cfe-commits]

Hi Kristóf,

There is clearly a typo in checking for -ctu-dir and -model-path being a
directory. Instead of checking -model-path, the code was checking for
-ctu-dir twice.
Landed a fix in r348125.

On Fri, Nov 30, 2018 at 10:27 PM Kristof Umann via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: szelethus
> Date: Fri Nov 30 13:24:31 2018
> New Revision: 348038
>
> URL: http://llvm.org/viewvc/llvm-project?rev=348038&view=rev
> Log:
> [analyzer] Emit an error for invalid -analyzer-config inputs
>
> Differential Revision: https://reviews.llvm.org/D53280
>
> Added:
> cfe/trunk/test/Analysis/invalid-analyzer-config-value.c
> Modified:
> cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
> cfe/trunk/include/clang/Driver/CC1Options.td
> cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
> cfe/trunk/lib/Driver/ToolChains/Clang.cpp
> cfe/trunk/lib/Frontend/CompilerInvocation.cpp
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td?rev=348038&r1=348037&r2=348038&view=diff
>
> ==
> --- cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td Fri Nov 30
> 13:24:31 2018
> @@ -299,6 +299,9 @@ def err_analyzer_config_no_value : Error
>"analyzer-config option '%0' has a key but no value">;
>  def err_analyzer_config_multiple_values : Error<
>"analyzer-config option '%0' should contain only one '='">;
> +def err_analyzer_config_invalid_input : Error<
> +  "invalid input for analyzer-config option '%0', that expects %1 value">;
> +def err_analyzer_config_unknown : Error<"unknown analyzer-config '%0'">;
>
>  def err_drv_invalid_hvx_length : Error<
>"-mhvx-length is not supported without a -mhvx/-mhvx= flag">;
>
> Modified: cfe/trunk/include/clang/Driver/CC1Options.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/CC1Options.td?rev=348038&r1=348037&r2=348038&view=diff
>
> ==
> --- cfe/trunk/include/clang/Driver/CC1Options.td (original)
> +++ cfe/trunk/include/clang/Driver/CC1Options.td Fri Nov 30 13:24:31 2018
> @@ -138,6 +138,12 @@ def analyzer_list_enabled_checkers : Fla
>  def analyzer_config : Separate<["-"], "analyzer-config">,
>HelpText<"Choose analyzer options to enable">;
>
> +def analyzer_config_compatibility_mode : Separate<["-"],
> "analyzer-config-compatibility-mode">,
> +  HelpText<"Don't emit errors on invalid analyzer-config inputs">;
> +
> +def analyzer_config_compatibility_mode_EQ : Joined<["-"],
> "analyzer-config-compatibility-mode=">,
> +  Alias;
> +
>
>  
> //===--===//
>  // Migrator Options
>
>  
> //===--===//
>
> Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h?rev=348038&r1=348037&r2=348038&view=diff
>
> ==
> --- cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
> (original)
> +++ cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h Fri Nov
> 30 13:24:31 2018
> @@ -200,6 +200,7 @@ public:
>unsigned ShowCheckerHelp : 1;
>unsigned ShowEnabledCheckerList : 1;
>unsigned ShowConfigOptionsList : 1;
> +  unsigned ShouldEmitErrorsOnInvalidConfigValue : 1;
>unsigned AnalyzeAll : 1;
>unsigned AnalyzerDisplayProgress : 1;
>unsigned AnalyzeNestedBlocks : 1;
> @@ -222,6 +223,7 @@ public:
>/// The mode of function selection used during inlining.
>AnalysisInliningMode InliningMode = NoRedundancy;
>
> +  // Create a field for each -analyzer-config option.
>  #define ANALYZER_OPTION_DEPENDS_ON_USER_MODE(TYPE, NAME, CMDFLAG, DESC,
>   \
>   SHALLOW_VAL, DEEP_VAL)
>   \
>ANALYZER_OPTION(TYPE, NAME, CMDFLAG, DESC, SHALLOW_VAL)
> @@ -233,13 +235,39 @@ public:
>  #undef ANALYZER_OPTION
>  #undef ANALYZER_OPTION_DEPENDS_ON_USER_MODE
>
> +  // Create an array of all -analyzer-config command line options. Sort
> it in
> +  // the constructor.
> +  std::vector AnalyzerConfigCmdFlags = {
> +#define ANALYZER_OPTION_DEPENDS_ON_USER_MODE(TYPE, NAME, CMDFLAG, DESC,
>   \
> + SHALLOW_VAL, DEEP_VAL)
>   \
> +  ANALYZER_OPTION(TYPE, NAME, CMDFLAG, DESC, SHALLOW_VAL)
> +
> +#define ANALYZER_OPTION(TYPE, NAME, CMDFLAG, DESC, DEFAULT_VAL)
>   \
> +CMDFLAG,
> +
> +#include "clang/StaticAnalyzer/Core/AnalyzerOptions.def"
> +#undef ANALYZER_OPTION
> +#undef ANALYZER_OPTION_DEPENDS_ON_USER_MODE
> +  };
> +
> +  bo

[PATCH] D54995: [MemoryBuffer] Add the setter to be able to force disabled mmap

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In D54995#1316437 , @yvvan wrote:

> Ok, no global option.
>  Why not placing your VolatileFSProvider in clang so that libclang could you 
> it too?


Would be happy to, will need to figure out what to do with PCH and PCM files 
first. However if we do this on clang level, I believe we should remove the 
`isVolatile` flag from the VFS interfaces in the first place.
It would be nice to not loose out on the opportunity to avoid fully loading the 
PCH files, but that obviously requires passing some flags into the VFS 
implementation or various hacks (matching on filenames/extensions?) to find out 
which files are PCHs. 
I actually don't know which approach to choose: on one hand, I'd really want to 
get rid of the isVolatile flag, on the other hand I'd really want to avoid 
loading large binary files into memory and that requires passing the flags.


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

https://reviews.llvm.org/D54995



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

[PATCH] D54947: [OpenCL][CodeGen] Fix replacing memcpy with addrspacecast

[Anastasia Stulova via Phabricator via cfe-commits]

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

LGTM! Thanks!


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

https://reviews.llvm.org/D54947



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

[PATCH] D55121: Make several Python scripts portable across Python2 and Python 3

[Michael Platings via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D55121



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

[PATCH] D54995: [MemoryBuffer] Add the setter to be able to force disabled mmap

[Ivan Donchevskii via Phabricator via cfe-commits]

yvvan added a comment.

In D54995#1316457 , @ilya-biryukov 
wrote:

> In D54995#1316437 , @yvvan wrote:
>
> > Ok, no global option.
> >  Why not placing your VolatileFSProvider in clang so that libclang could 
> > you it too?
>
>
> Would be happy to, will need to figure out what to do with PCH and PCM files 
> first. However if we do this on clang level, I believe we should remove the 
> `isVolatile` flag from the VFS interfaces in the first place.
>  It would be nice to not loose out on the opportunity to avoid fully loading 
> the PCH files, but that obviously requires passing some flags into the VFS 
> implementation or various hacks (matching on filenames/extensions?) to find 
> out which files are PCHs. 
>  I actually don't know which approach to choose: on one hand, I'd really want 
> to get rid of the isVolatile flag, on the other hand I'd really want to avoid 
> loading large binary files into memory and that requires passing the flags.


I don't think removing the flag is a good idea since I can easily assume cases 
when user wants mmap and is ready to encounter locks. In our case it can be an 
IDE option which behavior to choose.
The ideal solution in my opinion would be to have isSystem(filename) in 
FileSystem class. And is based on system directories with which ASTUnit feeds 
the FileSystem for example.


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

https://reviews.llvm.org/D54995



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

[PATCH] D55121: Make several Python scripts portable across Python2 and Python 3

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL348127: Portable Python script across version (authored by 
serge_sans_paille, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D55121?vs=176330&id=176363#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D55121

Files:
  cfe/trunk/bindings/python/clang/cindex.py
  cfe/trunk/docs/tools/dump_format_style.py
  cfe/trunk/tools/scan-view/share/Reporter.py
  cfe/trunk/tools/scan-view/share/ScanView.py
  cfe/trunk/utils/ABITest/ABITestGen.py
  cfe/trunk/utils/ABITest/TypeGen.py
  cfe/trunk/utils/analyzer/CmpRuns.py
  cfe/trunk/utils/modfuzz.py
  cfe/trunk/utils/token-delta.py

Index: cfe/trunk/tools/scan-view/share/Reporter.py
===
--- cfe/trunk/tools/scan-view/share/Reporter.py
+++ cfe/trunk/tools/scan-view/share/Reporter.py
@@ -16,7 +16,7 @@
 
 # Collect information about a bug.
 
-class BugReport:
+class BugReport(object):
 def __init__(self, title, description, files):
 self.title = title
 self.description = description
@@ -37,7 +37,7 @@
 # ReporterParameter
 #======#
 
-class ReporterParameter:
+class ReporterParameter(object):
   def __init__(self, n):
 self.name = n
   def getName(self):
@@ -75,7 +75,7 @@
 # Reporters
 #======#
 
-class EmailReporter:
+class EmailReporter(object):
 def getName(self):
 return 'Email'
 
@@ -143,7 +143,7 @@
 
 return "Message sent!"
 
-class BugzillaReporter:
+class BugzillaReporter(object):
 def getName(self):
 return 'Bugzilla'
 
@@ -174,7 +174,7 @@
 else:
   return '7'
 
-class RadarReporter:
+class RadarReporter(object):
 @staticmethod
 def isAvailable():
 # FIXME: Find this .scpt better
Index: cfe/trunk/tools/scan-view/share/ScanView.py
===
--- cfe/trunk/tools/scan-view/share/ScanView.py
+++ cfe/trunk/tools/scan-view/share/ScanView.py
@@ -422,7 +422,7 @@
 return self.send_string(res, 'text/plain')
 
 def get_report_context(self, report):
-class Context:
+class Context(object):
 pass
 if report is None or report == 'None':
 data = self.load_crashes()
Index: cfe/trunk/utils/ABITest/TypeGen.py
===
--- cfe/trunk/utils/ABITest/TypeGen.py
+++ cfe/trunk/utils/ABITest/TypeGen.py
@@ -17,7 +17,7 @@
 ###
 # Actual type types
 
-class Type:
+class Type(object):
 def isBitField(self):
 return False
 
Index: cfe/trunk/utils/ABITest/ABITestGen.py
===
--- cfe/trunk/utils/ABITest/ABITestGen.py
+++ cfe/trunk/utils/ABITest/ABITestGen.py
@@ -10,7 +10,7 @@
 
 
 
-class TypePrinter:
+class TypePrinter(object):
 def __init__(self, output, outputHeader=None, 
  outputTests=None, outputDriver=None,
  headerName=None, info=None):
Index: cfe/trunk/utils/analyzer/CmpRuns.py
===
--- cfe/trunk/utils/analyzer/CmpRuns.py
+++ cfe/trunk/utils/analyzer/CmpRuns.py
@@ -38,7 +38,7 @@
 
 STATS_REGEXP = re.compile(r"Statistics: (\{.+\})", re.MULTILINE | re.DOTALL)
 
-class Colors:
+class Colors(object):
 """
 Color for terminal highlight.
 """
@@ -50,14 +50,14 @@
 # path - the analysis output directory
 # root - the name of the root directory, which will be disregarded when
 # determining the source file name
-class SingleRunInfo:
+class SingleRunInfo(object):
 def __init__(self, path, root="", verboseLog=None):
 self.path = path
 self.root = root.rstrip("/\\")
 self.verboseLog = verboseLog
 
 
-class AnalysisDiagnostic:
+class AnalysisDiagnostic(object):
 def __init__(self, data, report, htmlReport):
 self._data = data
 self._loc = self._data['location']
@@ -117,14 +117,14 @@
 return self._data
 
 
-class AnalysisReport:
+class AnalysisReport(object):
 def __init__(self, run, files):
 self.run = run
 self.files = files
 self.diagnostics = []
 
 
-class AnalysisRun:
+class AnalysisRun(object):
 def __init__(self, info):
 self.path = info.path
 self.root = info.root
Index: cfe/trunk/utils/modfuzz.py
===
--- cfe/trunk/utils/modfuzz.py
+++ cfe/trunk/utils/modfuzz.py
@@ -12,7 +12,7 @@
 clang = sys.argv[1]
 none_opts = 0.3
 
-class Decl:
+class Decl(object):
   def __init__(self, text, depends=[], provides=[], conflicts=[]):
 self.text = text
 self.depends = depend

r348126 - Portable Python script across Python version

[Serge Guelton via cfe-commits]

Author: serge_sans_paille
Date: Mon Dec  3 04:11:21 2018
New Revision: 348126

URL: http://llvm.org/viewvc/llvm-project?rev=348126&view=rev
Log:
Portable Python script across Python version

Python2 supports the two following equivalent construct

raise ExceptionType, exception_value
and
raise ExceptionType(exception_value)

Only the later is supported by Python3.

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

Modified:
cfe/trunk/tools/scan-view/share/ScanView.py
cfe/trunk/utils/ABITest/ABITestGen.py
cfe/trunk/utils/ABITest/Enumeration.py
cfe/trunk/utils/ABITest/TypeGen.py
cfe/trunk/utils/analyzer/SATestBuild.py
cfe/trunk/utils/token-delta.py

Modified: cfe/trunk/tools/scan-view/share/ScanView.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/scan-view/share/ScanView.py?rev=348126&r1=348125&r2=348126&view=diff
==
--- cfe/trunk/tools/scan-view/share/ScanView.py (original)
+++ cfe/trunk/tools/scan-view/share/ScanView.py Mon Dec  3 04:11:21 2018
@@ -102,9 +102,9 @@ class ReporterThread(threading.Thread):
 time.sleep(3)
 if self.server.options.debug:
 print >>sys.stderr, "%s: SERVER: submission 
complete."%(sys.argv[0],)
-except Reporter.ReportFailure,e:
+except Reporter.ReportFailure as e:
 self.status = e.value
-except Exception,e:
+except Exception as e:
 s = StringIO.StringIO()
 import traceback
 print >>s,'Unhandled Exception'
@@ -163,7 +163,7 @@ class ScanViewServer(BaseHTTPServer.HTTP
 print >>sys.stderr, "%s: SERVER: waiting..." % (sys.argv[0],)
 try:
 self.handle_request()
-except OSError,e:
+except OSError as e:
 print 'OSError',e.errno
 
 def finish_request(self, request, client_address):
@@ -207,13 +207,13 @@ class ScanViewRequestHandler(SimpleHTTPS
 def do_HEAD(self):
 try:
 SimpleHTTPServer.SimpleHTTPRequestHandler.do_HEAD(self)
-except Exception,e:
+except Exception as e:
 self.handle_exception(e)
 
 def do_GET(self):
 try:
 SimpleHTTPServer.SimpleHTTPRequestHandler.do_GET(self)
-except Exception,e:
+except Exception as e:
 self.handle_exception(e)
 
 def do_POST(self):
@@ -230,7 +230,7 @@ class ScanViewRequestHandler(SimpleHTTPS
 if f:
 self.copyfile(f, self.wfile)
 f.close()
-except Exception,e:
+except Exception as e:
 self.handle_exception(e)
 
 def log_message(self, format, *args):
@@ -428,7 +428,7 @@ Submit
 data = self.load_crashes()
 # Don't allow empty reports.
 if not data:
-raise ValueError, 'No crashes detected!'
+raise ValueError('No crashes detected!')
 c = Context()
 c.title = 'clang static analyzer failures'
 
@@ -472,7 +472,7 @@ STDERR Summary
 # Check that this is a valid report.
 path = posixpath.join(self.server.root, 'report-%s.html' % report)
 if not posixpath.exists(path):
-raise ValueError, 'Invalid report ID'
+raise ValueError('Invalid report ID')
 keys = self.load_report(report)
 c = Context()
 c.title = keys.get('DESC','clang error (unrecognized')
@@ -501,7 +501,7 @@ Line: %s
 # report is None is used for crashes
 try:
 c = self.get_report_context(report)
-except ValueError, e:
+except ValueError as e:
 return self.send_error(400, e.message)
 
 title = c.title

Modified: cfe/trunk/utils/ABITest/ABITestGen.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/ABITest/ABITestGen.py?rev=348126&r1=348125&r2=348126&view=diff
==
--- cfe/trunk/utils/ABITest/ABITestGen.py (original)
+++ cfe/trunk/utils/ABITest/ABITestGen.py Mon Dec  3 04:11:21 2018
@@ -250,7 +250,7 @@ class TypePrinter:
 elements[i] = v
 yield '{ %s }'%(', '.join(elements))
 else:
-raise NotImplementedError,'Cannot make tests values of type: 
"%s"'%(t,)
+raise NotImplementedError('Cannot make tests values of type: 
"%s"'%(t,))
 
 def printSizeOfType(self, prefix, name, t, output=None, indent=2):
 print >>output, '%*sprintf("%s: sizeof(%s) = %%ld\\n", 
(long)sizeof(%s));'%(indent, '', prefix, name, name) 
@@ -310,7 +310,7 @@ class TypePrinter:
 else:
 self.printValueOfType(prefix, '%s[%d]'%(name,i), 
t.elementType, output=output,indent=indent)
 else:
-

r348116 - [CMake] Store path to vendor-specific headers in clang-headers target property

[Stefan Granitz via cfe-commits]

Author: stefan.graenitz
Date: Mon Dec  3 02:34:25 2018
New Revision: 348116

URL: http://llvm.org/viewvc/llvm-project?rev=348116&view=rev
Log:
[CMake] Store path to vendor-specific headers in clang-headers target property

Summary:
LLDB.framework wants a copy these headers. With this change LLDB can easily 
glob for the list of files:
```
get_target_property(clang_include_dir clang-headers RUNTIME_OUTPUT_DIRECTORY)
file(GLOB_RECURSE clang_vendor_headers RELATIVE ${clang_include_dir} 
"${clang_include_dir}/*")
```

By default `RUNTIME_OUTPUT_DIRECTORY` is unset for custom targets like 
`clang-headers`.

Reviewers: aprantl, JDevlieghere, davide, friss, dexonsmith

Reviewed By: JDevlieghere

Subscribers: mgorny, #lldb, cfe-commits, llvm-commits

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

Modified:
cfe/trunk/lib/Headers/CMakeLists.txt

Modified: cfe/trunk/lib/Headers/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/CMakeLists.txt?rev=348116&r1=348115&r2=348116&view=diff
==
--- cfe/trunk/lib/Headers/CMakeLists.txt (original)
+++ cfe/trunk/lib/Headers/CMakeLists.txt Mon Dec  3 02:34:25 2018
@@ -144,7 +144,7 @@ foreach( f ${files} ${cuda_wrapper_files
   list(APPEND out_files ${dst})
 endforeach( f )
 
-add_custom_command(OUTPUT ${output_dir}/arm_neon.h 
+add_custom_command(OUTPUT ${output_dir}/arm_neon.h
   DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/arm_neon.h
   COMMAND ${CMAKE_COMMAND} -E copy_if_different 
${CMAKE_CURRENT_BINARY_DIR}/arm_neon.h ${output_dir}/arm_neon.h
   COMMENT "Copying clang's arm_neon.h...")
@@ -156,7 +156,9 @@ add_custom_command(OUTPUT ${output_dir}/
 list(APPEND out_files ${output_dir}/arm_fp16.h)
 
 add_custom_target(clang-headers ALL DEPENDS ${out_files})
-set_target_properties(clang-headers PROPERTIES FOLDER "Misc")
+set_target_properties(clang-headers PROPERTIES
+  FOLDER "Misc"
+  RUNTIME_OUTPUT_DIRECTORY "${output_dir}")
 
 install(
   FILES ${files} ${CMAKE_CURRENT_BINARY_DIR}/arm_neon.h


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

r348127 - Portable Python script across version

[Serge Guelton via cfe-commits]

Author: serge_sans_paille
Date: Mon Dec  3 04:12:48 2018
New Revision: 348127

URL: http://llvm.org/viewvc/llvm-project?rev=348127&view=rev
Log:
Portable Python script across version

Have all classes derive from object: that's implicitly the default in Python3,
it needs to be done explicilty in Python2.

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

Modified:
cfe/trunk/bindings/python/clang/cindex.py
cfe/trunk/docs/tools/dump_format_style.py
cfe/trunk/tools/scan-view/share/Reporter.py
cfe/trunk/tools/scan-view/share/ScanView.py
cfe/trunk/utils/ABITest/ABITestGen.py
cfe/trunk/utils/ABITest/TypeGen.py
cfe/trunk/utils/analyzer/CmpRuns.py
cfe/trunk/utils/modfuzz.py
cfe/trunk/utils/token-delta.py

Modified: cfe/trunk/bindings/python/clang/cindex.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/bindings/python/clang/cindex.py?rev=348127&r1=348126&r2=348127&view=diff
==
--- cfe/trunk/bindings/python/clang/cindex.py (original)
+++ cfe/trunk/bindings/python/clang/cindex.py Mon Dec  3 04:12:48 2018
@@ -400,7 +400,7 @@ class Diagnostic(object):
 
 @property
 def ranges(self):
-class RangeIterator:
+class RangeIterator(object):
 def __init__(self, diag):
 self.diag = diag
 
@@ -416,7 +416,7 @@ class Diagnostic(object):
 
 @property
 def fixits(self):
-class FixItIterator:
+class FixItIterator(object):
 def __init__(self, diag):
 self.diag = diag
 
@@ -436,7 +436,7 @@ class Diagnostic(object):
 
 @property
 def children(self):
-class ChildDiagnosticsIterator:
+class ChildDiagnosticsIterator(object):
 def __init__(self, diag):
 self.diag_set = conf.lib.clang_getChildDiagnostics(diag)
 
@@ -2475,8 +2475,8 @@ SpellingCache = {
 # 20: CompletionChunk.Kind("VerticalSpace")
 }
 
-class CompletionChunk:
-class Kind:
+class CompletionChunk(object):
+class Kind(object):
 def __init__(self, name):
 self.name = name
 
@@ -2563,7 +2563,7 @@ completionChunkKindMap = {
 20: CompletionChunk.Kind("VerticalSpace")}
 
 class CompletionString(ClangObject):
-class Availability:
+class Availability(object):
 def __init__(self, name):
 self.name = name
 
@@ -2656,7 +2656,7 @@ class CodeCompletionResults(ClangObject)
 
 @property
 def diagnostics(self):
-class DiagnosticsItr:
+class DiagnosticsItr(object):
 def __init__(self, ccr):
 self.ccr= ccr
 
@@ -2958,7 +2958,7 @@ class TranslationUnit(ClangObject):
 """
 Return an iterable (and indexable) object containing the diagnostics.
 """
-class DiagIterator:
+class DiagIterator(object):
 def __init__(self, tu):
 self.tu = tu
 
@@ -4090,7 +4090,7 @@ def register_functions(lib, ignore_error
 for f in functionList:
 register(f)
 
-class Config:
+class Config(object):
 library_path = None
 library_file = None
 compatibility_check = True

Modified: cfe/trunk/docs/tools/dump_format_style.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/tools/dump_format_style.py?rev=348127&r1=348126&r2=348127&view=diff
==
--- cfe/trunk/docs/tools/dump_format_style.py (original)
+++ cfe/trunk/docs/tools/dump_format_style.py Mon Dec  3 04:12:48 2018
@@ -32,7 +32,7 @@ def indent(text, columns, indent_first_l
 return s
   return indent + s
 
-class Option:
+class Option(object):
   def __init__(self, name, type, comment):
 self.name = name
 self.type = type
@@ -50,7 +50,7 @@ class Option:
   2)
 return s
 
-class NestedStruct:
+class NestedStruct(object):
   def __init__(self, name, comment):
 self.name = name
 self.comment = comment.strip()
@@ -59,7 +59,7 @@ class NestedStruct:
   def __str__(self):
 return '\n'.join(map(str, self.values))
 
-class NestedField:
+class NestedField(object):
   def __init__(self, name, comment):
 self.name = name
 self.comment = comment.strip()
@@ -69,7 +69,7 @@ class NestedField:
 self.name,
 doxygen2rst(indent(self.comment, 2, indent_first_line=False)))
 
-class Enum:
+class Enum(object):
   def __init__(self, name, comment):
 self.name = name
 self.comment = comment.strip()
@@ -78,7 +78,7 @@ class Enum:
   def __str__(self):
 return '\n'.join(map(str, self.values))
 
-class EnumValue:
+class EnumValue(object):
   def __init__(self, name, comment):
 self.name = name
 self.comment = comment
@@ -101,7 +101,7 @@ def clean_comment_line(line):
   return line[4:] + '\n'
 
 def read_options(header):
-  class State:
+  class State(object):
 BeforeStruct, Finished, InStruct, InNestedStruct, InNestedFieldComent, \
 InF

Re: r348123 - [clang] Do not read from 'test/SemaCXX/Inputs' inside 'test/AST'

[Aaron Ballman via cfe-commits]

On Mon, Dec 3, 2018 at 6:29 AM Ilya Biryukov via cfe-commits
 wrote:
>
> Author: ibiryukov
> Date: Mon Dec  3 03:26:35 2018
> New Revision: 348123
>
> URL: http://llvm.org/viewvc/llvm-project?rev=348123&view=rev
> Log:
> [clang] Do not read from 'test/SemaCXX/Inputs' inside 'test/AST'
>
> Our integrate relies on test inputs being taken from the same diretory as the
> test itself.

Sorry about that! I was trying to avoid duplicating the same header
file in two different places and didn't realize this would cause an
issue.

~Aaron

>
> Added:
> cfe/trunk/test/AST/Inputs/std-coroutine.h
> Modified:
> cfe/trunk/test/AST/coroutine-source-location-crash.cpp
>
> Added: cfe/trunk/test/AST/Inputs/std-coroutine.h
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/AST/Inputs/std-coroutine.h?rev=348123&view=auto
> ==
> --- cfe/trunk/test/AST/Inputs/std-coroutine.h (added)
> +++ cfe/trunk/test/AST/Inputs/std-coroutine.h Mon Dec  3 03:26:35 2018
> @@ -0,0 +1,37 @@
> +// RUN: %clang_cc1 -triple x86_64-apple-darwin9 %s -std=c++14 
> -fcoroutines-ts -fsyntax-only -Wignored-qualifiers -Wno-error=return-type 
> -verify -fblocks -Wno-unreachable-code -Wno-unused-value
> +#ifndef STD_COROUTINE_H
> +#define STD_COROUTINE_H
> +
> +namespace std {
> +namespace experimental {
> +
> +template 
> +struct coroutine_traits { using promise_type = typename Ret::promise_type; };
> +
> +template 
> +struct coroutine_handle {
> +  static coroutine_handle from_address(void *);
> +};
> +template <>
> +struct coroutine_handle {
> +  template 
> +  coroutine_handle(coroutine_handle);
> +  static coroutine_handle from_address(void *);
> +};
> +
> +struct suspend_always {
> +  bool await_ready() { return false; }
> +  void await_suspend(coroutine_handle<>) {}
> +  void await_resume() {}
> +};
> +
> +struct suspend_never {
> +  bool await_ready() { return true; }
> +  void await_suspend(coroutine_handle<>) {}
> +  void await_resume() {}
> +};
> +
> +} // namespace experimental
> +} // namespace std
> +
> +#endif // STD_COROUTINE_H
>
> Modified: cfe/trunk/test/AST/coroutine-source-location-crash.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/AST/coroutine-source-location-crash.cpp?rev=348123&r1=348122&r2=348123&view=diff
> ==
> --- cfe/trunk/test/AST/coroutine-source-location-crash.cpp (original)
> +++ cfe/trunk/test/AST/coroutine-source-location-crash.cpp Mon Dec  3 
> 03:26:35 2018
> @@ -1,6 +1,6 @@
>  // RUN: %clang_cc1 -triple x86_64-apple-darwin9 %s -std=c++14 
> -fcoroutines-ts \
>  // RUN:-fsyntax-only -ast-dump | FileCheck %s
> -#include "../SemaCXX/Inputs/std-coroutine.h"
> +#include "Inputs/std-coroutine.h"
>
>  using namespace std::experimental;
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55068: NFC: Simplify dumpStmt child handling

[Aaron Ballman via Phabricator via cfe-commits]

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

In D55068#1315924 , @steveire wrote:

> Aaron, you added tests for the existing behavior which pass after this patch. 
> Is there anything holding it up?


Nope, I was just waiting on confirmation that the behavior hadn't changed. 
Thanks for letting me know!

LGTM!


Repository:
  rC Clang

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

https://reviews.llvm.org/D55068



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

r348128 - [AST][NFC] Pack CXXDeleteExpr

[Bruno Ricci via cfe-commits]

Author: brunoricci
Date: Mon Dec  3 04:32:32 2018
New Revision: 348128

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

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


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

Modified: cfe/trunk/include/clang/AST/ExprCXX.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ExprCXX.h?rev=348128&r1=348127&r2=348128&view=diff
==
--- cfe/trunk/include/clang/AST/ExprCXX.h (original)
+++ cfe/trunk/include/clang/AST/ExprCXX.h Mon Dec  3 04:32:32 2018
@@ -2086,55 +2086,43 @@ public:
 /// Represents a \c delete expression for memory deallocation and
 /// destructor calls, e.g. "delete[] pArray".
 class CXXDeleteExpr : public Expr {
+  friend class ASTStmtReader;
+
   /// Points to the operator delete overload that is used. Could be a member.
   FunctionDecl *OperatorDelete = nullptr;
 
   /// The pointer expression to be deleted.
   Stmt *Argument = nullptr;
 
-  /// Location of the expression.
-  SourceLocation Loc;
-
-  /// Is this a forced global delete, i.e. "::delete"?
-  bool GlobalDelete : 1;
-
-  /// Is this the array form of delete, i.e. "delete[]"?
-  bool ArrayForm : 1;
-
-  /// ArrayFormAsWritten can be different from ArrayForm if 'delete' is applied
-  /// to pointer-to-array type (ArrayFormAsWritten will be false while 
ArrayForm
-  /// will be true).
-  bool ArrayFormAsWritten : 1;
-
-  /// Does the usual deallocation function for the element type require
-  /// a size_t argument?
-  bool UsualArrayDeleteWantsSize : 1;
-
 public:
-  friend class ASTStmtReader;
+  CXXDeleteExpr(QualType Ty, bool GlobalDelete, bool ArrayForm,
+bool ArrayFormAsWritten, bool UsualArrayDeleteWantsSize,
+FunctionDecl *OperatorDelete, Expr *Arg, SourceLocation Loc)
+  : Expr(CXXDeleteExprClass, Ty, VK_RValue, OK_Ordinary, false, false,
+ Arg->isInstantiationDependent(),
+ Arg->containsUnexpandedParameterPack()),
+OperatorDelete(OperatorDelete), Argument(Arg) {
+CXXDeleteExprBits.GlobalDelete = GlobalDelete;
+CXXDeleteExprBits.ArrayForm = ArrayForm;
+CXXDeleteExprBits.ArrayFormAsWritten = ArrayFormAsWritten;
+CXXDeleteExprBits.UsualArrayDeleteWantsSize = UsualArrayDeleteWantsSize;
+CXXDeleteExprBits.Loc = Loc;
+  }
 
-  CXXDeleteExpr(QualType ty, bool globalDelete, bool arrayForm,
-bool arrayFormAsWritten, bool usualArrayDeleteWantsSize,
-FunctionDecl *operatorDelete, Expr *arg, SourceLocation loc)
-  : Expr(CXXDeleteExprClass, ty, VK_RValue, OK_Ordinary, false, false,
- arg->isInstantiationDependent(),
- arg->containsUnexpandedParameterPack()),
-OperatorDelete(operatorDelete), Argument(arg), Loc(loc),
-GlobalDelete(globalDelete),
-ArrayForm(arrayForm), ArrayFormAsWritten(arrayFormAsWritten),
-UsualArrayDeleteWantsSize(usualArrayDeleteWantsSize) {}
   explicit CXXDeleteExpr(EmptyShell Shell) : Expr(CXXDeleteExprClass, Shell) {}
 
-  bool isGlobalDelete() const { return GlobalDelete; }
-  bool isArrayForm() const { return ArrayForm; }
-  bool isArrayFormAsWritten() const { return ArrayFormAsWritten; }
+  bool isGlobalDelete() const { return CXXDeleteExprBits.GlobalDelete; }
+  bool isArrayForm() const { return CXXDeleteExprBits.ArrayForm; }
+  bool isArrayFormAsWritten() const {
+return CXXDeleteExprBits.ArrayFormAsWritten;
+  }
 
   /// Answers whether the usual array deallocation function for the
   /// allocated type expects the size of the allocation as a
   /// parameter.  This can be true even if the actual deallocation
   /// function that we're using doesn't want a size.
   bool doesUsualArrayDeleteWantSize() const {
-return UsualArrayDeleteWantsSize;
+return CXXDeleteExprBits.UsualArrayDeleteWantsSize;
   }
 
   FunctionDecl *getOperatorDelete() const { return OperatorDelete; }
@@ -2148,7 +2136,7 @@ public:
   /// be a pointer, return an invalid type.
   QualType getDestroyedType() const;
 
-  SourceLocation getBeginLoc() const LLVM_READONLY { return Loc; }
+  SourceLocation getBeginLoc() const { return CXXDeleteExprBits.Loc; }
   SourceLocation getEndLoc() const LLVM_READONLY {
 return Argument->getEndLoc();
   }
@@ -2158,7 +2146,7 @@ public:
   }
 
   // Iterators
-  child_range children() { return child_range(&Argument, &Argument+1); }
+  child_range children() { return child_range(&Argument, &Argument + 1); }
 };
 
 /// Stores the type being destroyed by a pseudo-destructor expression.

Modified: cfe/trunk/include/clang/AST/Stmt.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Stmt.h?rev=348128&r1=348127&r2=348128&view=dif

r348129 - Portable Python script across Python version

[Serge Guelton via cfe-commits]

Author: serge_sans_paille
Date: Mon Dec  3 04:41:35 2018
New Revision: 348129

URL: http://llvm.org/viewvc/llvm-project?rev=348129&view=rev
Log:
Portable Python script across Python version

Python3 does not support type destructuring in function parameters.

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

Modified:
cfe/trunk/utils/ABITest/Enumeration.py
cfe/trunk/utils/ABITest/TypeGen.py

Modified: cfe/trunk/utils/ABITest/Enumeration.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/ABITest/Enumeration.py?rev=348129&r1=348128&r2=348129&view=diff
==
--- cfe/trunk/utils/ABITest/Enumeration.py (original)
+++ cfe/trunk/utils/ABITest/Enumeration.py Mon Dec  3 04:41:35 2018
@@ -46,7 +46,8 @@ aleph0 = Aleph0()
 def base(line):
 return line*(line+1)//2
 
-def pairToN((x,y)):
+def pairToN(pair):
+x,y = pair
 line,index = x+y,y
 return base(line)+index
 

Modified: cfe/trunk/utils/ABITest/TypeGen.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/ABITest/TypeGen.py?rev=348129&r1=348128&r2=348129&view=diff
==
--- cfe/trunk/utils/ABITest/TypeGen.py (original)
+++ cfe/trunk/utils/ABITest/TypeGen.py Mon Dec  3 04:41:35 2018
@@ -99,7 +99,8 @@ class RecordType(Type):
 ' '.join(map(getField, self.fields)))
 
 def getTypedefDef(self, name, printer):
-def getField((i, t)):
+def getField(it):
+i, t = it
 if t.isBitField():
 if t.isPaddingBitField():
 return '%s : 0;'%(printer.getTypeName(t),)


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

[PATCH] D54903: [Sema] Improve static_assert diagnostics.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: lib/Sema/SemaTemplate.cpp:3055
 
+// Print a diagnostic for the failing static_assert expression. Defaults to
+// pretty-printing the expression.

Comment is a bit out of date as this is no longer specific to `static_assert`.

It looks like this may also change the behavior of enable_if diagnostic 
reporting. Do you need to update any of those tests from this change? If not, 
can you devise some tests for that case as well to show that this isn't just a 
static_assert behavior? (Do a search for `findFailedBooleanCondition` to find 
where the behavior has changed.)


Repository:
  rC Clang

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

https://reviews.llvm.org/D54903



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

[PATCH] D55212: Handle alloc_size attribute on function pointers

[Alexander Richardson via Phabricator via cfe-commits]

arichardson created this revision.
arichardson added reviewers: george.burgess.iv, aaron.ballman, jdenny.
Herald added a subscriber: cfe-commits.

I have been trying to statically find and analyze all calls to heap
allocation functions to determine how many of them use sizes known at
compile time vs only at runtime. While doing so I saw that quite a few
projects use replaceable function pointers for heap allocation and noticed
that clang was not able to annotate functions pointers with alloc_size.
I have changed the Sema checks to allow alloc_size on all function pointers
and typedefs for function pointers now and added checks that these
attributes are propagated to the LLVM IR correctly.

With this patch we can also compute __builtin_object_size() for calls to
allocation function pointers with the alloc_size attribute.


Repository:
  rC Clang

https://reviews.llvm.org/D55212

Files:
  include/clang/Basic/Attr.td
  lib/AST/ExprConstant.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/CodeGen/alloc-size-fnptr.c
  test/CodeGen/alloc-size.c
  test/Sema/alloc-size.c

Index: test/Sema/alloc-size.c
===
--- test/Sema/alloc-size.c
+++ test/Sema/alloc-size.c
@@ -14,7 +14,7 @@
 
 int fail9(int a) __attribute__((alloc_size(1))); //expected-warning{{'alloc_size' attribute only applies to return values that are pointers}}
 
-int fail10 __attribute__((alloc_size(1))); //expected-warning{{'alloc_size' attribute only applies to functions}}
+int fail10 __attribute__((alloc_size(1))); //expected-warning{{'alloc_size' attribute only applies to non-K&R-style functions}}
 
 void *fail11(void *a) __attribute__((alloc_size(1))); //expected-error{{'alloc_size' attribute argument may only refer to a function parameter of integer type}}
 
@@ -22,4 +22,25 @@
 void *fail12(int a) __attribute__((alloc_size(1, "abc"))); //expected-error{{'alloc_size' attribute requires parameter 2 to be an integer constant}}
 void *fail13(int a) __attribute__((alloc_size(1U<<31))); //expected-error{{integer constant expression evaluates to value 2147483648 that cannot be represented in a 32-bit signed integer type}}
 
-int (*PR31453)(int) __attribute__((alloc_size(1))); //expected-warning{{'alloc_size' attribute only applies to functions}}
+void *(*PR31453)(int)__attribute__((alloc_size(1)));
+
+void *KR() __attribute__((alloc_size(1))); //expected-warning{{'alloc_size' attribute only applies to non-K&R-style functions}}
+
+// Applying alloc_size to function pointers should work:
+void *(__attribute__((alloc_size(1))) * func_ptr1)(int);
+void *(__attribute__((alloc_size(1, 2))) func_ptr2)(int, int);
+
+// TODO: according to GCC documentation the following should actually be the type
+// “pointer to pointer to alloc_size attributed function returning void*” and should
+// therefore be supported
+void *(__attribute__((alloc_size(1))) **ptr_to_func_ptr)(int); // expected-warning{{'alloc_size' attribute only applies to non-K&R-style functions}}
+// The following definitions apply the attribute to the pointer to the function pointer which should not be possible
+void *(*__attribute__((alloc_size(1))) * ptr_to_func_ptr2)(int); // expected-warning{{'alloc_size' attribute only applies to non-K&R-style functions}}
+void *(**__attribute__((alloc_size(1))) ptr_to_func_ptr2)(int);  // expected-warning{{'alloc_size' attribute only applies to non-K&R-style functions}}
+
+// It should also work for typedefs:
+typedef void *(__attribute__((alloc_size(1))) allocator_function_typdef)(int);
+typedef void *(__attribute__((alloc_size(1, 2))) * allocator_function_typdef2)(int, int);
+void *(__attribute__((alloc_size(1, 2))) * allocator_function_typdef3)(int, int);
+// This typedef applies the alloc_size to the pointer to the function pointer and should not be allowed
+void *(**__attribute__((alloc_size(1, 2))) * allocator_function_typdef4)(int, int); // expected-warning{{'alloc_size' attribute only applies to non-K&R-style functions}}
Index: test/CodeGen/alloc-size.c
===
--- test/CodeGen/alloc-size.c
+++ test/CodeGen/alloc-size.c
@@ -1,3 +1,4 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm %s -o -
 // RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm %s -o - 2>&1 | FileCheck %s
 
 #define NULL ((void *)0)
@@ -350,3 +351,128 @@
   // CHECK: store i32 -1
   gi = __builtin_object_size(my_signed_calloc(-2, 1), 0);
 }
+
+void* (*malloc_function_pointer)(int) __attribute__((alloc_size(1)));
+void* (*calloc_function_pointer)(int, int) __attribute__((alloc_size(1, 2)));
+
+// CHECK-LABEL: @test_fn_pointer
+void test_fn_pointer() {
+  void *const vp = malloc_function_pointer(100);
+  // CHECK: store i32 100
+  gi = __builtin_object_size(vp, 0);
+  // CHECK: store i32 100
+  gi = __builtin_object_size(vp, 1);
+  // CHECK: store i32 100
+  gi = __builtin_object_size(vp, 2);
+  // CHECK: store i32 100
+  gi = __builtin_object_

[PATCH] D55212: Handle alloc_size attribute on function pointers

[Alexander Richardson via Phabricator via cfe-commits]

arichardson updated this revision to Diff 176368.
arichardson added a comment.

Remove RUN: line added for debugging


Repository:
  rC Clang

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

https://reviews.llvm.org/D55212

Files:
  include/clang/Basic/Attr.td
  lib/AST/ExprConstant.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/CodeGen/alloc-size-fnptr.c
  test/CodeGen/alloc-size.c
  test/Sema/alloc-size.c

Index: test/Sema/alloc-size.c
===
--- test/Sema/alloc-size.c
+++ test/Sema/alloc-size.c
@@ -14,7 +14,7 @@
 
 int fail9(int a) __attribute__((alloc_size(1))); //expected-warning{{'alloc_size' attribute only applies to return values that are pointers}}
 
-int fail10 __attribute__((alloc_size(1))); //expected-warning{{'alloc_size' attribute only applies to functions}}
+int fail10 __attribute__((alloc_size(1))); //expected-warning{{'alloc_size' attribute only applies to non-K&R-style functions}}
 
 void *fail11(void *a) __attribute__((alloc_size(1))); //expected-error{{'alloc_size' attribute argument may only refer to a function parameter of integer type}}
 
@@ -22,4 +22,25 @@
 void *fail12(int a) __attribute__((alloc_size(1, "abc"))); //expected-error{{'alloc_size' attribute requires parameter 2 to be an integer constant}}
 void *fail13(int a) __attribute__((alloc_size(1U<<31))); //expected-error{{integer constant expression evaluates to value 2147483648 that cannot be represented in a 32-bit signed integer type}}
 
-int (*PR31453)(int) __attribute__((alloc_size(1))); //expected-warning{{'alloc_size' attribute only applies to functions}}
+void *(*PR31453)(int)__attribute__((alloc_size(1)));
+
+void *KR() __attribute__((alloc_size(1))); //expected-warning{{'alloc_size' attribute only applies to non-K&R-style functions}}
+
+// Applying alloc_size to function pointers should work:
+void *(__attribute__((alloc_size(1))) * func_ptr1)(int);
+void *(__attribute__((alloc_size(1, 2))) func_ptr2)(int, int);
+
+// TODO: according to GCC documentation the following should actually be the type
+// “pointer to pointer to alloc_size attributed function returning void*” and should
+// therefore be supported
+void *(__attribute__((alloc_size(1))) **ptr_to_func_ptr)(int); // expected-warning{{'alloc_size' attribute only applies to non-K&R-style functions}}
+// The following definitions apply the attribute to the pointer to the function pointer which should not be possible
+void *(*__attribute__((alloc_size(1))) * ptr_to_func_ptr2)(int); // expected-warning{{'alloc_size' attribute only applies to non-K&R-style functions}}
+void *(**__attribute__((alloc_size(1))) ptr_to_func_ptr2)(int);  // expected-warning{{'alloc_size' attribute only applies to non-K&R-style functions}}
+
+// It should also work for typedefs:
+typedef void *(__attribute__((alloc_size(1))) allocator_function_typdef)(int);
+typedef void *(__attribute__((alloc_size(1, 2))) * allocator_function_typdef2)(int, int);
+void *(__attribute__((alloc_size(1, 2))) * allocator_function_typdef3)(int, int);
+// This typedef applies the alloc_size to the pointer to the function pointer and should not be allowed
+void *(**__attribute__((alloc_size(1, 2))) * allocator_function_typdef4)(int, int); // expected-warning{{'alloc_size' attribute only applies to non-K&R-style functions}}
Index: test/CodeGen/alloc-size.c
===
--- test/CodeGen/alloc-size.c
+++ test/CodeGen/alloc-size.c
@@ -350,3 +350,128 @@
   // CHECK: store i32 -1
   gi = __builtin_object_size(my_signed_calloc(-2, 1), 0);
 }
+
+void* (*malloc_function_pointer)(int) __attribute__((alloc_size(1)));
+void* (*calloc_function_pointer)(int, int) __attribute__((alloc_size(1, 2)));
+
+// CHECK-LABEL: @test_fn_pointer
+void test_fn_pointer() {
+  void *const vp = malloc_function_pointer(100);
+  // CHECK: store i32 100
+  gi = __builtin_object_size(vp, 0);
+  // CHECK: store i32 100
+  gi = __builtin_object_size(vp, 1);
+  // CHECK: store i32 100
+  gi = __builtin_object_size(vp, 2);
+  // CHECK: store i32 100
+  gi = __builtin_object_size(vp, 3);
+
+  void *const arr = calloc_function_pointer(100, 5);
+  // CHECK: store i32 500
+  gi = __builtin_object_size(arr, 0);
+  // CHECK: store i32 500
+  gi = __builtin_object_size(arr, 1);
+  // CHECK: store i32 500
+  gi = __builtin_object_size(arr, 2);
+  // CHECK: store i32 500
+  gi = __builtin_object_size(arr, 3);
+
+  // CHECK: store i32 100
+  gi = __builtin_object_size(malloc_function_pointer(100), 0);
+  // CHECK: store i32 100
+  gi = __builtin_object_size(malloc_function_pointer(100), 1);
+  // CHECK: store i32 100
+  gi = __builtin_object_size(malloc_function_pointer(100), 2);
+  // CHECK: store i32 100
+  gi = __builtin_object_size(malloc_function_pointer(100), 3);
+
+  // CHECK: store i32 500
+  gi = __builtin_object_size(calloc_function_pointer(100, 5), 0);
+  // CHECK: store i32 500
+  gi = __builtin_object_

[clang-tools-extra] r348130 - [clangd] Get rid of AST matchers in CodeComplete, NFC

[Haojian Wu via cfe-commits]

Author: hokein
Date: Mon Dec  3 04:53:19 2018
New Revision: 348130

URL: http://llvm.org/viewvc/llvm-project?rev=348130&view=rev
Log:
[clangd] Get rid of AST matchers in CodeComplete, NFC

Summary:
The isIndexedForCodeCompletion is called in the code patch of
SymbolCollector.

Reviewers: kadircet

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

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

Modified:
clang-tools-extra/trunk/clangd/CodeComplete.cpp

Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=348130&r1=348129&r2=348130&view=diff
==
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Mon Dec  3 04:53:19 2018
@@ -37,7 +37,6 @@
 #include "index/Index.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
-#include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Format/Format.h"
@@ -1645,14 +1644,24 @@ SignatureHelp signatureHelp(PathRef File
 }
 
 bool isIndexedForCodeCompletion(const NamedDecl &ND, ASTContext &ASTCtx) {
-  using namespace clang::ast_matchers;
-  auto InTopLevelScope = hasDeclContext(
-  anyOf(namespaceDecl(), translationUnitDecl(), linkageSpecDecl()));
-  return !match(decl(anyOf(InTopLevelScope,
-   hasDeclContext(
-   enumDecl(InTopLevelScope, 
unless(isScoped()),
-ND, ASTCtx)
-  .empty();
+  auto InTopLevelScope = [](const NamedDecl &ND) {
+switch (ND.getDeclContext()->getDeclKind()) {
+case Decl::TranslationUnit:
+case Decl::Namespace:
+case Decl::LinkageSpec:
+  return true;
+default:
+  break;
+};
+return false;
+  };
+  if (InTopLevelScope(ND))
+return true;
+
+  if (const auto *EnumDecl = dyn_cast(ND.getDeclContext()))
+return InTopLevelScope(*EnumDecl) && !EnumDecl->isScoped();
+
+  return false;
 }
 
 CompletionItem CodeCompletion::render(const CodeCompleteOptions &Opts) const {


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

[PATCH] D55206: [clangd] Get rid of AST matchers in CodeComplete, NFC

[Haojian Wu via Phabricator via cfe-commits]

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

Using break.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55206

Files:
  clangd/CodeComplete.cpp


Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -37,7 +37,6 @@
 #include "index/Index.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
-#include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Format/Format.h"
@@ -1645,14 +1644,24 @@
 }
 
 bool isIndexedForCodeCompletion(const NamedDecl &ND, ASTContext &ASTCtx) {
-  using namespace clang::ast_matchers;
-  auto InTopLevelScope = hasDeclContext(
-  anyOf(namespaceDecl(), translationUnitDecl(), linkageSpecDecl()));
-  return !match(decl(anyOf(InTopLevelScope,
-   hasDeclContext(
-   enumDecl(InTopLevelScope, 
unless(isScoped()),
-ND, ASTCtx)
-  .empty();
+  auto InTopLevelScope = [](const NamedDecl &ND) {
+switch (ND.getDeclContext()->getDeclKind()) {
+case Decl::TranslationUnit:
+case Decl::Namespace:
+case Decl::LinkageSpec:
+  return true;
+default:
+  break;
+};
+return false;
+  };
+  if (InTopLevelScope(ND))
+return true;
+
+  if (const auto *EnumDecl = dyn_cast(ND.getDeclContext()))
+return InTopLevelScope(*EnumDecl) && !EnumDecl->isScoped();
+
+  return false;
 }
 
 CompletionItem CodeCompletion::render(const CodeCompleteOptions &Opts) const {


Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -37,7 +37,6 @@
 #include "index/Index.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
-#include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Format/Format.h"
@@ -1645,14 +1644,24 @@
 }
 
 bool isIndexedForCodeCompletion(const NamedDecl &ND, ASTContext &ASTCtx) {
-  using namespace clang::ast_matchers;
-  auto InTopLevelScope = hasDeclContext(
-  anyOf(namespaceDecl(), translationUnitDecl(), linkageSpecDecl()));
-  return !match(decl(anyOf(InTopLevelScope,
-   hasDeclContext(
-   enumDecl(InTopLevelScope, unless(isScoped()),
-ND, ASTCtx)
-  .empty();
+  auto InTopLevelScope = [](const NamedDecl &ND) {
+switch (ND.getDeclContext()->getDeclKind()) {
+case Decl::TranslationUnit:
+case Decl::Namespace:
+case Decl::LinkageSpec:
+  return true;
+default:
+  break;
+};
+return false;
+  };
+  if (InTopLevelScope(ND))
+return true;
+
+  if (const auto *EnumDecl = dyn_cast(ND.getDeclContext()))
+return InTopLevelScope(*EnumDecl) && !EnumDecl->isScoped();
+
+  return false;
 }
 
 CompletionItem CodeCompletion::render(const CodeCompleteOptions &Opts) const {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55206: [clangd] Get rid of AST matchers in CodeComplete, NFC

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL348130: [clangd] Get rid of AST matchers in CodeComplete, 
NFC (authored by hokein, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D55206

Files:
  clang-tools-extra/trunk/clangd/CodeComplete.cpp


Index: clang-tools-extra/trunk/clangd/CodeComplete.cpp
===
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp
@@ -37,7 +37,6 @@
 #include "index/Index.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
-#include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Format/Format.h"
@@ -1645,14 +1644,24 @@
 }
 
 bool isIndexedForCodeCompletion(const NamedDecl &ND, ASTContext &ASTCtx) {
-  using namespace clang::ast_matchers;
-  auto InTopLevelScope = hasDeclContext(
-  anyOf(namespaceDecl(), translationUnitDecl(), linkageSpecDecl()));
-  return !match(decl(anyOf(InTopLevelScope,
-   hasDeclContext(
-   enumDecl(InTopLevelScope, 
unless(isScoped()),
-ND, ASTCtx)
-  .empty();
+  auto InTopLevelScope = [](const NamedDecl &ND) {
+switch (ND.getDeclContext()->getDeclKind()) {
+case Decl::TranslationUnit:
+case Decl::Namespace:
+case Decl::LinkageSpec:
+  return true;
+default:
+  break;
+};
+return false;
+  };
+  if (InTopLevelScope(ND))
+return true;
+
+  if (const auto *EnumDecl = dyn_cast(ND.getDeclContext()))
+return InTopLevelScope(*EnumDecl) && !EnumDecl->isScoped();
+
+  return false;
 }
 
 CompletionItem CodeCompletion::render(const CodeCompleteOptions &Opts) const {


Index: clang-tools-extra/trunk/clangd/CodeComplete.cpp
===
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp
@@ -37,7 +37,6 @@
 #include "index/Index.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
-#include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Format/Format.h"
@@ -1645,14 +1644,24 @@
 }
 
 bool isIndexedForCodeCompletion(const NamedDecl &ND, ASTContext &ASTCtx) {
-  using namespace clang::ast_matchers;
-  auto InTopLevelScope = hasDeclContext(
-  anyOf(namespaceDecl(), translationUnitDecl(), linkageSpecDecl()));
-  return !match(decl(anyOf(InTopLevelScope,
-   hasDeclContext(
-   enumDecl(InTopLevelScope, unless(isScoped()),
-ND, ASTCtx)
-  .empty();
+  auto InTopLevelScope = [](const NamedDecl &ND) {
+switch (ND.getDeclContext()->getDeclKind()) {
+case Decl::TranslationUnit:
+case Decl::Namespace:
+case Decl::LinkageSpec:
+  return true;
+default:
+  break;
+};
+return false;
+  };
+  if (InTopLevelScope(ND))
+return true;
+
+  if (const auto *EnumDecl = dyn_cast(ND.getDeclContext()))
+return InTopLevelScope(*EnumDecl) && !EnumDecl->isScoped();
+
+  return false;
 }
 
 CompletionItem CodeCompletion::render(const CodeCompleteOptions &Opts) const {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Daniel Krupp via Phabricator via cfe-commits]

dkrupp updated this revision to Diff 176372.
dkrupp added a comment.

new undef/defined testcase added


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

https://reviews.llvm.org/D55125

Files:
  clang-tidy/misc/RedundantExpressionCheck.cpp
  test/clang-tidy/misc-redundant-expression.cpp

Index: test/clang-tidy/misc-redundant-expression.cpp
===
--- test/clang-tidy/misc-redundant-expression.cpp
+++ test/clang-tidy/misc-redundant-expression.cpp
@@ -106,6 +106,7 @@
 
 #define COND_OP_MACRO 9
 #define COND_OP_OTHER_MACRO 9
+#define COND_OP_THIRD_MACRO COND_OP_MACRO
 int TestConditional(int x, int y) {
   int k = 0;
   k += (y < 0) ? x : x;
@@ -114,16 +115,26 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: 'true' and 'false' expressions are equivalent
   k += (y < 0) ? COND_OP_MACRO : COND_OP_MACRO;
   // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: 'true' and 'false' expressions are equivalent
+  k += (y < 0) ? COND_OP_MACRO + COND_OP_OTHER_MACRO : COND_OP_MACRO + COND_OP_OTHER_MACRO;
+  // CHECK-MESSAGES: :[[@LINE-1]]:54: warning: 'true' and 'false' expressions are equivalent
 
   // Do not match for conditional operators with a macro and a const.
   k += (y < 0) ? COND_OP_MACRO : 9;
   // Do not match for conditional operators with expressions from different macros.
   k += (y < 0) ? COND_OP_MACRO : COND_OP_OTHER_MACRO;
+  // Do not match for conditional operators when a macro is defined to another macro
+  k += (y < 0) ? COND_OP_MACRO : COND_OP_THIRD_MACRO;
+#undef COND_OP_THIRD_MACRO
+#define   COND_OP_THIRD_MACRO 8
+  k += (y < 0) ? COND_OP_MACRO : COND_OP_THIRD_MACRO;
+#undef COND_OP_THIRD_MACRO
+
   return k;
 }
 #undef COND_OP_MACRO
 #undef COND_OP_OTHER_MACRO
 
+
 // Overloaded operators that compare two instances of a struct.
 struct MyStruct {
   int x;  
Index: clang-tidy/misc/RedundantExpressionCheck.cpp
===
--- clang-tidy/misc/RedundantExpressionCheck.cpp
+++ clang-tidy/misc/RedundantExpressionCheck.cpp
@@ -598,23 +598,56 @@
   return true;
 }
 
+static bool compareToks(Token &T1, Token &T2, const SourceManager &SM) {
+  if (T1.getLength() != T2.getLength())
+return false;
+  return strncmp(SM.getCharacterData(T1.getLocation()),
+ SM.getCharacterData(T2.getLocation()), T1.getLength()) == 0;
+}
+
+//Returns true if both LhsEpxr and RhsExpr are
+//macro expressions and they are expanded
+//from different macros.
 static bool areExprsFromDifferentMacros(const Expr *LhsExpr,
 const Expr *RhsExpr,
 const ASTContext *AstCtx) {
   if (!LhsExpr || !RhsExpr)
 return false;
-
-  SourceLocation LhsLoc = LhsExpr->getExprLoc();
-  SourceLocation RhsLoc = RhsExpr->getExprLoc();
-
-  if (!LhsLoc.isMacroID() || !RhsLoc.isMacroID())
+  SourceRange Lsr = LhsExpr->getSourceRange();
+  SourceRange Rsr = RhsExpr->getSourceRange();
+  if (!Lsr.getBegin().isMacroID() || !Rsr.getBegin().isMacroID())
 return false;
-
   const SourceManager &SM = AstCtx->getSourceManager();
   const LangOptions &LO = AstCtx->getLangOpts();
 
-  return !(Lexer::getImmediateMacroName(LhsLoc, SM, LO) ==
-  Lexer::getImmediateMacroName(RhsLoc, SM, LO));
+  std::pair LsrLocInfo =
+  SM.getDecomposedLoc(SM.getExpansionLoc(Lsr.getBegin()));
+  std::pair RsrLocInfo =
+  SM.getDecomposedLoc(SM.getExpansionLoc(Rsr.getBegin()));
+  const llvm::MemoryBuffer *MB = SM.getBuffer(LsrLocInfo.first);
+
+  const char *LTokenPos = MB->getBufferStart() + LsrLocInfo.second;
+  const char *RTokenPos = MB->getBufferStart() + RsrLocInfo.second;
+  Lexer LRawLex(SM.getLocForStartOfFile(LsrLocInfo.first), LO,
+   MB->getBufferStart(), LTokenPos, MB->getBufferEnd());
+  Lexer RRawLex(SM.getLocForStartOfFile(RsrLocInfo.first), LO,
+   MB->getBufferStart(), RTokenPos, MB->getBufferEnd());
+
+  Token LTok, RTok;
+  SourceLocation LLoc, RLoc;
+  int LRem, RRem;
+  do {//we compare the expressions token-by-token
+LRawLex.LexFromRawLexer(LTok);
+RRawLex.LexFromRawLexer(RTok);
+LLoc = LTok.getLocation();
+RLoc = RTok.getLocation();
+LRem = SM.getCharacterData(SM.getExpansionLoc(Lsr.getEnd())) -
+   SM.getCharacterData(LLoc);//remaining characters until the end of expr
+RRem = SM.getCharacterData(SM.getExpansionLoc(Rsr.getEnd())) -
+   SM.getCharacterData(RLoc);
+  } while (!LTok.is(tok::eof) && !RTok.is(tok::eof) &&
+   compareToks(LTok, RTok, SM) && LRem > 0 && RRem > 0);
+  return !((LRem == 0 && RRem == 0) && compareToks(LTok, RTok, SM));
 }
 
 static bool areExprsMacroAndNonMacro(const Expr *&LhsExpr,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Daniel Krupp via Phabricator via cfe-commits]

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



Comment at: test/clang-tidy/misc-redundant-expression.cpp:109
 #define COND_OP_OTHER_MACRO 9
+#define COND_OP_THIRD_MACRO COND_OP_MACRO
 int TestConditional(int x, int y) {

JonasToth wrote:
> dkrupp wrote:
> > JonasToth wrote:
> > > Could you please add a test where the macro is `undef`ed and redefined to 
> > > something else?
> > I am not sure what you exactly suggest here. It should not matter what 
> > COND_OP_THIRD_MACRO is defined to as we lexically compare tokens as they 
> > are written in the original source code.
> > 
> > Could you be a bit more specific what test case you would like to add? 
> *should* not matter is my point, please show that :)
undef/define testcase added and passes. @JonasToth is this what you thought of?


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

https://reviews.llvm.org/D55125



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

r348131 - [AST] Fix an uninitialized bug in the bits of FunctionDecl

[Bruno Ricci via cfe-commits]

Author: brunoricci
Date: Mon Dec  3 05:04:10 2018
New Revision: 348131

URL: http://llvm.org/viewvc/llvm-project?rev=348131&view=rev
Log:
[AST] Fix an uninitialized bug in the bits of FunctionDecl

FunctionDeclBits.IsCopyDeductionCandidate was not initialized.
This caused a warning with valgrind.


Modified:
cfe/trunk/lib/AST/Decl.cpp

Modified: cfe/trunk/lib/AST/Decl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=348131&r1=348130&r2=348131&view=diff
==
--- cfe/trunk/lib/AST/Decl.cpp (original)
+++ cfe/trunk/lib/AST/Decl.cpp Mon Dec  3 05:04:10 2018
@@ -2653,27 +2653,29 @@ FunctionDecl::FunctionDecl(Kind DK, ASTC
   DeclContext(DK), redeclarable_base(C), ODRHash(0),
   EndRangeLoc(NameInfo.getEndLoc()), DNLoc(NameInfo.getInfo()) {
   assert(T.isNull() || T->isFunctionType());
-  setStorageClass(S);
-  setInlineSpecified(isInlineSpecified);
-  setExplicitSpecified(false);
-  setVirtualAsWritten(false);
-  setPure(false);
-  setHasInheritedPrototype(false);
-  setHasWrittenPrototype(true);
-  setDeletedAsWritten(false);
-  setTrivial(false);
-  setTrivialForCall(false);
-  setDefaulted(false);
-  setExplicitlyDefaulted(false);
-  setHasImplicitReturnZero(false);
-  setLateTemplateParsed(false);
-  setConstexpr(isConstexprSpecified);
-  setInstantiationIsPending(false);
-  setUsesSEHTry(false);
-  setHasSkippedBody(false);
-  setWillHaveBody(false);
-  setIsMultiVersion(false);
-  setHasODRHash(false);
+  FunctionDeclBits.SClass = S;
+  FunctionDeclBits.IsInline = isInlineSpecified;
+  FunctionDeclBits.IsInlineSpecified = isInlineSpecified;
+  FunctionDeclBits.IsExplicitSpecified = false;
+  FunctionDeclBits.IsVirtualAsWritten = false;
+  FunctionDeclBits.IsPure = false;
+  FunctionDeclBits.HasInheritedPrototype = false;
+  FunctionDeclBits.HasWrittenPrototype = true;
+  FunctionDeclBits.IsDeleted = false;
+  FunctionDeclBits.IsTrivial = false;
+  FunctionDeclBits.IsTrivialForCall = false;
+  FunctionDeclBits.IsDefaulted = false;
+  FunctionDeclBits.IsExplicitlyDefaulted = false;
+  FunctionDeclBits.HasImplicitReturnZero = false;
+  FunctionDeclBits.IsLateTemplateParsed = false;
+  FunctionDeclBits.IsConstexpr = isConstexprSpecified;
+  FunctionDeclBits.InstantiationIsPending = false;
+  FunctionDeclBits.UsesSEHTry = false;
+  FunctionDeclBits.HasSkippedBody = false;
+  FunctionDeclBits.WillHaveBody = false;
+  FunctionDeclBits.IsMultiVersion = false;
+  FunctionDeclBits.IsCopyDeductionCandidate = false;
+  FunctionDeclBits.HasODRHash = false;
 }
 
 void FunctionDecl::getNameForDiagnostic(


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

[clang-tools-extra] r348133 - [clangd] Fix a stale comment, NFC.

[Haojian Wu via cfe-commits]

Author: hokein
Date: Mon Dec  3 05:16:04 2018
New Revision: 348133

URL: http://llvm.org/viewvc/llvm-project?rev=348133&view=rev
Log:
[clangd] Fix a stale comment, NFC.

Modified:
clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
clang-tools-extra/trunk/clangd/index/SymbolCollector.h

Modified: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp?rev=348133&r1=348132&r2=348133&view=diff
==
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp Mon Dec  3 
05:16:04 2018
@@ -260,7 +260,7 @@ void SymbolCollector::initialize(ASTCont
 }
 
 bool SymbolCollector::shouldCollectSymbol(const NamedDecl &ND,
-  ASTContext &ASTCtx,
+  const ASTContext &ASTCtx,
   const Options &Opts) {
   if (ND.isImplicit())
 return false;

Modified: clang-tools-extra/trunk/clangd/index/SymbolCollector.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolCollector.h?rev=348133&r1=348132&r2=348133&view=diff
==
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.h (original)
+++ clang-tools-extra/trunk/clangd/index/SymbolCollector.h Mon Dec  3 05:16:04 
2018
@@ -80,8 +80,7 @@ public:
   SymbolCollector(Options Opts);
 
   /// Returns true is \p ND should be collected.
-  /// AST matchers require non-const ASTContext.
-  static bool shouldCollectSymbol(const NamedDecl &ND, ASTContext &ASTCtx,
+  static bool shouldCollectSymbol(const NamedDecl &ND, const ASTContext 
&ASTCtx,
   const Options &Opts);
 
   void initialize(ASTContext &Ctx) override;


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

[PATCH] D54466: [Analyzer] Iterator Checkers - Use the region of the topmost base class for iterators stored in a region

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

baloghadamsoftware updated this revision to Diff 176374.
baloghadamsoftware added a comment.

Now I think I understand the terminology and the concept so I could add Doxygen 
comment. I also refactored the code as you suggested, original code was based 
on `getBaseRegion()`.


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

https://reviews.llvm.org/D54466

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
  lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
  lib/StaticAnalyzer/Core/MemRegion.cpp
  test/Analysis/iterator-range.cpp

Index: test/Analysis/iterator-range.cpp
===
--- test/Analysis/iterator-range.cpp
+++ test/Analysis/iterator-range.cpp
@@ -200,3 +200,40 @@
   ++i0;
   *++i0; // expected-warning{{Iterator accessed outside of its range}}
 }
+
+struct simple_iterator_base {
+  simple_iterator_base();
+  simple_iterator_base(const simple_iterator_base& rhs);
+  simple_iterator_base &operator=(const simple_iterator_base& rhs);
+  virtual ~simple_iterator_base();
+  bool friend operator==(const simple_iterator_base &lhs,
+ const simple_iterator_base &rhs);
+  bool friend operator!=(const simple_iterator_base &lhs,
+ const simple_iterator_base &rhs);
+private:
+  int *ptr;
+};
+
+struct simple_derived_iterator: public simple_iterator_base {
+  int& operator*();
+  int* operator->();
+  simple_iterator_base &operator++();
+  simple_iterator_base operator++(int);
+  simple_iterator_base &operator--();
+  simple_iterator_base operator--(int);
+};
+
+struct simple_container {
+  typedef simple_derived_iterator iterator;
+
+  iterator begin();
+  iterator end();
+};
+
+void good_derived(simple_container c) {
+  auto i0 = c.end();
+  if (i0 != c.end()) {
+clang_analyzer_warnIfReached();
+*i0; // no-warning
+  }
+}
Index: lib/StaticAnalyzer/Core/MemRegion.cpp
===
--- lib/StaticAnalyzer/Core/MemRegion.cpp
+++ lib/StaticAnalyzer/Core/MemRegion.cpp
@@ -1175,6 +1175,15 @@
   return R;
 }
 
+// getgetMostDerivedObjectRegion gets the region of the root class of a C++
+// class hierarchy.
+const MemRegion *MemRegion::getMostDerivedObjectRegion() const {
+  const MemRegion *R = this;
+  while (const auto *BR = dyn_cast(R))
+R = BR->getSuperRegion();
+  return R;
+}
+
 bool MemRegion::isSubRegionOf(const MemRegion *) const {
   return false;
 }
Index: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -1089,9 +1089,7 @@
 void IteratorChecker::verifyMatch(CheckerContext &C, const SVal &Iter,
   const MemRegion *Cont) const {
   // Verify match between a container and the container of an iterator
-  while (const auto *CBOR = Cont->getAs()) {
-Cont = CBOR->getSuperRegion();
-  }
+  Cont = Cont->getMostDerivedObjectRegion();
 
   auto State = C.getState();
   const auto *Pos = getIteratorPosition(State, Iter);
@@ -1125,9 +1123,7 @@
   if (!ContReg)
 return;
 
-  while (const auto *CBOR = ContReg->getAs()) {
-ContReg = CBOR->getSuperRegion();
-  }
+  ContReg = ContReg->getMostDerivedObjectRegion();
 
   // If the container already has a begin symbol then use it. Otherwise first
   // create a new one.
@@ -1151,9 +1147,7 @@
   if (!ContReg)
 return;
 
-  while (const auto *CBOR = ContReg->getAs()) {
-ContReg = CBOR->getSuperRegion();
-  }
+  ContReg = ContReg->getMostDerivedObjectRegion();
 
   // If the container already has an end symbol then use it. Otherwise first
   // create a new one.
@@ -1174,9 +1168,7 @@
 void IteratorChecker::assignToContainer(CheckerContext &C, const Expr *CE,
 const SVal &RetVal,
 const MemRegion *Cont) const {
-  while (const auto *CBOR = Cont->getAs()) {
-Cont = CBOR->getSuperRegion();
-  }
+  Cont = Cont->getMostDerivedObjectRegion();
 
   auto State = C.getState();
   auto &SymMgr = C.getSymbolManager();
@@ -1194,9 +1186,7 @@
   if (!ContReg)
 return;
 
-  while (const auto *CBOR = ContReg->getAs()) {
-ContReg = CBOR->getSuperRegion();
-  }
+  ContReg = ContReg->getMostDerivedObjectRegion();
 
   // Assignment of a new value to a container always invalidates all its
   // iterators
@@ -1211,9 +1201,7 @@
   if (!OldCont.isUndef()) {
 const auto *OldContReg = OldCont.getAsRegion();
 if (OldContReg) {
-  while (const auto *CBOR = OldContReg->getAs()) {
-OldContReg = CBOR->getSuperRegion();
-  }
+  OldContReg = OldContReg->getMostDerivedObjectRegion();
   const auto OldCData = getContainerData(State, OldContReg);
   if (OldCData) {
 if (const auto OldEndSym = OldCData->getEnd()) {
@@ -1273,9 +1261,7 @@
   if (!ContReg)

[PATCH] D51211: [Sema] Emit -Wformat properly for bitfield promotions.

[Bevin Hansson via Phabricator via cfe-commits]

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



Comment at: lib/Sema/SemaChecking.cpp:7711-7715
+  // It's an integer promotion if the destination type is the promoted
+  // source type.
+  return ICE->getCastKind() == CK_IntegralCast &&
+ From->isPromotableIntegerType() &&
+ S.Context.getPromotedIntegerType(From) == To;

aaron.ballman wrote:
> This check is not checking against the promoted type of the bit-field. See 
> `Sema::UsualArithmeticConversions()` for an example of what I'm talking 
> about. Is that expected?
I'm not entirely sure what you mean. Are you referring to the type produced by 
`isPromotableBitField`? The source type of that promotion is what we don't want 
to see in these implicit casts.

We don't want to look through promotions here if we promoted from a type which 
was the result of a bitfield promotion, and that bitfield promotion was from a 
higher ranked type to a lower ranked type. so, if we have a bitfield of type 
`short`, then promoting that to `int` is fine, and we will give a warning for 
the `short`. But if the type of the bitfield is `long`, it could be promoted to 
`int`. However, the format specifier warning will look through these promotions 
and think that we passed an expression of `long` to the function even though it 
was `int`.


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

https://reviews.llvm.org/D51211



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

[PATCH] D55188: Extract TextChildDumper class

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: include/clang/AST/ASTDumperUtils.h:22
+
+namespace ast_dumper {
+// Colors used for various parts of the AST dump

I'm not certain this namespace is useful, especially when it gets imported at 
TU scope in ASTDumper.cpp.



Comment at: include/clang/AST/ASTDumperUtils.h:96
+
+struct TextChildDumper {
+  raw_ostream &OS;

I'm not sold on the name for this class. It's a bit too generic to understand 
what it does. How about `ASTDumpLayoutFormatter` (and `ASTDumpNodeFormatter` 
for the node dumper)?



Comment at: include/clang/AST/ASTDumperUtils.h:97-110
+  raw_ostream &OS;
+  const bool ShowColors;
+
+  /// Pending[i] is an action to dump an entity at level i.
+  llvm::SmallVector, 32> Pending;
+
+  /// Indicates whether we're at the top level.

I don't think these members should be public.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55188



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

r348134 - [Sema] Avoid CallExpr::setNumArgs in Sema::BuildCallToObjectOfClassType

[Bruno Ricci via cfe-commits]

Author: brunoricci
Date: Mon Dec  3 05:23:56 2018
New Revision: 348134

URL: http://llvm.org/viewvc/llvm-project?rev=348134&view=rev
Log:
[Sema] Avoid CallExpr::setNumArgs in Sema::BuildCallToObjectOfClassType

CallExpr::setNumArgs is the only thing that prevents storing the arguments
of a call expression in a trailing array since it might resize the argument
array. setNumArgs is only called in 3 places in Sema, and for all of them it
is possible to avoid it.

This deals with the call to setNumArgs in BuildCallToObjectOfClassType.
Instead of constructing the CXXOperatorCallExpr first and later calling
setNumArgs if we have default arguments, we first construct a large
enough SmallVector, do the promotion/check of the arguments, and
then construct the CXXOperatorCallExpr.

Incidentally this also avoid reallocating the arguments when the
call operator has default arguments but this is not the primary goal.

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

Reviewed By: aaron.ballman


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

Modified: cfe/trunk/lib/Sema/SemaOverload.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOverload.cpp?rev=348134&r1=348133&r2=348134&view=diff
==
--- cfe/trunk/lib/Sema/SemaOverload.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOverload.cpp Mon Dec  3 05:23:56 2018
@@ -13257,29 +13257,14 @@ Sema::BuildCallToObjectOfClassType(Scope
   if (NewFn.isInvalid())
 return true;
 
+  // The number of argument slots to allocate in the call. If we have default
+  // arguments we need to allocate space for them as well. We additionally
+  // need one more slot for the object parameter.
+  unsigned NumArgsSlots = 1 + std::max(Args.size(), NumParams);
+
   // Build the full argument list for the method call (the implicit object
   // parameter is placed at the beginning of the list).
-  SmallVector MethodArgs(Args.size() + 1);
-  MethodArgs[0] = Object.get();
-  std::copy(Args.begin(), Args.end(), MethodArgs.begin() + 1);
-
-  // Once we've built TheCall, all of the expressions are properly
-  // owned.
-  QualType ResultTy = Method->getReturnType();
-  ExprValueKind VK = Expr::getValueKindForType(ResultTy);
-  ResultTy = ResultTy.getNonLValueExprType(Context);
-
-  CXXOperatorCallExpr *TheCall = new (Context)
-  CXXOperatorCallExpr(Context, OO_Call, NewFn.get(), MethodArgs, ResultTy,
-  VK, RParenLoc, FPOptions());
-
-  if (CheckCallReturnType(Method->getReturnType(), LParenLoc, TheCall, Method))
-return true;
-
-  // We may have default arguments. If so, we need to allocate more
-  // slots in the call for them.
-  if (Args.size() < NumParams)
-TheCall->setNumArgs(Context, NumParams + 1);
+  SmallVector MethodArgs(NumArgsSlots);
 
   bool IsError = false;
 
@@ -13291,7 +13276,7 @@ Sema::BuildCallToObjectOfClassType(Scope
 IsError = true;
   else
 Object = ObjRes;
-  TheCall->setArg(0, Object.get());
+  MethodArgs[0] = Object.get();
 
   // Check the argument types.
   for (unsigned i = 0; i != NumParams; i++) {
@@ -13320,7 +13305,7 @@ Sema::BuildCallToObjectOfClassType(Scope
   Arg = DefArg.getAs();
 }
 
-TheCall->setArg(i + 1, Arg);
+MethodArgs[i + 1] = Arg;
   }
 
   // If this is a variadic call, handle args passed through "...".
@@ -13330,14 +13315,27 @@ Sema::BuildCallToObjectOfClassType(Scope
   ExprResult Arg = DefaultVariadicArgumentPromotion(Args[i], 
VariadicMethod,
 nullptr);
   IsError |= Arg.isInvalid();
-  TheCall->setArg(i + 1, Arg.get());
+  MethodArgs[i + 1] = Arg.get();
 }
   }
 
-  if (IsError) return true;
+  if (IsError)
+return true;
 
   DiagnoseSentinelCalls(Method, LParenLoc, Args);
 
+  // Once we've built TheCall, all of the expressions are properly owned.
+  QualType ResultTy = Method->getReturnType();
+  ExprValueKind VK = Expr::getValueKindForType(ResultTy);
+  ResultTy = ResultTy.getNonLValueExprType(Context);
+
+  CXXOperatorCallExpr *TheCall = new (Context)
+  CXXOperatorCallExpr(Context, OO_Call, NewFn.get(), MethodArgs, ResultTy,
+  VK, RParenLoc, FPOptions());
+
+  if (CheckCallReturnType(Method->getReturnType(), LParenLoc, TheCall, Method))
+return true;
+
   if (CheckFunctionCall(Method, TheCall, Proto))
 return true;
 


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

[PATCH] D54900: [Sema] Avoid CallExpr::setNumArgs in Sema::BuildCallToObjectOfClassType

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL348134: [Sema] Avoid CallExpr::setNumArgs in 
Sema::BuildCallToObjectOfClassType (authored by brunoricci, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D54900?vs=175431&id=176375#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D54900

Files:
  cfe/trunk/lib/Sema/SemaOverload.cpp


Index: cfe/trunk/lib/Sema/SemaOverload.cpp
===
--- cfe/trunk/lib/Sema/SemaOverload.cpp
+++ cfe/trunk/lib/Sema/SemaOverload.cpp
@@ -13257,29 +13257,14 @@
   if (NewFn.isInvalid())
 return true;
 
+  // The number of argument slots to allocate in the call. If we have default
+  // arguments we need to allocate space for them as well. We additionally
+  // need one more slot for the object parameter.
+  unsigned NumArgsSlots = 1 + std::max(Args.size(), NumParams);
+
   // Build the full argument list for the method call (the implicit object
   // parameter is placed at the beginning of the list).
-  SmallVector MethodArgs(Args.size() + 1);
-  MethodArgs[0] = Object.get();
-  std::copy(Args.begin(), Args.end(), MethodArgs.begin() + 1);
-
-  // Once we've built TheCall, all of the expressions are properly
-  // owned.
-  QualType ResultTy = Method->getReturnType();
-  ExprValueKind VK = Expr::getValueKindForType(ResultTy);
-  ResultTy = ResultTy.getNonLValueExprType(Context);
-
-  CXXOperatorCallExpr *TheCall = new (Context)
-  CXXOperatorCallExpr(Context, OO_Call, NewFn.get(), MethodArgs, ResultTy,
-  VK, RParenLoc, FPOptions());
-
-  if (CheckCallReturnType(Method->getReturnType(), LParenLoc, TheCall, Method))
-return true;
-
-  // We may have default arguments. If so, we need to allocate more
-  // slots in the call for them.
-  if (Args.size() < NumParams)
-TheCall->setNumArgs(Context, NumParams + 1);
+  SmallVector MethodArgs(NumArgsSlots);
 
   bool IsError = false;
 
@@ -13291,7 +13276,7 @@
 IsError = true;
   else
 Object = ObjRes;
-  TheCall->setArg(0, Object.get());
+  MethodArgs[0] = Object.get();
 
   // Check the argument types.
   for (unsigned i = 0; i != NumParams; i++) {
@@ -13320,7 +13305,7 @@
   Arg = DefArg.getAs();
 }
 
-TheCall->setArg(i + 1, Arg);
+MethodArgs[i + 1] = Arg;
   }
 
   // If this is a variadic call, handle args passed through "...".
@@ -13330,14 +13315,27 @@
   ExprResult Arg = DefaultVariadicArgumentPromotion(Args[i], 
VariadicMethod,
 nullptr);
   IsError |= Arg.isInvalid();
-  TheCall->setArg(i + 1, Arg.get());
+  MethodArgs[i + 1] = Arg.get();
 }
   }
 
-  if (IsError) return true;
+  if (IsError)
+return true;
 
   DiagnoseSentinelCalls(Method, LParenLoc, Args);
 
+  // Once we've built TheCall, all of the expressions are properly owned.
+  QualType ResultTy = Method->getReturnType();
+  ExprValueKind VK = Expr::getValueKindForType(ResultTy);
+  ResultTy = ResultTy.getNonLValueExprType(Context);
+
+  CXXOperatorCallExpr *TheCall = new (Context)
+  CXXOperatorCallExpr(Context, OO_Call, NewFn.get(), MethodArgs, ResultTy,
+  VK, RParenLoc, FPOptions());
+
+  if (CheckCallReturnType(Method->getReturnType(), LParenLoc, TheCall, Method))
+return true;
+
   if (CheckFunctionCall(Method, TheCall, Proto))
 return true;
 


Index: cfe/trunk/lib/Sema/SemaOverload.cpp
===
--- cfe/trunk/lib/Sema/SemaOverload.cpp
+++ cfe/trunk/lib/Sema/SemaOverload.cpp
@@ -13257,29 +13257,14 @@
   if (NewFn.isInvalid())
 return true;
 
+  // The number of argument slots to allocate in the call. If we have default
+  // arguments we need to allocate space for them as well. We additionally
+  // need one more slot for the object parameter.
+  unsigned NumArgsSlots = 1 + std::max(Args.size(), NumParams);
+
   // Build the full argument list for the method call (the implicit object
   // parameter is placed at the beginning of the list).
-  SmallVector MethodArgs(Args.size() + 1);
-  MethodArgs[0] = Object.get();
-  std::copy(Args.begin(), Args.end(), MethodArgs.begin() + 1);
-
-  // Once we've built TheCall, all of the expressions are properly
-  // owned.
-  QualType ResultTy = Method->getReturnType();
-  ExprValueKind VK = Expr::getValueKindForType(ResultTy);
-  ResultTy = ResultTy.getNonLValueExprType(Context);
-
-  CXXOperatorCallExpr *TheCall = new (Context)
-  CXXOperatorCallExpr(Context, OO_Call, NewFn.get(), MethodArgs, ResultTy,
-  VK, RParenLoc, FPOptions());
-
-  if (CheckCallReturnType(Method->getReturnType(), LParenLoc, TheCall, Method))
-return true;
-
-  // We may have default arguments. If so, we need to allocate more
-  // slots in the call for them

[PATCH] D55191: [clangd] Refine the way of checking a declaration is referenced by the written code.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

The **idea** of using the AST-based approach here was really nice, it was less 
expensive and seemed to clearly look at the semantic.
I wonder if there's a way to keep it on the AST level, without looking at the 
source locations.

What are the cases we're trying to filter out? Only implicit constructor or 
anything else?




Comment at: unittests/clangd/XRefsTests.cpp:1228
 
+TEST(FindReferences, ExplicitSymbols) {
+  const char *Tests[] = {

I'm missing what does this test actually tests.
The absence of implicit references (I guess constructor expressions)?



Comment at: unittests/clangd/XRefsTests.cpp:1259
+  namespace ns {
+  using [fo^o];
+  }

Shouldn't the `usings` always be qualified? Isn't this a compiler error?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55191



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

r348135 - [CodeComplete] Cleanup access checking in code completion

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Mon Dec  3 05:29:17 2018
New Revision: 348135

URL: http://llvm.org/viewvc/llvm-project?rev=348135&view=rev
Log:
[CodeComplete] Cleanup access checking in code completion

Summary: Also fixes a crash (see the added 'accessibility-crash.cpp' test).

Reviewers: ioeric, kadircet

Reviewed By: kadircet

Subscribers: cfe-commits

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

Added:
cfe/trunk/test/CodeCompletion/accessibility-crash.cpp
cfe/trunk/test/CodeCompletion/accessibility.cpp
Modified:
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Parse/ParseExprCXX.cpp
cfe/trunk/lib/Sema/CodeCompleteConsumer.cpp
cfe/trunk/lib/Sema/SemaAccess.cpp
cfe/trunk/lib/Sema/SemaCodeComplete.cpp

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=348135&r1=348134&r2=348135&view=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Mon Dec  3 05:29:17 2018
@@ -6065,7 +6065,8 @@ public:
 bool ForceCheck = false,
 bool ForceUnprivileged = false);
   void CheckLookupAccess(const LookupResult &R);
-  bool IsSimplyAccessible(NamedDecl *decl, DeclContext *Ctx);
+  bool IsSimplyAccessible(NamedDecl *Decl, CXXRecordDecl *NamingClass,
+  QualType BaseType);
   bool isSpecialMemberAccessibleForDeletion(CXXMethodDecl *decl,
 AccessSpecifier access,
 QualType objectType);
@@ -10340,7 +10341,7 @@ public:
   void CodeCompleteAssignmentRHS(Scope *S, Expr *LHS);
 
   void CodeCompleteQualifiedId(Scope *S, CXXScopeSpec &SS,
-   bool EnteringContext);
+   bool EnteringContext, QualType BaseType);
   void CodeCompleteUsing(Scope *S);
   void CodeCompleteUsingDirective(Scope *S);
   void CodeCompleteNamespaceDecl(Scope *S);

Modified: cfe/trunk/lib/Parse/ParseExprCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseExprCXX.cpp?rev=348135&r1=348134&r2=348135&view=diff
==
--- cfe/trunk/lib/Parse/ParseExprCXX.cpp (original)
+++ cfe/trunk/lib/Parse/ParseExprCXX.cpp Mon Dec  3 05:29:17 2018
@@ -235,22 +235,11 @@ bool Parser::ParseOptionalCXXScopeSpecif
 
   while (true) {
 if (HasScopeSpecifier) {
-  // C++ [basic.lookup.classref]p5:
-  //   If the qualified-id has the form
-  //
-  //   ::class-name-or-namespace-name::...
-  //
-  //   the class-name-or-namespace-name is looked up in global scope as a
-  //   class-name or namespace-name.
-  //
-  // To implement this, we clear out the object type as soon as we've
-  // seen a leading '::' or part of a nested-name-specifier.
-  ObjectType = nullptr;
-
   if (Tok.is(tok::code_completion)) {
 // Code completion for a nested-name-specifier, where the code
 // completion token follows the '::'.
-Actions.CodeCompleteQualifiedId(getCurScope(), SS, EnteringContext);
+Actions.CodeCompleteQualifiedId(getCurScope(), SS, EnteringContext,
+ObjectType.get());
 // Include code completion token into the range of the scope otherwise
 // when we try to annotate the scope tokens the dangling code 
completion
 // token will cause assertion in
@@ -259,6 +248,18 @@ bool Parser::ParseOptionalCXXScopeSpecif
 cutOffParsing();
 return true;
   }
+
+  // C++ [basic.lookup.classref]p5:
+  //   If the qualified-id has the form
+  //
+  //   ::class-name-or-namespace-name::...
+  //
+  //   the class-name-or-namespace-name is looked up in global scope as a
+  //   class-name or namespace-name.
+  //
+  // To implement this, we clear out the object type as soon as we've
+  // seen a leading '::' or part of a nested-name-specifier.
+  ObjectType = nullptr;
 }
 
 // nested-name-specifier:

Modified: cfe/trunk/lib/Sema/CodeCompleteConsumer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/CodeCompleteConsumer.cpp?rev=348135&r1=348134&r2=348135&view=diff
==
--- cfe/trunk/lib/Sema/CodeCompleteConsumer.cpp (original)
+++ cfe/trunk/lib/Sema/CodeCompleteConsumer.cpp Mon Dec  3 05:29:17 2018
@@ -555,6 +555,9 @@ void PrintingCodeCompleteConsumer::Proce
   Tags.push_back("Hidden");
 if (Results[I].InBaseClass)
   Tags.push_back("InBase");
+if (Results[I].Availability ==
+CXAvailabilityKind::CXAvailability_NotAccessible)
+  Tags.push_back("Inaccessible");
 if (!Tags.empty())
  

[PATCH] D55124: [CodeComplete] Cleanup access checking in code completion

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL348135: [CodeComplete] Cleanup access checking in code 
completion (authored by ibiryukov, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D55124

Files:
  cfe/trunk/include/clang/Sema/Sema.h
  cfe/trunk/lib/Parse/ParseExprCXX.cpp
  cfe/trunk/lib/Sema/CodeCompleteConsumer.cpp
  cfe/trunk/lib/Sema/SemaAccess.cpp
  cfe/trunk/lib/Sema/SemaCodeComplete.cpp
  cfe/trunk/test/CodeCompletion/accessibility-crash.cpp
  cfe/trunk/test/CodeCompletion/accessibility.cpp

Index: cfe/trunk/include/clang/Sema/Sema.h
===
--- cfe/trunk/include/clang/Sema/Sema.h
+++ cfe/trunk/include/clang/Sema/Sema.h
@@ -6065,7 +6065,8 @@
 bool ForceCheck = false,
 bool ForceUnprivileged = false);
   void CheckLookupAccess(const LookupResult &R);
-  bool IsSimplyAccessible(NamedDecl *decl, DeclContext *Ctx);
+  bool IsSimplyAccessible(NamedDecl *Decl, CXXRecordDecl *NamingClass,
+  QualType BaseType);
   bool isSpecialMemberAccessibleForDeletion(CXXMethodDecl *decl,
 AccessSpecifier access,
 QualType objectType);
@@ -10340,7 +10341,7 @@
   void CodeCompleteAssignmentRHS(Scope *S, Expr *LHS);
 
   void CodeCompleteQualifiedId(Scope *S, CXXScopeSpec &SS,
-   bool EnteringContext);
+   bool EnteringContext, QualType BaseType);
   void CodeCompleteUsing(Scope *S);
   void CodeCompleteUsingDirective(Scope *S);
   void CodeCompleteNamespaceDecl(Scope *S);
Index: cfe/trunk/test/CodeCompletion/accessibility.cpp
===
--- cfe/trunk/test/CodeCompletion/accessibility.cpp
+++ cfe/trunk/test/CodeCompletion/accessibility.cpp
@@ -0,0 +1,73 @@
+class X {
+public:
+ int pub;
+protected:
+ int prot;
+private:
+ int priv;
+};
+
+class Unrelated {
+public:
+  static int pub;
+protected:
+  static int prot;
+private:
+  static int priv;
+};
+
+class Y : public X {
+  int test() {
+this->pub = 10;
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:21:11 %s -o - \
+// RUN: | FileCheck -check-prefix=THIS %s
+// THIS: priv (InBase,Inaccessible)
+// THIS: prot (InBase)
+// THIS: pub (InBase)
+//
+// Also check implicit 'this->', i.e. complete at the start of the line.
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:21:1 %s -o - \
+// RUN: | FileCheck -check-prefix=THIS %s
+
+X().pub + 10;
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:32:9 %s -o - \
+// RUN: | FileCheck -check-prefix=X-OBJ %s
+// X-OBJ: priv (Inaccessible)
+// X-OBJ: prot (Inaccessible)
+// X-OBJ: pub : [#int#]pub
+
+Y().pub + 10;
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:39:9 %s -o - \
+// RUN: | FileCheck -check-prefix=Y-OBJ %s
+// Y-OBJ: priv (InBase,Inaccessible)
+// Y-OBJ: prot (InBase)
+// Y-OBJ: pub (InBase)
+
+this->X::pub = 10;
+X::pub = 10;
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:46:14 %s -o - \
+// RUN: | FileCheck -check-prefix=THIS-BASE %s
+//
+// THIS-BASE: priv (Inaccessible)
+// THIS-BASE: prot : [#int#]prot
+// THIS-BASE: pub : [#int#]pub
+//
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:47:8 %s -o - \
+// RUN: | FileCheck -check-prefix=THIS-BASE %s
+
+
+this->Unrelated::pub = 10; // a check we don't crash in this cases.
+Y().Unrelated::pub = 10; // a check we don't crash in this cases.
+Unrelated::pub = 10;
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:59:22 %s -o - \
+// RUN: | FileCheck -check-prefix=UNRELATED %s
+// UNRELATED: priv (Inaccessible)
+// UNRELATED: prot (Inaccessible)
+// UNRELATED: pub : [#int#]pub
+//
+// RUN: not %clang_cc1 -fsyntax-only -code-completion-at=%s:60:20 %s -o - \
+// RUN: | FileCheck -check-prefix=UNRELATED %s
+// RUN: not %clang_cc1 -fsyntax-only -code-completion-at=%s:61:16 %s -o - \
+// RUN: | FileCheck -check-prefix=UNRELATED %s
+  }
+};
Index: cfe/trunk/test/CodeCompletion/accessibility-crash.cpp
===
--- cfe/trunk/test/CodeCompletion/accessibility-crash.cpp
+++ cfe/trunk/test/CodeCompletion/accessibility-crash.cpp
@@ -0,0 +1,23 @@
+class X {
+public:
+ int pub;
+protected:
+ int prot;
+private:
+ int priv;
+};
+
+class Y : public X {
+  int test() {
+[]() {
+
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:13:1 %s -o - \
+  // RUN: | FileCheck %s
+  // CHECK: priv (InBase,Inaccessible)
+  // CHECK: prot (InBase)
+  

[PATCH] D54903: [Sema] Improve static_assert diagnostics.

[Clement Courbet via Phabricator via cfe-commits]

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



Comment at: lib/Sema/SemaTemplate.cpp:3055
 
+// Print a diagnostic for the failing static_assert expression. Defaults to
+// pretty-printing the expression.

aaron.ballman wrote:
> Comment is a bit out of date as this is no longer specific to `static_assert`.
> 
> It looks like this may also change the behavior of enable_if diagnostic 
> reporting. Do you need to update any of those tests from this change? If not, 
> can you devise some tests for that case as well to show that this isn't just 
> a static_assert behavior? (Do a search for `findFailedBooleanCondition` to 
> find where the behavior has changed.)
Nope, unfortunately the code that calls this is actually untested...
I could not find a test that actually enters the:
```
if (TypeAliasTemplateDecl *AliasTemplate =
  dyn_cast(Template))
```
in `Sema::CheckTemplateIdType`.

I stopped at the following insanity:

```
// RUN: %clang_cc1 -std=c++11 -verify %s

template 
struct is_same {
  enum { value = 0 };
};

template  struct is_same {
  enum { value = 1 };
};

struct Dispatch {
  template  using SameAs = is_same;
};

template 
struct S {
  template  using same = typename DispatchT::template SameAs;

  template 
  static void foo() __attribute__((enable_if(same::value, "")));

};

void runFoo() {
  // S::foo();
  S::foo();

}
```

This one exercises the code up to `if (CanonType.isNull()) {`, but I'm not sure 
how to get a null type here.



Repository:
  rC Clang

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

https://reviews.llvm.org/D54903



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

[PATCH] D55139: [clangd] Avoid memory-mapping files on Windows

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 176381.
ilya-biryukov marked an inline comment as done.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

- Keep using memory-mapped files for PCHs
- Move once


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55139

Files:
  clangd/CMakeLists.txt
  clangd/FSProvider.cpp
  clangd/FSProvider.h

Index: clangd/FSProvider.h
===
--- clangd/FSProvider.h
+++ clangd/FSProvider.h
@@ -33,9 +33,7 @@
 public:
   // FIXME: returns the single real FS instance, which is not threadsafe.
   llvm::IntrusiveRefCntPtr
-  getFileSystem() const override {
-return llvm::vfs::getRealFileSystem();
-  }
+  getFileSystem() const override;
 };
 
 } // namespace clangd
Index: clangd/FSProvider.cpp
===
--- /dev/null
+++ clangd/FSProvider.cpp
@@ -0,0 +1,85 @@
+//===--- FSProvider.cpp - VFS provider for ClangdServer ---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "FSProvider.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/VirtualFileSystem.h"
+#include 
+
+using namespace llvm;
+
+namespace clang {
+namespace clangd {
+
+namespace {
+/// Always opens files in the underlying filesystem as "volatile", meaning they
+/// won't be memory-mapped. This avoid locking the files on Windows.
+class VolatileFSProvider : public llvm::vfs::ProxyFileSystem {
+public:
+  explicit VolatileFSProvider(llvm::IntrusiveRefCntPtr FS)
+  : ProxyFileSystem(std::move(FS)) {}
+
+  llvm::ErrorOr>
+  openFileForRead(const Twine &InPath) override {
+SmallString<128> Path;
+InPath.toVector(Path);
+
+auto File = getUnderlyingFS().openFileForRead(Path);
+if (!File)
+  return File;
+// Try to guess preamble files, they can be memory-mapped even on Windows as
+// clangd has exclusive access to those.
+StringRef FileName = llvm::sys::path::filename(Path);
+if (FileName.startswith("preamble-") && FileName.endswith(".pch"))
+  return File;
+return std::unique_ptr(
+new VolatileFile(std::move(*File)));
+  }
+
+private:
+  class VolatileFile : public vfs::File {
+  public:
+VolatileFile(std::unique_ptr Wrapped)
+: Wrapped(std::move(Wrapped)) {
+  assert(this->Wrapped);
+}
+
+virtual llvm::ErrorOr>
+getBuffer(const Twine &Name, int64_t FileSize, bool RequiresNullTerminator,
+  bool /*IsVolatile*/) override {
+  return Wrapped->getBuffer(Name, FileSize, RequiresNullTerminator,
+/*IsVolatile=*/true);
+}
+
+llvm::ErrorOr status() override { return Wrapped->status(); }
+llvm::ErrorOr getName() override { return Wrapped->getName(); }
+std::error_code close() override { return Wrapped->close(); }
+
+  private:
+std::unique_ptr Wrapped;
+  };
+};
+} // namespace
+
+llvm::IntrusiveRefCntPtr
+clang::clangd::RealFileSystemProvider::getFileSystem() const {
+// Avoid using memory-mapped files on Windows, they cause file locking issues.
+// FIXME: Try to use a similar approach in Sema instead of relying on
+//error-prone propagation of the 'isVolatile' flag through all layers.
+#ifdef _WIN32
+  return new VolatileFSProvider(vfs::getRealFileSystem());
+#else
+  return vfs::getRealFileSystem();
+#endif
+}
+} // namespace clangd
+} // namespace clang
Index: clangd/CMakeLists.txt
===
--- clangd/CMakeLists.txt
+++ clangd/CMakeLists.txt
@@ -23,6 +23,7 @@
   FindSymbols.cpp
   FileDistance.cpp
   FS.cpp
+  FSProvider.cpp
   FuzzyMatch.cpp
   GlobalCompilationDatabase.cpp
   Headers.cpp
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55139: [clangd] Avoid memory-mapping files on Windows

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov marked an inline comment as done.
ilya-biryukov added inline comments.



Comment at: clangd/FSProvider.cpp:33
+  return File;
+return std::unique_ptr(
+new VolatileFile(std::move(std::move(*File;

kadircet wrote:
> make_unique?
Unfortunately it doesn't work here - `VolatileFile` is private in this class.



Comment at: clangd/FSProvider.cpp:34
+return std::unique_ptr(
+new VolatileFile(std::move(std::move(*File;
+  }

kadircet wrote:
> can we move once ?
We need moar moves!!! xD

Thanks for noticing, fixed.




Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55139



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

[PATCH] D55139: [clangd] Avoid memory-mapping files on Windows

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 176382.
ilya-biryukov marked an inline comment as done.
ilya-biryukov added a comment.

- s/VolatileFSProvider/VolatileFileSystem


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55139

Files:
  clangd/CMakeLists.txt
  clangd/FSProvider.cpp
  clangd/FSProvider.h

Index: clangd/FSProvider.h
===
--- clangd/FSProvider.h
+++ clangd/FSProvider.h
@@ -33,9 +33,7 @@
 public:
   // FIXME: returns the single real FS instance, which is not threadsafe.
   llvm::IntrusiveRefCntPtr
-  getFileSystem() const override {
-return llvm::vfs::getRealFileSystem();
-  }
+  getFileSystem() const override;
 };
 
 } // namespace clangd
Index: clangd/FSProvider.cpp
===
--- /dev/null
+++ clangd/FSProvider.cpp
@@ -0,0 +1,85 @@
+//===--- FSProvider.cpp - VFS provider for ClangdServer ---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "FSProvider.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/VirtualFileSystem.h"
+#include 
+
+using namespace llvm;
+
+namespace clang {
+namespace clangd {
+
+namespace {
+/// Always opens files in the underlying filesystem as "volatile", meaning they
+/// won't be memory-mapped. This avoid locking the files on Windows.
+class VolatileFileSystem : public llvm::vfs::ProxyFileSystem {
+public:
+  explicit VolatileFileSystem(llvm::IntrusiveRefCntPtr FS)
+  : ProxyFileSystem(std::move(FS)) {}
+
+  llvm::ErrorOr>
+  openFileForRead(const Twine &InPath) override {
+SmallString<128> Path;
+InPath.toVector(Path);
+
+auto File = getUnderlyingFS().openFileForRead(Path);
+if (!File)
+  return File;
+// Try to guess preamble files, they can be memory-mapped even on Windows as
+// clangd has exclusive access to those.
+StringRef FileName = llvm::sys::path::filename(Path);
+if (FileName.startswith("preamble-") && FileName.endswith(".pch"))
+  return File;
+return std::unique_ptr(
+new VolatileFile(std::move(*File)));
+  }
+
+private:
+  class VolatileFile : public vfs::File {
+  public:
+VolatileFile(std::unique_ptr Wrapped)
+: Wrapped(std::move(Wrapped)) {
+  assert(this->Wrapped);
+}
+
+virtual llvm::ErrorOr>
+getBuffer(const Twine &Name, int64_t FileSize, bool RequiresNullTerminator,
+  bool /*IsVolatile*/) override {
+  return Wrapped->getBuffer(Name, FileSize, RequiresNullTerminator,
+/*IsVolatile=*/true);
+}
+
+llvm::ErrorOr status() override { return Wrapped->status(); }
+llvm::ErrorOr getName() override { return Wrapped->getName(); }
+std::error_code close() override { return Wrapped->close(); }
+
+  private:
+std::unique_ptr Wrapped;
+  };
+};
+} // namespace
+
+llvm::IntrusiveRefCntPtr
+clang::clangd::RealFileSystemProvider::getFileSystem() const {
+// Avoid using memory-mapped files on Windows, they cause file locking issues.
+// FIXME: Try to use a similar approach in Sema instead of relying on
+//error-prone propagation of the 'isVolatile' flag through all layers.
+#ifdef _WIN32
+  return new VolatileFSProvider(vfs::getRealFileSystem());
+#else
+  return vfs::getRealFileSystem();
+#endif
+}
+} // namespace clangd
+} // namespace clang
Index: clangd/CMakeLists.txt
===
--- clangd/CMakeLists.txt
+++ clangd/CMakeLists.txt
@@ -23,6 +23,7 @@
   FindSymbols.cpp
   FileDistance.cpp
   FS.cpp
+  FSProvider.cpp
   FuzzyMatch.cpp
   GlobalCompilationDatabase.cpp
   Headers.cpp
___
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 inline comments.



Comment at: clang/lib/Sema/SemaExpr.cpp:1304
+RHSTy = ResultTy;
+  }
+

rjmccall wrote:
> leonardchan wrote:
> > rjmccall wrote:
> > > ebevhan wrote:
> > > > rjmccall wrote:
> > > > > Hmm.  So adding a signed integer to an unsigned fixed-point type 
> > > > > always converts the integer to unsigned?  That's a little weird, but 
> > > > > only because the fixed-point rule seems to be the other way.  Anyway, 
> > > > > I assume it's not a bug in the spec.
> > > > `handleFixedPointConversion` only calculates the result type of the 
> > > > expression, not the calculation type. The final result type of the 
> > > > operation will be the unsigned fixed-point type, but the calculation 
> > > > itself will be done in a signed type with enough precision to represent 
> > > > both the signed integer and the unsigned fixed-point type. 
> > > > 
> > > > Though, if the result ends up being negative, the final result is 
> > > > undefined unless the type is saturating. I don't think there is a test 
> > > > for the saturating case (signed integer + unsigned saturating 
> > > > fixed-point) in the SaturatedAddition tests.
> > > > `handleFixedPointConversion` only calculates the result type of the 
> > > > expression, not the calculation type.
> > > 
> > > Right, I understand that, but the result type of the expression obviously 
> > > impacts the expressive range of the result, since you can end up with a 
> > > negative value.
> > > 
> > > Hmm.  With that said, if the general principle is to perform the 
> > > operation with full precision on the original operand values, why are 
> > > unsigned fixed-point operands coerced to their corresponding signed types 
> > > *before* the operation?  This is precision-limiting if the unsigned 
> > > representation doesn't use a padding bit.  That seems like a bug in the 
> > > spec.
> > > Hmm. With that said, if the general principle is to perform the operation 
> > > with full precision on the original operand values, why are unsigned 
> > > fixed-point operands coerced to their corresponding signed types *before* 
> > > the operation? This is precision-limiting if the unsigned representation 
> > > doesn't use a padding bit. That seems like a bug in the spec.
> > 
> > Possibly. When the standard mentions converting from signed to unsigned 
> > fixed point, the only requirement involved is conservation of magnitude 
> > (the number of integral bits excluding the sign)
> > 
> > ```
> > when signed and unsigned fixed-point types are mixed, the unsigned type is 
> > converted to the corresponding signed type, and this should go without loss 
> > of magnitude
> > ```
> > 
> > This is just speculation, but I'm under the impression that not as much 
> > "attention" was given for unsigned types as for signed types since "`In the 
> > embedded processor world, support for unsigned fixed-point data types is 
> > rare; normally only signed fixed-point data types are supported`", but was 
> > kept anyway since unsigned types are used a lot.
> > 
> > ```
> > However, to disallow unsigned fixed-point arithmetic from programming 
> > languages (in general, and from C in particular) based on this observation, 
> > seems overly restrictive.
> > ```
> > 
> > I also imagine that if the programmer needs more precision, the correct 
> > approach would be to cast up to a type with a higher scale. The standard 
> > seems to make an effort to expose as much in regards to the underlying 
> > fixed point types as possible:
> > 
> > ```
> > it should be possible to write fixed-point algorithms that are independent 
> > of the actual fixed-point hardware support. This implies that a programmer 
> > (or a running program) should have access to all parameters that define the 
> > behavior of the underlying hardware (in other words: even if these 
> > parameters are implementation-defined).
> > ```
> > 
> > So the user would at least know that unsigned types may not have the same 
> > scale as their corresponding signed types if the hardware handles them with 
> > different scales.
> > 
> > Also added test for signed integer + unsigned saturating fixed point.
> As long as we maintain the same typing behavior, does the standard permit us 
> to just Do The Right Thing here and do the extended arithmetic with the 
> original unsigned operand?  I'm sure there are some cases where we would 
> produce a slightly different value than an implementation that does the 
> coercion before the operation, but that might be permitted under the 
> standard, and as you say, it would only affect some situations that it 
> doesn't seem the standard has given much thought to.
The coercion of unsigned to signed is likely done for pragmatic reasons; if you 
have a signed and unsigned type of the same rank, operating on them together 
won't require any 'extra bits'. This means that if your hardware has native 
support for the operations, you won't end up in a si

r348139 - [OpenCL][Sema] Improving formatting

[Marco Antognini via cfe-commits]

Author: mantognini
Date: Mon Dec  3 06:03:49 2018
New Revision: 348139

URL: http://llvm.org/viewvc/llvm-project?rev=348139&view=rev
Log:
[OpenCL][Sema] Improving formatting

Reformat comment added in r348120 following
review https://reviews.llvm.org/D55136.


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

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=348139&r1=348138&r2=348139&view=diff
==
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Mon Dec  3 06:03:49 2018
@@ -5567,9 +5567,8 @@ Sema::BuildResolvedCallExpr(Expr *Fn, Na
   Fn->getType()->isSpecificBuiltinType(BuiltinType::BuiltinFn)) {
 // Extract the return type from the (builtin) function pointer type.
 // FIXME Several builtins still have setType in
-//   Sema::CheckBuiltinFunctionCall. One should review their
-//   definitions in Builtins.def to ensure they are correct before
-//   removing setType calls.
+// Sema::CheckBuiltinFunctionCall. One should review their definitions in
+// Builtins.def to ensure they are correct before removing setType calls.
 QualType FnPtrTy = Context.getPointerType(FDecl->getType());
 Result = ImpCastExprToType(Fn, FnPtrTy, CK_BuiltinFnToFnPtr).get();
 ResultTy = FDecl->getCallResultType();


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

[PATCH] D55188: Extract TextChildDumper class

[Stephen Kelly via Phabricator via cfe-commits]

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



Comment at: include/clang/AST/ASTDumperUtils.h:22
+
+namespace ast_dumper {
+// Colors used for various parts of the AST dump

aaron.ballman wrote:
> I'm not certain this namespace is useful, especially when it gets imported at 
> TU scope in ASTDumper.cpp.
> it gets imported at TU scope in ASTDumper.cpp

Today that's the only place it is used. In the future that won't be true.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55188



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

[PATCH] D55212: Handle alloc_size attribute on function pointers

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: include/clang/Basic/Attr.td:1072
 def AllocSize : InheritableAttr {
   let Spellings = [GCC<"alloc_size">];
+  let Subjects = SubjectList<[HasFunctionProto]>;

Does GCC support writing `alloc_size` on function pointers?



Comment at: lib/Sema/SemaDecl.cpp:6269
+static void copyAttrFromTypedefToDecl(Sema &S, Decl *D, const TypedefType *TT) 
{
+  TypedefNameDecl *TND = TT->getDecl();
+  if (AttrTy *Attribute = TND->getAttr()) {

`const TypedefNameDecl *` ?



Comment at: lib/Sema/SemaDecl.cpp:6270
+  TypedefNameDecl *TND = TT->getDecl();
+  if (AttrTy *Attribute = TND->getAttr()) {
+AttrTy *Clone = Attribute->clone(S.Context);

`const auto *`



Comment at: lib/Sema/SemaDecl.cpp:6723
+  if (R->isFunctionPointerType()) {
+if (auto TT = R->getAs()) {
+  copyAttrFromTypedefToDecl(*this, NewVD, TT);

`const auto *` and can elide the braces.

This should probably be generalized at some point into something declarative in 
Attr.td, though the type checking may be hard to do that for (I'm not certain). 
It doesn't need to be done as part of this patch, though.



Comment at: test/Sema/alloc-size.c:46
+// This typedef applies the alloc_size to the pointer to the function pointer 
and should not be allowed
+void *(**__attribute__((alloc_size(1, 2))) * allocator_function_typdef4)(int, 
int); // expected-warning{{'alloc_size' attribute only applies to non-K&R-style 
functions}}

What should happen in these situations?
```
typedef void * (__attribute__((alloc_size(1))) * 
my_malloc_fn_pointer_type)(int);
typedef void * (* my_other_malloc_fn_pointer_type)(int);

void *fn(int i);
__attribute__((alloc_size(1))) void *fn2(int i);

int main() {
  my_malloc_fn_pointer_type f = fn; // ???
  my_other_malloc_fn_pointer_type f2 = fn2; // ???
}
```
Should this code do something special?
```
typedef void * (__attribute__((alloc_size(1))) * 
my_malloc_fn_pointer_type)(int);
typedef void * (* my_other_malloc_fn_pointer_type)(int);

void overloaded(my_malloc_fn_pointer_type fn);
void overloaded(my_other_malloc_fn_pointer_type fn);

void *fn(int i);
__attribute__((alloc_size(1))) void *fn2(int i);

int main() {
  overloaded(fn);
  overloaded(fn2);
}

```


Repository:
  rC Clang

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

https://reviews.llvm.org/D55212



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

[PATCH] D55188: Extract TextChildDumper class

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: include/clang/AST/ASTDumperUtils.h:22
+
+namespace ast_dumper {
+// Colors used for various parts of the AST dump

steveire wrote:
> aaron.ballman wrote:
> > I'm not certain this namespace is useful, especially when it gets imported 
> > at TU scope in ASTDumper.cpp.
> > it gets imported at TU scope in ASTDumper.cpp
> 
> Today that's the only place it is used. In the future that won't be true.
Then we can add the namespace in the future when we find a situation where 
we're worried about collisions? As it stands, this only provides the illusion 
of safety. If you really want to keep it, please pull the global using 
declaration.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55188



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

[PATCH] D51211: [Sema] Emit -Wformat properly for bitfield promotions.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: lib/Sema/SemaChecking.cpp:7711-7715
+  // It's an integer promotion if the destination type is the promoted
+  // source type.
+  return ICE->getCastKind() == CK_IntegralCast &&
+ From->isPromotableIntegerType() &&
+ S.Context.getPromotedIntegerType(From) == To;

ebevhan wrote:
> aaron.ballman wrote:
> > This check is not checking against the promoted type of the bit-field. See 
> > `Sema::UsualArithmeticConversions()` for an example of what I'm talking 
> > about. Is that expected?
> I'm not entirely sure what you mean. Are you referring to the type produced 
> by `isPromotableBitField`? The source type of that promotion is what we don't 
> want to see in these implicit casts.
> 
> We don't want to look through promotions here if we promoted from a type 
> which was the result of a bitfield promotion, and that bitfield promotion was 
> from a higher ranked type to a lower ranked type. so, if we have a bitfield 
> of type `short`, then promoting that to `int` is fine, and we will give a 
> warning for the `short`. But if the type of the bitfield is `long`, it could 
> be promoted to `int`. However, the format specifier warning will look through 
> these promotions and think that we passed an expression of `long` to the 
> function even though it was `int`.
Ahhh, okay, I see what's going on then. This makes more sense to me now, thank 
you for the explanation.



Comment at: test/Sema/format-strings-bitfield-promotion.c:1
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -verify %s
+

aaron.ballman wrote:
> Running your test through GCC looks like the behavior matches here for C; can 
> you also add a C++ test that demonstrates the behavior does not change?
> 
> https://godbolt.org/z/zRYDMG
Strangely, the above godbolt link dropped the output windows, here's a 
different link that shows the behavioral differences between C and C++ mode in 
GCC: https://godbolt.org/z/R3zRHe


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

https://reviews.llvm.org/D51211



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

[PATCH] D55135: [CTU][Analyzer]Add DisplayCTUProgress analyzer switch

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

Szelethus added a comment.

Also, `AnalyzerOptions.def` was recently clan-formatted, feel free to run it 
again after the changes you make in it.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55135



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

[PATCH] D32950: Support C++1z features in `__has_extension`

[Filipe Cabecinhas via Phabricator via cfe-commits]

filcab added a comment.

Hi Eric,

I know this is old, but are you interested in reviving this patch? I don't know 
enough about clang's extensions to LGTM such a patch (updated for the current 
code), but would really like to have a way to know if extensions are supported.
We just now had people ask how to detect if `if constexpr` is available, and 
not having a feature check makes it much harder to figure out (and makes our 
test based on current clang behaviour).

Thank you,
Filipe


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

https://reviews.llvm.org/D32950



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

[PATCH] D54903: [Sema] Improve static_assert diagnostics.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: lib/Sema/SemaTemplate.cpp:3055
 
+// Print a diagnostic for the failing static_assert expression. Defaults to
+// pretty-printing the expression.

courbet wrote:
> aaron.ballman wrote:
> > Comment is a bit out of date as this is no longer specific to 
> > `static_assert`.
> > 
> > It looks like this may also change the behavior of enable_if diagnostic 
> > reporting. Do you need to update any of those tests from this change? If 
> > not, can you devise some tests for that case as well to show that this 
> > isn't just a static_assert behavior? (Do a search for 
> > `findFailedBooleanCondition` to find where the behavior has changed.)
> Nope, unfortunately the code that calls this is actually untested...
> I could not find a test that actually enters the:
> ```
> if (TypeAliasTemplateDecl *AliasTemplate =
>   dyn_cast(Template))
> ```
> in `Sema::CheckTemplateIdType`.
> 
> I stopped at the following insanity:
> 
> ```
> // RUN: %clang_cc1 -std=c++11 -verify %s
> 
> template 
> struct is_same {
>   enum { value = 0 };
> };
> 
> template  struct is_same {
>   enum { value = 1 };
> };
> 
> struct Dispatch {
>   template  using SameAs = is_same;
> };
> 
> template 
> struct S {
>   template  using same = typename DispatchT::template SameAs;
> 
>   template 
>   static void foo() __attribute__((enable_if(same::value, "")));
> 
> };
> 
> void runFoo() {
>   // S::foo();
>   S::foo();
> 
> }
> ```
> 
> This one exercises the code up to `if (CanonType.isNull()) {`, but I'm not 
> sure how to get a null type here.
> 
How about this:
```
namespace boost {
  template struct enable_if {};
  template struct enable_if { typedef T type; };
}

template struct NonTemplateFunction {
  typename boost::enable_if::type f();
};

template 
struct Bobble {
  using type = T;
};

struct Frobble {
  using type = char;
};
NonTemplateFunction::type> NTFC;
```
That should trigger the path in `Sema::CheckTypenameType()`, I believe.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54903



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

[PATCH] D51211: [Sema] Emit -Wformat properly for bitfield promotions.

[Bevin Hansson via Phabricator via cfe-commits]

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



Comment at: test/Sema/format-strings-bitfield-promotion.c:1
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -verify %s
+

aaron.ballman wrote:
> aaron.ballman wrote:
> > Running your test through GCC looks like the behavior matches here for C; 
> > can you also add a C++ test that demonstrates the behavior does not change?
> > 
> > https://godbolt.org/z/zRYDMG
> Strangely, the above godbolt link dropped the output windows, here's a 
> different link that shows the behavioral differences between C and C++ mode 
> in GCC: https://godbolt.org/z/R3zRHe
Hmm, I'll have a look at this.


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

https://reviews.llvm.org/D51211



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

r348142 - [AArch64] Add command-line option for SSBS

[Pablo Barrio via cfe-commits]

Author: pabbar01
Date: Mon Dec  3 06:40:37 2018
New Revision: 348142

URL: http://llvm.org/viewvc/llvm-project?rev=348142&view=rev
Log:
[AArch64] Add command-line option for SSBS

Summary:
SSBS (Speculative Store Bypass Safe) is only mandatory from 8.5
onwards but is optional from Armv8.0-A. This patch adds testing for
the ssbs command line option, added to allow enabling the feature
in previous Armv8-A architectures to 8.5.

Reviewers: olista01, samparker, aemerson

Reviewed By: samparker

Subscribers: javed.absar, kristof.beyls, cfe-commits

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

Added:
cfe/trunk/test/Driver/aarch64-ssbs.c

Added: cfe/trunk/test/Driver/aarch64-ssbs.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/aarch64-ssbs.c?rev=348142&view=auto
==
--- cfe/trunk/test/Driver/aarch64-ssbs.c (added)
+++ cfe/trunk/test/Driver/aarch64-ssbs.c Mon Dec  3 06:40:37 2018
@@ -0,0 +1,9 @@
+// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8a+ssbs   %s 
2>&1 | FileCheck %s
+// CHECK: "-target-feature" "+ssbs"
+
+// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8a+nossbs %s 
2>&1 | FileCheck %s --check-prefix=NOSSBS
+// NOSSBS: "-target-feature" "-ssbs"
+
+// RUN: %clang -### -target aarch64-none-none-eabi  %s 
2>&1 | FileCheck %s --check-prefix=ABSENTSSBS
+// ABSENTSSBS-NOT: "-target-feature" "+ssbs"
+// ABSENTSSBS-NOT: "-target-feature" "-ssbs"


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

[PATCH] D54961: [AArch64] Add command-line option for SSBS

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC348142: [AArch64] Add command-line option for SSBS (authored 
by pabbar01, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D54961?vs=175537&id=176388#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D54961

Files:
  test/Driver/aarch64-ssbs.c


Index: test/Driver/aarch64-ssbs.c
===
--- test/Driver/aarch64-ssbs.c
+++ test/Driver/aarch64-ssbs.c
@@ -0,0 +1,9 @@
+// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8a+ssbs   %s 
2>&1 | FileCheck %s
+// CHECK: "-target-feature" "+ssbs"
+
+// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8a+nossbs %s 
2>&1 | FileCheck %s --check-prefix=NOSSBS
+// NOSSBS: "-target-feature" "-ssbs"
+
+// RUN: %clang -### -target aarch64-none-none-eabi  %s 
2>&1 | FileCheck %s --check-prefix=ABSENTSSBS
+// ABSENTSSBS-NOT: "-target-feature" "+ssbs"
+// ABSENTSSBS-NOT: "-target-feature" "-ssbs"


Index: test/Driver/aarch64-ssbs.c
===
--- test/Driver/aarch64-ssbs.c
+++ test/Driver/aarch64-ssbs.c
@@ -0,0 +1,9 @@
+// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8a+ssbs   %s 2>&1 | FileCheck %s
+// CHECK: "-target-feature" "+ssbs"
+
+// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8a+nossbs %s 2>&1 | FileCheck %s --check-prefix=NOSSBS
+// NOSSBS: "-target-feature" "-ssbs"
+
+// RUN: %clang -### -target aarch64-none-none-eabi  %s 2>&1 | FileCheck %s --check-prefix=ABSENTSSBS
+// ABSENTSSBS-NOT: "-target-feature" "+ssbs"
+// ABSENTSSBS-NOT: "-target-feature" "-ssbs"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53812: [Analyzer] Iterator Checker - Forbid decrements past the begin() and increments past the end() of containers

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

baloghadamsoftware updated this revision to Diff 176389.
baloghadamsoftware added a comment.

One error message slightly updated.


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

https://reviews.llvm.org/D53812

Files:
  lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
  test/Analysis/iterator-range.cpp

Index: test/Analysis/iterator-range.cpp
===
--- test/Analysis/iterator-range.cpp
+++ test/Analysis/iterator-range.cpp
@@ -23,34 +23,13 @@
 
 void simple_bad_end(const std::vector &v) {
   auto i = v.end();
-  *i; // expected-warning{{Iterator accessed outside of its range}}
-}
-
-void simple_good_begin(const std::vector &v) {
-  auto i = v.begin();
-  if (i != v.begin()) {
-clang_analyzer_warnIfReached();
-*--i; // no-warning
-  }
-}
-
-void simple_good_begin_negated(const std::vector &v) {
-  auto i = v.begin();
-  if (!(i == v.begin())) {
-clang_analyzer_warnIfReached();
-*--i; // no-warning
-  }
-}
-
-void simple_bad_begin(const std::vector &v) {
-  auto i = v.begin();
-  *--i; // expected-warning{{Iterator accessed outside of its range}}
+  *i; // expected-warning{{Past-the-end iterator dereferenced}}
 }
 
 void copy(const std::vector &v) {
   auto i1 = v.end();
   auto i2 = i1;
-  *i2; // expected-warning{{Iterator accessed outside of its range}}
+  *i2; // expected-warning{{Past-the-end iterator dereferenced}}
 }
 
 void decrease(const std::vector &v) {
@@ -70,7 +49,7 @@
   auto i1 = v.end();
   auto i2 = i1;
   --i1;
-  *i2; // expected-warning{{Iterator accessed outside of its range}}
+  *i2; // expected-warning{{Past-the-end iterator dereferenced}}
 }
 
 void copy_and_increase1(const std::vector &v) {
@@ -86,7 +65,7 @@
   auto i2 = i1;
   ++i1;
   if (i2 == v.end())
-*i2; // expected-warning{{Iterator accessed outside of its range}}
+*i2; // expected-warning{{Past-the-end iterator dereferenced}}
 }
 
 void copy_and_increase3(const std::vector &v) {
@@ -94,7 +73,7 @@
   auto i2 = i1;
   ++i1;
   if (v.end() == i2)
-*i2; // expected-warning{{Iterator accessed outside of its range}}
+*i2; // expected-warning{{Past-the-end iterator dereferenced}}
 }
 
 template 
@@ -116,14 +95,14 @@
 
 void bad_non_std_find(std::vector &V, int e) {
   auto first = nonStdFind(V.begin(), V.end(), e);
-  *first; // expected-warning{{Iterator accessed outside of its range}}
+  *first; // expected-warning{{Past-the-end iterator dereferenced}}
 }
 
 void tricky(std::vector &V, int e) {
   const auto first = V.begin();
   const auto comp1 = (first != V.end()), comp2 = (first == V.end());
   if (comp1)
-*first;
+*first; // no-warning
 }
 
 void loop(std::vector &V, int e) {
@@ -147,7 +126,7 @@
   auto i0 = --L.cend();
   L.push_back(n);
   ++i0;
-  *++i0; // expected-warning{{Iterator accessed outside of its range}}
+  *++i0; // expected-warning{{Past-the-end iterator dereferenced}}
 }
 
 void good_pop_back(std::list &L, int n) {
@@ -159,7 +138,7 @@
 void bad_pop_back(std::list &L, int n) {
   auto i0 = --L.cend(); --i0;
   L.pop_back();
-  *++i0; // expected-warning{{Iterator accessed outside of its range}}
+  *++i0; // expected-warning{{Past-the-end iterator dereferenced}}
 }
 
 void good_push_front(std::list &L, int n) {
@@ -172,7 +151,7 @@
   auto i0 = L.cbegin();
   L.push_front(n);
   --i0;
-  *--i0; // expected-warning{{Iterator accessed outside of its range}}
+  --i0; // expected-warning{{Iterator decremented ahead of its valid range}}
 }
 
 void good_pop_front(std::list &L, int n) {
@@ -184,13 +163,13 @@
 void bad_pop_front(std::list &L, int n) {
   auto i0 = ++L.cbegin();
   L.pop_front();
-  *--i0; // expected-warning{{Iterator accessed outside of its range}}
+  --i0; // expected-warning{{Iterator decremented ahead of its valid range}}
 }
 
 void bad_move(std::list &L1, std::list &L2) {
   auto i0 = --L2.cend();
   L1 = std::move(L2);
-  *++i0; // expected-warning{{Iterator accessed outside of its range}}
+  *++i0; // expected-warning{{Past-the-end iterator dereferenced}}
 }
 
 void bad_move_push_back(std::list &L1, std::list &L2, int n) {
@@ -198,5 +177,25 @@
   L2.push_back(n);
   L1 = std::move(L2);
   ++i0;
-  *++i0; // expected-warning{{Iterator accessed outside of its range}}
+  *++i0; // expected-warning{{Past-the-end iterator dereferenced}}
+}
+
+void good_incr_begin(const std::list &L) {
+  auto i0 = L.begin();
+  ++i0; // no-warning
+}
+
+void bad_decr_begin(const std::list &L) {
+  auto i0 = L.begin();
+  --i0;  // expected-warning{{Iterator decremented ahead of its valid range}}
+}
+
+void good_decr_end(const std::list &L) {
+  auto i0 = L.end();
+  --i0; // no-warning
+}
+
+void bad_incr_end(const std::list &L) {
+  auto i0 = L.end();
+  ++i0;  // expected-warning{{Iterator incremented behind the past-the-end iterator}}
 }
Index: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/Iterato

[PATCH] D54630: Move detection of libc++ include dirs to Driver on MacOS

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In D54630#1315168 , @arphaman wrote:

> So far everything looks fine, but I have to rerun a couple of things due to 
> infrastructural issues. I should have the final results next Monday.


Thanks for the update. Will be waiting for the final results.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54630



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

r348145 - [AST][Sema] Remove CallExpr::setNumArgs

[Bruno Ricci via cfe-commits]

Author: brunoricci
Date: Mon Dec  3 06:54:03 2018
New Revision: 348145

URL: http://llvm.org/viewvc/llvm-project?rev=348145&view=rev
Log:
[AST][Sema] Remove CallExpr::setNumArgs

CallExpr::setNumArgs is the only thing that prevents storing the arguments
in a trailing array. There is only 3 places in Sema where setNumArgs is called.
D54900 dealt with one of them.

This patch remove the other two calls to setNumArgs in ConvertArgumentsForCall.
To do this we do the following changes:

1.) Replace the first call to setNumArgs by an assertion since we are moving the
responsability to allocate enough space for the arguments from
Sema::ConvertArgumentsForCall to its callers
(which are Sema::BuildCallToMemberFunction, and Sema::BuildResolvedCallExpr).

2.) Add a new member function CallExpr::shrinkNumArgs, which can only be used
to drop arguments and then replace the second call to setNumArgs by
shrinkNumArgs.

3.) Add a new defaulted parameter MinNumArgs to CallExpr and its derived
classes which specifies a minimum number of argument slots to allocate.
The actual number of arguments slots allocated will be
max(number of args, MinNumArgs) with the extra args nulled. Note that
after the creation of the call expression all of the arguments will be
non-null. It is just during the creation of the call expression that some of
the last arguments can be temporarily null, until filled by default arguments.

4.) Update Sema::BuildCallToMemberFunction by passing the number of parameters
in the function prototype to the constructor of CXXMemberCallExpr. Here the
change is pretty straightforward.

5.) Update Sema::BuildResolvedCallExpr. Here the change is more complicated
since the type-checking for the function type was done after the creation of
the call expression. We need to move this before the creation of the call
expression, and then pass the number of parameters in the function prototype
(if any) to the constructor of the call expression.

6.) Update the deserialization of CallExpr and its derived classes.

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

Reviewed By: aaron.ballman


Modified:
cfe/trunk/include/clang/AST/Expr.h
cfe/trunk/include/clang/AST/ExprCXX.h
cfe/trunk/lib/AST/Expr.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Sema/SemaOverload.cpp
cfe/trunk/lib/Serialization/ASTReaderStmt.cpp

Modified: cfe/trunk/include/clang/AST/Expr.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=348145&r1=348144&r2=348145&view=diff
==
--- cfe/trunk/include/clang/AST/Expr.h (original)
+++ cfe/trunk/include/clang/AST/Expr.h Mon Dec  3 06:54:03 2018
@@ -2416,11 +2416,12 @@ protected:
   // These versions of the constructor are for derived classes.
   CallExpr(const ASTContext &C, StmtClass SC, Expr *fn,
ArrayRef preargs, ArrayRef args, QualType t,
-   ExprValueKind VK, SourceLocation rparenloc);
+   ExprValueKind VK, SourceLocation rparenloc, unsigned MinNumArgs = 
0);
   CallExpr(const ASTContext &C, StmtClass SC, Expr *fn, ArrayRef args,
-   QualType t, ExprValueKind VK, SourceLocation rparenloc);
+   QualType t, ExprValueKind VK, SourceLocation rparenloc,
+   unsigned MinNumArgs = 0);
   CallExpr(const ASTContext &C, StmtClass SC, unsigned NumPreArgs,
-   EmptyShell Empty);
+   unsigned NumArgs, EmptyShell Empty);
 
   Stmt *getPreArg(unsigned i) {
 assert(i < getNumPreArgs() && "Prearg access out of range!");
@@ -2438,11 +2439,14 @@ protected:
   unsigned getNumPreArgs() const { return CallExprBits.NumPreArgs; }
 
 public:
-  CallExpr(const ASTContext& C, Expr *fn, ArrayRef args, QualType t,
-   ExprValueKind VK, SourceLocation rparenloc);
+  /// Build a call expression. MinNumArgs specifies the minimum number of
+  /// arguments. The actual number of arguments will be the greater of
+  /// args.size() and MinNumArgs.
+  CallExpr(const ASTContext &C, Expr *fn, ArrayRef args, QualType t,
+   ExprValueKind VK, SourceLocation rparenloc, unsigned MinNumArgs = 
0);
 
   /// Build an empty call expression.
-  CallExpr(const ASTContext &C, StmtClass SC, EmptyShell Empty);
+  CallExpr(const ASTContext &C, unsigned NumArgs, EmptyShell Empty);
 
   const Expr *getCallee() const { return cast(SubExprs[FN]); }
   Expr *getCallee() { return cast(SubExprs[FN]); }
@@ -2488,10 +2492,18 @@ public:
 SubExprs[Arg+getNumPreArgs()+PREARGS_START] = ArgExpr;
   }
 
-  /// setNumArgs - This changes the number of arguments present in this call.
-  /// Any orphaned expressions are deleted by this, and any new operands are 
set
-  /// to null.
-  void setNumArgs(const ASTContext& C, unsigned NumArgs);
+  /// Reduce the number of arguments in this call expression. This is used for
+  /// example during error recovery to drop extra arguments. There is no way
+  /// to perform the opposite because: 1.) We don't track h

[PATCH] D51211: [Sema] Emit -Wformat properly for bitfield promotions.

[Bevin Hansson via Phabricator via cfe-commits]

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



Comment at: test/Sema/format-strings-bitfield-promotion.c:1
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -verify %s
+

ebevhan wrote:
> aaron.ballman wrote:
> > aaron.ballman wrote:
> > > Running your test through GCC looks like the behavior matches here for C; 
> > > can you also add a C++ test that demonstrates the behavior does not 
> > > change?
> > > 
> > > https://godbolt.org/z/zRYDMG
> > Strangely, the above godbolt link dropped the output windows, here's a 
> > different link that shows the behavioral differences between C and C++ mode 
> > in GCC: https://godbolt.org/z/R3zRHe
> Hmm, I'll have a look at this.
That gcc godbolt is a bit odd. The type of the bitfield expression in the C++ 
example is `long` and not `int`, but in Clang, it's clearly being converted. If 
I change the example a bit, we get this warning:
```
:11:12: warning: format '%d' expects argument of type 'int', but 
argument 2 has type 'long int' [-Wformat=]
   11 |   printf("%d", bf.a); // expected-warning {{format specifies type 
'long' but the argument has type 'int'}}
  |   ~^   
  ||  |
  |intlong int
```
But in Clang, we get a cast to `int`:
```
| `-ImplicitCastExpr 0xd190748  'int' 
|   `-ImplicitCastExpr 0xd190730  'long' 
| `-MemberExpr 0xd190618  'long' lvalue bitfield .a 
0xd18f790
|   `-DeclRefExpr 0xd1905f8  'struct bitfields':'bitfields' 
lvalue Var 0xd18fa18 'bf' 'struct bitfields':'bitfields'
```

So gcc and Clang are doing things differently here.

The code in `isPromotableBitField` says:
```
  // FIXME: C does not permit promotion of a 'long : 3' bitfield to int.
  //We perform that promotion here to match GCC and C++.
```
but clearly gcc isn't doing this in the C++ case. The comments also mention 
some things about gcc bugs that Clang does not follow, but that's in reference 
to a C DR.


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

https://reviews.llvm.org/D51211



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

[PATCH] D54902: [AST][Sema] Remove CallExpr::setNumArgs

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL348145: [AST][Sema] Remove CallExpr::setNumArgs (authored by 
brunoricci, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D54902?vs=175434&id=176394#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D54902

Files:
  cfe/trunk/include/clang/AST/Expr.h
  cfe/trunk/include/clang/AST/ExprCXX.h
  cfe/trunk/lib/AST/Expr.cpp
  cfe/trunk/lib/Sema/SemaExpr.cpp
  cfe/trunk/lib/Sema/SemaOverload.cpp
  cfe/trunk/lib/Serialization/ASTReaderStmt.cpp

Index: cfe/trunk/include/clang/AST/Expr.h
===
--- cfe/trunk/include/clang/AST/Expr.h
+++ cfe/trunk/include/clang/AST/Expr.h
@@ -2416,11 +2416,12 @@
   // These versions of the constructor are for derived classes.
   CallExpr(const ASTContext &C, StmtClass SC, Expr *fn,
ArrayRef preargs, ArrayRef args, QualType t,
-   ExprValueKind VK, SourceLocation rparenloc);
+   ExprValueKind VK, SourceLocation rparenloc, unsigned MinNumArgs = 0);
   CallExpr(const ASTContext &C, StmtClass SC, Expr *fn, ArrayRef args,
-   QualType t, ExprValueKind VK, SourceLocation rparenloc);
+   QualType t, ExprValueKind VK, SourceLocation rparenloc,
+   unsigned MinNumArgs = 0);
   CallExpr(const ASTContext &C, StmtClass SC, unsigned NumPreArgs,
-   EmptyShell Empty);
+   unsigned NumArgs, EmptyShell Empty);
 
   Stmt *getPreArg(unsigned i) {
 assert(i < getNumPreArgs() && "Prearg access out of range!");
@@ -2438,11 +2439,14 @@
   unsigned getNumPreArgs() const { return CallExprBits.NumPreArgs; }
 
 public:
-  CallExpr(const ASTContext& C, Expr *fn, ArrayRef args, QualType t,
-   ExprValueKind VK, SourceLocation rparenloc);
+  /// Build a call expression. MinNumArgs specifies the minimum number of
+  /// arguments. The actual number of arguments will be the greater of
+  /// args.size() and MinNumArgs.
+  CallExpr(const ASTContext &C, Expr *fn, ArrayRef args, QualType t,
+   ExprValueKind VK, SourceLocation rparenloc, unsigned MinNumArgs = 0);
 
   /// Build an empty call expression.
-  CallExpr(const ASTContext &C, StmtClass SC, EmptyShell Empty);
+  CallExpr(const ASTContext &C, unsigned NumArgs, EmptyShell Empty);
 
   const Expr *getCallee() const { return cast(SubExprs[FN]); }
   Expr *getCallee() { return cast(SubExprs[FN]); }
@@ -2488,10 +2492,18 @@
 SubExprs[Arg+getNumPreArgs()+PREARGS_START] = ArgExpr;
   }
 
-  /// setNumArgs - This changes the number of arguments present in this call.
-  /// Any orphaned expressions are deleted by this, and any new operands are set
-  /// to null.
-  void setNumArgs(const ASTContext& C, unsigned NumArgs);
+  /// Reduce the number of arguments in this call expression. This is used for
+  /// example during error recovery to drop extra arguments. There is no way
+  /// to perform the opposite because: 1.) We don't track how much storage
+  /// we have for the argument array 2.) This would potentially require growing
+  /// the argument array, something we cannot support since the arguments will
+  /// be stored in a trailing array in the future.
+  /// (TODO: update this comment when this is done).
+  void shrinkNumArgs(unsigned NewNumArgs) {
+assert((NewNumArgs <= NumArgs) &&
+   "shrinkNumArgs cannot increase the number of arguments!");
+NumArgs = NewNumArgs;
+  }
 
   typedef ExprIterator arg_iterator;
   typedef ConstExprIterator const_arg_iterator;
Index: cfe/trunk/include/clang/AST/ExprCXX.h
===
--- cfe/trunk/include/clang/AST/ExprCXX.h
+++ cfe/trunk/include/clang/AST/ExprCXX.h
@@ -98,8 +98,10 @@
 Range = getSourceRangeImpl();
   }
 
-  explicit CXXOperatorCallExpr(ASTContext& C, EmptyShell Empty)
-  : CallExpr(C, CXXOperatorCallExprClass, Empty) {}
+  explicit CXXOperatorCallExpr(ASTContext &C, unsigned NumArgs,
+   EmptyShell Empty)
+  : CallExpr(C, CXXOperatorCallExprClass, /*NumPreArgs=*/0, NumArgs,
+ Empty) {}
 
   /// Returns the kind of overloaded operator that this
   /// expression refers to.
@@ -163,12 +165,13 @@
 /// the object argument).
 class CXXMemberCallExpr : public CallExpr {
 public:
-  CXXMemberCallExpr(ASTContext &C, Expr *fn, ArrayRef args,
-QualType t, ExprValueKind VK, SourceLocation RP)
-  : CallExpr(C, CXXMemberCallExprClass, fn, args, t, VK, RP) {}
+  CXXMemberCallExpr(ASTContext &C, Expr *fn, ArrayRef args, QualType t,
+ExprValueKind VK, SourceLocation RP,
+unsigned MinNumArgs = 0)
+  : CallExpr(C, CXXMemberCallExprClass, fn, args, t, VK, RP, MinNumArgs) {}
 
-  CXXMemberCallExpr(ASTContext &C, EmptyShell Empty)
-  : CallExpr(C, CXXMemberCallExprCl

[PATCH] D54903: [Sema] Improve static_assert diagnostics.

[Clement Courbet via Phabricator via cfe-commits]

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



Comment at: lib/Sema/SemaTemplate.cpp:3055
 
+// Print a diagnostic for the failing static_assert expression. Defaults to
+// pretty-printing the expression.

aaron.ballman wrote:
> courbet wrote:
> > aaron.ballman wrote:
> > > Comment is a bit out of date as this is no longer specific to 
> > > `static_assert`.
> > > 
> > > It looks like this may also change the behavior of enable_if diagnostic 
> > > reporting. Do you need to update any of those tests from this change? If 
> > > not, can you devise some tests for that case as well to show that this 
> > > isn't just a static_assert behavior? (Do a search for 
> > > `findFailedBooleanCondition` to find where the behavior has changed.)
> > Nope, unfortunately the code that calls this is actually untested...
> > I could not find a test that actually enters the:
> > ```
> > if (TypeAliasTemplateDecl *AliasTemplate =
> >   dyn_cast(Template))
> > ```
> > in `Sema::CheckTemplateIdType`.
> > 
> > I stopped at the following insanity:
> > 
> > ```
> > // RUN: %clang_cc1 -std=c++11 -verify %s
> > 
> > template 
> > struct is_same {
> >   enum { value = 0 };
> > };
> > 
> > template  struct is_same {
> >   enum { value = 1 };
> > };
> > 
> > struct Dispatch {
> >   template  using SameAs = is_same;
> > };
> > 
> > template 
> > struct S {
> >   template  using same = typename DispatchT::template SameAs;
> > 
> >   template 
> >   static void foo() __attribute__((enable_if(same::value, "")));
> > 
> > };
> > 
> > void runFoo() {
> >   // S::foo();
> >   S::foo();
> > 
> > }
> > ```
> > 
> > This one exercises the code up to `if (CanonType.isNull()) {`, but I'm not 
> > sure how to get a null type here.
> > 
> How about this:
> ```
> namespace boost {
>   template struct enable_if {};
>   template struct enable_if { typedef T type; };
> }
> 
> template struct NonTemplateFunction {
>   typename boost::enable_if::type f();
> };
> 
> template 
> struct Bobble {
>   using type = T;
> };
> 
> struct Frobble {
>   using type = char;
> };
> NonTemplateFunction::type> NTFC;
> ```
> That should trigger the path in `Sema::CheckTypenameType()`, I believe.
This triggers:
```
else if (ClassTemplateDecl *ClassTemplate
   = dyn_cast(Template))
```

Modifying it to:
```
namespace boost {
  template struct enable_if {};
  template struct enable_if { typedef T type; };
}

template struct NonTemplateFunction {
  template  using toto = typename boost::enable_if::type;
  toto f();
};

template 
struct Bobble {
  using type = T;
};

struct Frobble {
  using type = char;
};
NonTemplateFunction::type> NTFC;
```

triggers the same path as my previous example, but not the `IsNull()` test.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54903



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

[PATCH] D55062: [clangd] Partition include graph on auto-index.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Everything looks good, just a single important request about testing the 
included files do not have any edges in the resulting graph




Comment at: clangd/index/Background.cpp:79
+
+  // Since the strings in direct includes are references to the keys of the
+  // fullgraph, we need to create entries in our new sub-graph for those

NIT: the comment could probably be simplified to something like `URIs inside 
nodes must point into the keys of the same IncludeGraph`



Comment at: clangd/index/Background.cpp:386
 
-  log("Indexed {0} ({1} symbols, {2} refs)", Inputs.CompileCommand.Filename,
-  Symbols.size(), Refs.numRefs());
-  SPAN_ATTACH(Tracer, "symbols", int(Symbols.size()));
-  SPAN_ATTACH(Tracer, "refs", int(Refs.numRefs()));
-  IndexFileIn Index;
-  Index.Symbols = std::move(Symbols);
-  Index.Refs = std::move(Refs);
+  log("Indexed {0} ({1} symbols, {2} refs, {3} sources)",
+  Inputs.CompileCommand.Filename, Index.Symbols->size(),

"sources" might be a bit confusing. I'd probably use "files" instead, but that 
doesn't look ideal too.



Comment at: unittests/clangd/BackgroundIndexTests.cpp:197
+  auto ShardSource = MSS.loadShard(testPath("root/A.cc"));
+  EXPECT_TRUE(ShardSource->Sources);
+  EXPECT_EQ(ShardSource->Sources->size(), 2U); // A.cc, A.h

Maybe check the full shape of the graph? E.g. also add a check that `A.h` does 
not have any includes and its digest is populated.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55062



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

[PATCH] D54903: [Sema] Improve static_assert diagnostics.

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!




Comment at: lib/Sema/SemaTemplate.cpp:3055
 
+// Print a diagnostic for the failing static_assert expression. Defaults to
+// pretty-printing the expression.

courbet wrote:
> aaron.ballman wrote:
> > courbet wrote:
> > > aaron.ballman wrote:
> > > > Comment is a bit out of date as this is no longer specific to 
> > > > `static_assert`.
> > > > 
> > > > It looks like this may also change the behavior of enable_if diagnostic 
> > > > reporting. Do you need to update any of those tests from this change? 
> > > > If not, can you devise some tests for that case as well to show that 
> > > > this isn't just a static_assert behavior? (Do a search for 
> > > > `findFailedBooleanCondition` to find where the behavior has changed.)
> > > Nope, unfortunately the code that calls this is actually untested...
> > > I could not find a test that actually enters the:
> > > ```
> > > if (TypeAliasTemplateDecl *AliasTemplate =
> > >   dyn_cast(Template))
> > > ```
> > > in `Sema::CheckTemplateIdType`.
> > > 
> > > I stopped at the following insanity:
> > > 
> > > ```
> > > // RUN: %clang_cc1 -std=c++11 -verify %s
> > > 
> > > template 
> > > struct is_same {
> > >   enum { value = 0 };
> > > };
> > > 
> > > template  struct is_same {
> > >   enum { value = 1 };
> > > };
> > > 
> > > struct Dispatch {
> > >   template  using SameAs = is_same;
> > > };
> > > 
> > > template 
> > > struct S {
> > >   template  using same = typename DispatchT::template 
> > > SameAs;
> > > 
> > >   template 
> > >   static void foo() __attribute__((enable_if(same::value, "")));
> > > 
> > > };
> > > 
> > > void runFoo() {
> > >   // S::foo();
> > >   S::foo();
> > > 
> > > }
> > > ```
> > > 
> > > This one exercises the code up to `if (CanonType.isNull()) {`, but I'm 
> > > not sure how to get a null type here.
> > > 
> > How about this:
> > ```
> > namespace boost {
> >   template struct enable_if {};
> >   template struct enable_if { typedef T type; };
> > }
> > 
> > template struct NonTemplateFunction {
> >   typename boost::enable_if::type f();
> > };
> > 
> > template 
> > struct Bobble {
> >   using type = T;
> > };
> > 
> > struct Frobble {
> >   using type = char;
> > };
> > NonTemplateFunction::type> NTFC;
> > ```
> > That should trigger the path in `Sema::CheckTypenameType()`, I believe.
> This triggers:
> ```
> else if (ClassTemplateDecl *ClassTemplate
>= dyn_cast(Template))
> ```
> 
> Modifying it to:
> ```
> namespace boost {
>   template struct enable_if {};
>   template struct enable_if { typedef T type; };
> }
> 
> template struct NonTemplateFunction {
>   template  using toto = typename boost::enable_if 4, int>::type;
>   toto f();
> };
> 
> template 
> struct Bobble {
>   using type = T;
> };
> 
> struct Frobble {
>   using type = char;
> };
> NonTemplateFunction::type> NTFC;
> ```
> 
> triggers the same path as my previous example, but not the `IsNull()` test.
Aw crud, well, we tried.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54903



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

[PATCH] D53866: [Preamble] Fix preamble for circular #includes

[Nikolai Kosjar via Phabricator via cfe-commits]

nik added a comment.

Ping.


Repository:
  rC Clang

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

https://reviews.llvm.org/D53866



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

[PATCH] D54995: [MemoryBuffer] Add the setter to be able to force disabled mmap

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In D54995#1316476 , @yvvan wrote:

> I don't think removing the flag is a good idea since I can easily assume 
> cases when user wants mmap and is ready to encounter locks. In our case it 
> can be an IDE option which behavior to choose.


Would that option be useful if changing it forces the files user is editing in 
the IDE to be indefinitely locked and stops them from saving the files?
Anyway, even if you feel this behavior should be configurable, that's totally 
fine - I was referring to removing the flag from the vfs::FileSystem 
**interface**, possibly replacing it with a filesystem implementation that 
would handle the same thing at a lower level.
This flag is not useful to any of the filesystem implementation, except 
RealFileSystem, which actually has to deal with opening OS files.

> The ideal solution in my opinion would be to have isSystem(filename) in 
> FileSystem class. And is based on system directories with which ASTUnit feeds 
> the FileSystem for example.

I would disagree that `isSystem()` is a property of the filesystem. The 
filesystem abstraction is responsible for providing access to the files and 
their contents, why should classification of the files be filesystem-dependent? 
It's the wrong layer for the `isSystem()` check.
E.g. imagine implementing a filesystem that gets files from the network. The 
concerns filesystem is responsible for clearly include doing network IO. But 
why should the system/user classification of the files be different for it? 
What policy decisions does it have to make there? Clearly some other should do 
this classification and it's clearly independent of the filesystem being used.
OTOH, passing whether we want to open the files as memory-mapped does not sound 
completely crazy. But it would still be nice to avoid having it in the 
interface, since we **only** need it to workaround limitations specific to 
Windows + interactive editors, which is only a small subset of LLVM-based tools 
uses.


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

https://reviews.llvm.org/D54995



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

[clang-tools-extra] r348147 - [clangd] Avoid memory-mapping files on Windows

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Mon Dec  3 07:21:49 2018
New Revision: 348147

URL: http://llvm.org/viewvc/llvm-project?rev=348147&view=rev
Log:
[clangd] Avoid memory-mapping files on Windows

Summary:
Memory-mapping files on Windows leads to them being locked and prevents
editors from saving changes to those files on disk. This is fine for the
compiler, but not acceptable for an interactive tool like clangd.
Therefore, we choose to avoid using memory-mapped files on Windows.

Reviewers: hokein, kadircet

Reviewed By: kadircet

Subscribers: yvvan, zturner, nik, malaperle, mgorny, ioeric, MaskRay, jkorous, 
arphaman, cfe-commits

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

Added:
clang-tools-extra/trunk/clangd/FSProvider.cpp
Modified:
clang-tools-extra/trunk/clangd/CMakeLists.txt
clang-tools-extra/trunk/clangd/FSProvider.h

Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CMakeLists.txt?rev=348147&r1=348146&r2=348147&view=diff
==
--- clang-tools-extra/trunk/clangd/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clangd/CMakeLists.txt Mon Dec  3 07:21:49 2018
@@ -23,6 +23,7 @@ add_clang_library(clangDaemon
   FindSymbols.cpp
   FileDistance.cpp
   FS.cpp
+  FSProvider.cpp
   FuzzyMatch.cpp
   GlobalCompilationDatabase.cpp
   Headers.cpp

Added: clang-tools-extra/trunk/clangd/FSProvider.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/FSProvider.cpp?rev=348147&view=auto
==
--- clang-tools-extra/trunk/clangd/FSProvider.cpp (added)
+++ clang-tools-extra/trunk/clangd/FSProvider.cpp Mon Dec  3 07:21:49 2018
@@ -0,0 +1,85 @@
+//===--- FSProvider.cpp - VFS provider for ClangdServer 
---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "FSProvider.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/VirtualFileSystem.h"
+#include 
+
+using namespace llvm;
+
+namespace clang {
+namespace clangd {
+
+namespace {
+/// Always opens files in the underlying filesystem as "volatile", meaning they
+/// won't be memory-mapped. This avoid locking the files on Windows.
+class VolatileFileSystem : public llvm::vfs::ProxyFileSystem {
+public:
+  explicit VolatileFileSystem(llvm::IntrusiveRefCntPtr FS)
+  : ProxyFileSystem(std::move(FS)) {}
+
+  llvm::ErrorOr>
+  openFileForRead(const Twine &InPath) override {
+SmallString<128> Path;
+InPath.toVector(Path);
+
+auto File = getUnderlyingFS().openFileForRead(Path);
+if (!File)
+  return File;
+// Try to guess preamble files, they can be memory-mapped even on Windows 
as
+// clangd has exclusive access to those.
+StringRef FileName = llvm::sys::path::filename(Path);
+if (FileName.startswith("preamble-") && FileName.endswith(".pch"))
+  return File;
+return std::unique_ptr(
+new VolatileFile(std::move(*File)));
+  }
+
+private:
+  class VolatileFile : public vfs::File {
+  public:
+VolatileFile(std::unique_ptr Wrapped)
+: Wrapped(std::move(Wrapped)) {
+  assert(this->Wrapped);
+}
+
+virtual llvm::ErrorOr>
+getBuffer(const Twine &Name, int64_t FileSize, bool RequiresNullTerminator,
+  bool /*IsVolatile*/) override {
+  return Wrapped->getBuffer(Name, FileSize, RequiresNullTerminator,
+/*IsVolatile=*/true);
+}
+
+llvm::ErrorOr status() override { return Wrapped->status(); }
+llvm::ErrorOr getName() override { return Wrapped->getName(); 
}
+std::error_code close() override { return Wrapped->close(); }
+
+  private:
+std::unique_ptr Wrapped;
+  };
+};
+} // namespace
+
+llvm::IntrusiveRefCntPtr
+clang::clangd::RealFileSystemProvider::getFileSystem() const {
+// Avoid using memory-mapped files on Windows, they cause file locking issues.
+// FIXME: Try to use a similar approach in Sema instead of relying on
+//propagation of the 'isVolatile' flag through all layers.
+#ifdef _WIN32
+  return new VolatileFSProvider(vfs::getRealFileSystem());
+#else
+  return vfs::getRealFileSystem();
+#endif
+}
+} // namespace clangd
+} // namespace clang

Modified: clang-tools-extra/trunk/clangd/FSProvider.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/FSProvider.h?rev=348147&r1=348146&r2=348147&view=diff
==
--- clang-tools-extra/trunk/clangd/FSProvider.h (original)
+++ clang-tools-extra/trunk/clangd/FSProvider.h Mon Dec  

[PATCH] D41005: Reuse preamble even if an unsaved file does not exist

[Nikolai Kosjar via Phabricator via cfe-commits]

nik added a comment.

Ping.


Repository:
  rC Clang

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

https://reviews.llvm.org/D41005



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

[PATCH] D55139: [clangd] Avoid memory-mapping files on Windows

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE348147: [clangd] Avoid memory-mapping files on Windows 
(authored by ibiryukov, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D55139?vs=176382&id=176401#toc

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55139

Files:
  clangd/CMakeLists.txt
  clangd/FSProvider.cpp
  clangd/FSProvider.h

Index: clangd/FSProvider.cpp
===
--- clangd/FSProvider.cpp
+++ clangd/FSProvider.cpp
@@ -0,0 +1,85 @@
+//===--- FSProvider.cpp - VFS provider for ClangdServer ---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "FSProvider.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/VirtualFileSystem.h"
+#include 
+
+using namespace llvm;
+
+namespace clang {
+namespace clangd {
+
+namespace {
+/// Always opens files in the underlying filesystem as "volatile", meaning they
+/// won't be memory-mapped. This avoid locking the files on Windows.
+class VolatileFileSystem : public llvm::vfs::ProxyFileSystem {
+public:
+  explicit VolatileFileSystem(llvm::IntrusiveRefCntPtr FS)
+  : ProxyFileSystem(std::move(FS)) {}
+
+  llvm::ErrorOr>
+  openFileForRead(const Twine &InPath) override {
+SmallString<128> Path;
+InPath.toVector(Path);
+
+auto File = getUnderlyingFS().openFileForRead(Path);
+if (!File)
+  return File;
+// Try to guess preamble files, they can be memory-mapped even on Windows as
+// clangd has exclusive access to those.
+StringRef FileName = llvm::sys::path::filename(Path);
+if (FileName.startswith("preamble-") && FileName.endswith(".pch"))
+  return File;
+return std::unique_ptr(
+new VolatileFile(std::move(*File)));
+  }
+
+private:
+  class VolatileFile : public vfs::File {
+  public:
+VolatileFile(std::unique_ptr Wrapped)
+: Wrapped(std::move(Wrapped)) {
+  assert(this->Wrapped);
+}
+
+virtual llvm::ErrorOr>
+getBuffer(const Twine &Name, int64_t FileSize, bool RequiresNullTerminator,
+  bool /*IsVolatile*/) override {
+  return Wrapped->getBuffer(Name, FileSize, RequiresNullTerminator,
+/*IsVolatile=*/true);
+}
+
+llvm::ErrorOr status() override { return Wrapped->status(); }
+llvm::ErrorOr getName() override { return Wrapped->getName(); }
+std::error_code close() override { return Wrapped->close(); }
+
+  private:
+std::unique_ptr Wrapped;
+  };
+};
+} // namespace
+
+llvm::IntrusiveRefCntPtr
+clang::clangd::RealFileSystemProvider::getFileSystem() const {
+// Avoid using memory-mapped files on Windows, they cause file locking issues.
+// FIXME: Try to use a similar approach in Sema instead of relying on
+//propagation of the 'isVolatile' flag through all layers.
+#ifdef _WIN32
+  return new VolatileFSProvider(vfs::getRealFileSystem());
+#else
+  return vfs::getRealFileSystem();
+#endif
+}
+} // namespace clangd
+} // namespace clang
Index: clangd/CMakeLists.txt
===
--- clangd/CMakeLists.txt
+++ clangd/CMakeLists.txt
@@ -23,6 +23,7 @@
   FindSymbols.cpp
   FileDistance.cpp
   FS.cpp
+  FSProvider.cpp
   FuzzyMatch.cpp
   GlobalCompilationDatabase.cpp
   Headers.cpp
Index: clangd/FSProvider.h
===
--- clangd/FSProvider.h
+++ clangd/FSProvider.h
@@ -33,9 +33,7 @@
 public:
   // FIXME: returns the single real FS instance, which is not threadsafe.
   llvm::IntrusiveRefCntPtr
-  getFileSystem() const override {
-return llvm::vfs::getRealFileSystem();
-  }
+  getFileSystem() const override;
 };
 
 } // namespace clangd
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52984: [analyzer] Checker reviewer's checklist

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

xazax.hun updated this revision to Diff 176403.
xazax.hun marked 4 inline comments as done.
xazax.hun added a comment.

- Addressed further comments.


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

https://reviews.llvm.org/D52984

Files:
  www/analyzer/checker_dev_manual.html

Index: www/analyzer/checker_dev_manual.html
===
--- www/analyzer/checker_dev_manual.html
+++ www/analyzer/checker_dev_manual.html
@@ -675,6 +675,111 @@
 (gdb) p C.getPredecessor()->getCodeDecl().getBody()->dump()
 
 
+Making Your Checker Better
+
+User facing documentation is important for adoption! Make sure the checker list is updated
+at the homepage of the analyzer. Also ensure the description is clear to
+non-analyzer-developers in Checkers.td.
+Warning and note messages should be clear and easy to understand, even if a bit long.
+
+  Messages should start with a capital letter (unlike Clang warnings!) and should not
+  end with ..
+  Articles are usually omitted, eg. Dereference of a null pointer ->
+  Dereference of null pointer.
+  Introduce BugReporterVisitors to emit additional notes that explain the warning
+  to the user better. There are some existing visitors that might be useful for your check,
+  e.g. trackNullOrUndefValue. For example, SimpleStreamChecker should highlight
+  the event of opening the file when reporting a file descriptor leak.
+
+If the check tracks anything in the program state, it needs to implement the
+checkDeadSymbolscallback to clean the state up.
+The check should conservatively assume that the program is correct when a tracked symbol
+is passed to a function that is unknown to the analyzer.
+checkPointerEscape callback could help you handle that case.
+Use safe and convenient APIs!
+
+  Always use CheckerContext::generateErrorNode and
+CheckerContext::generateNonFatalErrorNode for emitting bug reports.
+Most importantly, never emit report against CheckerContext::getPredecessor.
+  Prefer checkPreCall and checkPostCall to
+checkPreStmt and checkPostStmt.
+  Use CallDescription to detect hardcoded API calls in the program.
+  Simplify C.getState()->getSVal(E, C.getLocationContext()) to C.getSVal(E).
+
+Common sources of crashes:
+
+  CallEvent::getOriginExpr is nullable - for example, it returns null for an
+automatic destructor of a variable. The same applies to some values generated while the
+call was modeled, eg. SymbolConjured::getStmt is nullable.
+  CallEvent::getDecl is nullable - for example, it returns null for a
+  call of symbolic function pointer.
+  addTransition, generateSink, generateNonFatalErrorNode,
+generateErrorNode are nullable because you can transition to a node that you have already visited.
+  Methods of CallExpr/FunctionDecl/CallEvent that
+return arguments crash when the argument is out-of-bounds. If you checked the function name,
+it doesn't mean that the function has the expected number of arguments!
+Which is why you should use CallDescription.
+  Nullability of different entities within different kinds of symbols and regions is usually
+  documented via assertions in their constructors.
+  NamedDecl::getName will fail if the name of the declaration is not a single token,
+e.g. for destructors. You could use NamedDecl::getNameAsString for those cases.
+Note that this method is much slower and should be used sparringly, e.g. only when generating reports
+but not during analysis.
+  Is -analyzer-checker=core included in all test RUN: lines? It was never supported
+to run the analyzer with the core checks disabled. It might cause unexpected behavior and
+crashes. You should do all your testing with the core checks enabled.
+
+
+Patterns that you should most likely avoid even if they're not technically wrong:
+
+  BugReporterVisitor should most likely not match the AST of the current program point
+  to decide when to emit a note. It is much easier to determine that by observing changes in
+  the program state.
+  In State->getSVal(Region), if Region is not known to be a TypedValueRegion
+  and the optional type argument is not specified, the checker may accidentally try to dereference a
+  void pointer.
+  Checker logic should not depend on whether a certain value is a Loc or NonLoc.
+It should be immediately obvious whether the SVal is a Loc or a
+NonLoc depending on the AST that is being checked. Checking whether a value
+is Loc or Unknown/Undefined or whether the value is
+NonLoc or Unknown/Undefined is totally fine.
+  New symbols should not be constructed in the checker via direct calls to SymbolManager,
+unless they are of SymbolMetadata class tagged by the checker,
+or they represent newly created values such as the return value in evalCall.
+For modeling arithmetic/bitwise/comparison operations, SValBuilder s

[PATCH] D55066: [ASan] Minor documentation fix: remove static linking limitation.

[Max Moroz via Phabricator via cfe-commits]

Dor1s added a comment.

In D55066#1315365 , @eugenis wrote:

> Sorry for the delay.
>  This is wrong, static linking is NOT supported.
>  You could be confusing it with static linking of asan runtime library to an 
> executable - that is and has always been the default.


Thanks, @eugenis! Should I change the message to something like "ASan runtime 
cannot be built with static linking."? Does it sound correct and perhaps less 
confusing?


Repository:
  rC Clang

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

https://reviews.llvm.org/D55066



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

[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 again for your time.  As far as I can tell, it's ready for 
another round of review!


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

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] D54881: Prevent Clang-Format from editing leading whitespace on lines outside of the format range

[Russell McClellan via Phabricator via cfe-commits]

russellmcc added a comment.

Bump!  Thanks for your consideration.


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

https://reviews.llvm.org/D54881



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

Re: r348123 - [clang] Do not read from 'test/SemaCXX/Inputs' inside 'test/AST'

[Ilya Biryukov via cfe-commits]

No worries, this was easy to fix.
It's impossible to catch those in advance, we miss things like that all the
time.

On Mon, Dec 3, 2018 at 1:19 PM Aaron Ballman  wrote:

> On Mon, Dec 3, 2018 at 6:29 AM Ilya Biryukov via cfe-commits
>  wrote:
> >
> > Author: ibiryukov
> > Date: Mon Dec  3 03:26:35 2018
> > New Revision: 348123
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=348123&view=rev
> > Log:
> > [clang] Do not read from 'test/SemaCXX/Inputs' inside 'test/AST'
> >
> > Our integrate relies on test inputs being taken from the same diretory
> as the
> > test itself.
>
> Sorry about that! I was trying to avoid duplicating the same header
> file in two different places and didn't realize this would cause an
> issue.
>
> ~Aaron
>
> >
> > Added:
> > cfe/trunk/test/AST/Inputs/std-coroutine.h
> > Modified:
> > cfe/trunk/test/AST/coroutine-source-location-crash.cpp
> >
> > Added: cfe/trunk/test/AST/Inputs/std-coroutine.h
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/AST/Inputs/std-coroutine.h?rev=348123&view=auto
> >
> ==
> > --- cfe/trunk/test/AST/Inputs/std-coroutine.h (added)
> > +++ cfe/trunk/test/AST/Inputs/std-coroutine.h Mon Dec  3 03:26:35 2018
> > @@ -0,0 +1,37 @@
> > +// RUN: %clang_cc1 -triple x86_64-apple-darwin9 %s -std=c++14
> -fcoroutines-ts -fsyntax-only -Wignored-qualifiers -Wno-error=return-type
> -verify -fblocks -Wno-unreachable-code -Wno-unused-value
> > +#ifndef STD_COROUTINE_H
> > +#define STD_COROUTINE_H
> > +
> > +namespace std {
> > +namespace experimental {
> > +
> > +template 
> > +struct coroutine_traits { using promise_type = typename
> Ret::promise_type; };
> > +
> > +template 
> > +struct coroutine_handle {
> > +  static coroutine_handle from_address(void *);
> > +};
> > +template <>
> > +struct coroutine_handle {
> > +  template 
> > +  coroutine_handle(coroutine_handle);
> > +  static coroutine_handle from_address(void *);
> > +};
> > +
> > +struct suspend_always {
> > +  bool await_ready() { return false; }
> > +  void await_suspend(coroutine_handle<>) {}
> > +  void await_resume() {}
> > +};
> > +
> > +struct suspend_never {
> > +  bool await_ready() { return true; }
> > +  void await_suspend(coroutine_handle<>) {}
> > +  void await_resume() {}
> > +};
> > +
> > +} // namespace experimental
> > +} // namespace std
> > +
> > +#endif // STD_COROUTINE_H
> >
> > Modified: cfe/trunk/test/AST/coroutine-source-location-crash.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/AST/coroutine-source-location-crash.cpp?rev=348123&r1=348122&r2=348123&view=diff
> >
> ==
> > --- cfe/trunk/test/AST/coroutine-source-location-crash.cpp (original)
> > +++ cfe/trunk/test/AST/coroutine-source-location-crash.cpp Mon Dec  3
> 03:26:35 2018
> > @@ -1,6 +1,6 @@
> >  // RUN: %clang_cc1 -triple x86_64-apple-darwin9 %s -std=c++14
> -fcoroutines-ts \
> >  // RUN:-fsyntax-only -ast-dump | FileCheck %s
> > -#include "../SemaCXX/Inputs/std-coroutine.h"
> > +#include "Inputs/std-coroutine.h"
> >
> >  using namespace std::experimental;
> >
> >
> >
> > ___
> > cfe-commits mailing list
> > cfe-commits@lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>


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

[PATCH] D41005: Reuse preamble even if an unsaved file does not exist

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: include/clang/Frontend/ASTUnit.h:581
 
+  unsigned getPreambleCounter() const { return PreambleCounter; }
+

NIT: `getPreambleCounterForTests()`? This is clearly an internal detail, would 
try giving it a name that discourages using it outside the testing code.



Comment at: lib/Frontend/ASTUnit.cpp:1369
 // Try again next time.
-PreambleRebuildCounter = 1;
+++PreambleCounter;
+PreambleRebuildCountdown = 1;

Why not increase this counter unconditionally for **every** error? (And a 
non-error case too, I guess)



Comment at: lib/Frontend/PrecompiledPreamble.cpp:471
+} else {
+  auto FilePath = PathNormalized(RB.first.begin(), RB.first.end());
+  OverridenFileBuffers[FilePath] = PreambleHash;

Could we avoid adding path normalization in this patch? Would it break anything?
This is clearly an important detail that preamble **might** get wrong, but I'd 
rather add it in a separate patch to avoid putting multiple things in the same 
patch.


Repository:
  rC Clang

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

https://reviews.llvm.org/D41005



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

[PATCH] D54995: [MemoryBuffer] Add the setter to be able to force disabled mmap

[Ivan Donchevskii via Phabricator via cfe-commits]

yvvan added a comment.

@ilya-biryukov

Hm. What about another way around? - We have user include paths (-I) and report 
them to the filesystem. This means that we have specific paths under which 
nothing can be mmaped and everything else can be. In particular cases we can 
also report -isystem there. This is quite the same logic as current isVolatile 
parameter but is set only once for each path.


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

https://reviews.llvm.org/D54995



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