[PATCH] D36918: [Sema] Take into account the current context when checking the accessibility of a member function pointer

[Akira Hatanaka via Phabricator via cfe-commits]

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

Add a test case for the Chromium failure. Also, simplify a bit by capturing the 
calling context in Sema::DeduceTemplateArguments.


Repository:
  rC Clang

https://reviews.llvm.org/D36918

Files:
  lib/Sema/SemaTemplateDeduction.cpp
  test/SemaCXX/access.cpp


Index: test/SemaCXX/access.cpp
===
--- test/SemaCXX/access.cpp
+++ test/SemaCXX/access.cpp
@@ -169,3 +169,50 @@
   }
   void bar() { foo(); }
 }
+
+namespace OverloadedMemberFunctionPointer {
+  template
+  void func0() {}
+
+  template
+  void func1() {}
+
+  template
+  void func2(void(*fn)()) {} // expected-note 2 {{candidate function not 
viable: no overload of 'func}}
+
+  class C {
+  private:
+friend void friendFunc();
+void overloadedMethod();
+  protected:
+void overloadedMethod(int);
+  public:
+void overloadedMethod(int, int);
+void method() {
+  func2();
+  func2();
+}
+  };
+
+  void friendFunc() {
+func2();
+func2();
+  }
+
+  void nonFriendFunc() {
+func2(); // expected-error {{no 
matching function for call to 'func2'}}
+func2(); // expected-error {{no 
matching function for call to 'func2'}}
+  }
+
+  // r325321 caused an assertion failure when the following code was compiled.
+  class A {
+template  static bool foo1() { return true; }
+
+  public:
+void init(bool c) {
+  if (c) {
+auto f = foo1;
+  }
+}
+  };
+}
Index: lib/Sema/SemaTemplateDeduction.cpp
===
--- lib/Sema/SemaTemplateDeduction.cpp
+++ lib/Sema/SemaTemplateDeduction.cpp
@@ -3803,10 +3803,17 @@
   return Result;
   }
 
+  // Capture the context in which the function call is made. This is the 
context
+  // that is needed when the accessibility of a class member used as a template
+  // argument is checked.
+  DeclContext *CallingCtx = CurContext;
+
   return FinishTemplateArgumentDeduction(
   FunctionTemplate, Deduced, NumExplicitlySpecified, Specialization, Info,
-  , PartialOverloading,
-  [&]() { return CheckNonDependent(ParamTypesForArgChecking); });
+  , PartialOverloading, [&, CallingCtx]() {
+ContextRAII SavedContext(*this, CallingCtx);
+return CheckNonDependent(ParamTypesForArgChecking);
+  });
 }
 
 QualType Sema::adjustCCAndNoReturn(QualType ArgFunctionType,


Index: test/SemaCXX/access.cpp
===
--- test/SemaCXX/access.cpp
+++ test/SemaCXX/access.cpp
@@ -169,3 +169,50 @@
   }
   void bar() { foo(); }
 }
+
+namespace OverloadedMemberFunctionPointer {
+  template
+  void func0() {}
+
+  template
+  void func1() {}
+
+  template
+  void func2(void(*fn)()) {} // expected-note 2 {{candidate function not viable: no overload of 'func}}
+
+  class C {
+  private:
+friend void friendFunc();
+void overloadedMethod();
+  protected:
+void overloadedMethod(int);
+  public:
+void overloadedMethod(int, int);
+void method() {
+  func2();
+  func2();
+}
+  };
+
+  void friendFunc() {
+func2();
+func2();
+  }
+
+  void nonFriendFunc() {
+func2(); // expected-error {{no matching function for call to 'func2'}}
+func2(); // expected-error {{no matching function for call to 'func2'}}
+  }
+
+  // r325321 caused an assertion failure when the following code was compiled.
+  class A {
+template  static bool foo1() { return true; }
+
+  public:
+void init(bool c) {
+  if (c) {
+auto f = foo1;
+  }
+}
+  };
+}
Index: lib/Sema/SemaTemplateDeduction.cpp
===
--- lib/Sema/SemaTemplateDeduction.cpp
+++ lib/Sema/SemaTemplateDeduction.cpp
@@ -3803,10 +3803,17 @@
   return Result;
   }
 
+  // Capture the context in which the function call is made. This is the context
+  // that is needed when the accessibility of a class member used as a template
+  // argument is checked.
+  DeclContext *CallingCtx = CurContext;
+
   return FinishTemplateArgumentDeduction(
   FunctionTemplate, Deduced, NumExplicitlySpecified, Specialization, Info,
-  , PartialOverloading,
-  [&]() { return CheckNonDependent(ParamTypesForArgChecking); });
+  , PartialOverloading, [&, CallingCtx]() {
+ContextRAII SavedContext(*this, CallingCtx);
+return CheckNonDependent(ParamTypesForArgChecking);
+  });
 }
 
 QualType Sema::adjustCCAndNoReturn(QualType ArgFunctionType,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: include/clang/Basic/DiagnosticCommonKinds.td:172
+def err_fixed_point_only_allowed_in_c : Error<
+  "Fixed point types are only allowed in C">;
 

leonardchan wrote:
> leonardchan wrote:
> > leonardchan wrote:
> > > rsmith wrote:
> > > > Diagnostics should not be capitalized. Also, we generally allow 
> > > > conforming C extensions to be used in other languages unless there is a 
> > > > really good reason not to.
> > > We decided not to allow fixed point types in other languages because 
> > > there is no specification provided in N1169 for addressing some features 
> > > in other languages. Using C++ as an example, N1169 does not provide 
> > > recommended characters when name mangling so we do not allow this in C++.
> > Actually, scratch that. We will be enabling it since GCC does. Will update 
> > this and other relevant C++ related code appropriately.
> Actually, the main thing that was preventing us from allowing this in C++ was 
> no standardized characters for name mangling. GCC seems to use the same 
> characters as some integral types (`short _Accum` uses `s`, `_Accum` uses 
> `i`, ...) but this would mean that a function that takes a `short _Accum` as 
> a sole argument would also be mangled the same as a similarly named function 
> that takes a `short`.
> 
> Would copying GCC take priority over not having characters specific for these 
> types? This standard also proposes 24 different types, of which only 6 are 
> included in this patch.
That makes it sound like GCC ignores `_Accum` and just mangles the unmodified 
type, which is clearly a bug that should not be imitated.  You should raise 
this issue to the Itanium C++ ABI group and pick something unambiguous as a 
placeholder.


Repository:
  rC Clang

https://reviews.llvm.org/D46084



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

[PATCH] D47103: Implement strip.invariant.group

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/lib/CodeGen/CGExpr.cpp:3858
+}
+  }
+

Prazek wrote:
> rjmccall wrote:
> > Please add a comment explaining why this is necessary.  (I'm actually not 
> > sure why it is, because surely the invariant groups we generate don't 
> > contain assumptions about memory from fields, right?)
> Short answer: you can only make virtual calls on a dynamic pointer that 
> carries invariant.group and you can't do anything other because it could leak 
> the information about this pointer (which when used with comparison could 
> break devirtualization). 
Alright, sure.


Repository:
  rL LLVM

https://reviews.llvm.org/D47103



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

[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

After further discussion, we think the best approach for now would be only 
supporting fixed point types in C, then go back and support C++ once there is a 
standardized way for mangling the fixed point types under itanium.


Repository:
  rC Clang

https://reviews.llvm.org/D46084



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

[PATCH] D47223: [clangd] Handle enumerators in named, unscoped enums similarly to scoped enums

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

malaperle added a comment.

In https://reviews.llvm.org/D47223#1109247, @ilya-biryukov wrote:

> I'm not sure if we have tests for that, but I remember that we kept the 
> enumerators in the outer scope so that completion could find them..
>  Am I right that this patch will change the behavior and we won't get 
> enumerators in the following example:
>
>   /// foo.h
>   enum Foo {
> A, B, C
>   };
>  
>   /// foo.cpp
>   #include "foo.h"
>  
>   int a = ^ // <-- A, B, C should be in completion list here.
>


Not quite but still potentially problematic. With the patch, A, B, C would be 
found but not ::A, ::B, ::C.

> It's one of those cases where code completion and workspace symbol search 
> seem to want different results :-(
>  I suggest to add an extra string field for containing unscoped enum name, 
> maybe into symbol details? And add a parameter to `Index::fuzzyFind` on 
> whether we need to match enum scopes or not.
>  +@ioeric, +@sammccall,  WDYT?

I'll wait to see what others think before changing it. But I feel it's a bit 
odd that completion and workspace symbols would be inconsistent. I'd rather 
have it that A, ::A, and Foo::A work for both completion and workspace. Maybe 
it would complicate things too much...


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D47223



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

[PATCH] D44954: [clangd] Add "member" symbols to the index

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

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

Use "SupportGlobalCompletion", white-list decl contexts, add more tests


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44954

Files:
  clangd/CodeComplete.cpp
  clangd/index/Index.h
  clangd/index/MemIndex.cpp
  clangd/index/SymbolCollector.cpp
  clangd/index/SymbolYAML.cpp
  unittests/clangd/CodeCompleteTests.cpp
  unittests/clangd/FileIndexTests.cpp
  unittests/clangd/FindSymbolsTests.cpp
  unittests/clangd/SymbolCollectorTests.cpp

Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -67,6 +67,9 @@
   Pos.end.character);
 }
 MATCHER_P(Refs, R, "") { return int(arg.References) == R; }
+MATCHER_P(SupportGlobalCompletion, SupportGlobalCompletion, "") {
+  return arg.SupportGlobalCompletion == SupportGlobalCompletion;
+}
 
 namespace clang {
 namespace clangd {
@@ -163,8 +166,20 @@
 TEST_F(SymbolCollectorTest, CollectSymbols) {
   const std::string Header = R"(
 class Foo {
+  Foo() {}
+  Foo(int a) {}
+  void f();
+  friend void f1();
+  friend class Friend;
+  Foo& operator=(const Foo&);
+  ~Foo();
+  class Nested {
   void f();
+  };
 };
+class Friend {
+};
+
 void f1();
 inline void f2() {}
 static const int KInt = 2;
@@ -198,25 +213,68 @@
 } // namespace foo
   )";
   runSymbolCollector(Header, /*Main=*/"");
-  EXPECT_THAT(Symbols,
-  UnorderedElementsAreArray(
-  {QName("Foo"), QName("f1"), QName("f2"), QName("KInt"),
-   QName("kStr"), QName("foo"), QName("foo::bar"),
-   QName("foo::int32"), QName("foo::int32_t"), QName("foo::v1"),
-   QName("foo::bar::v2"), QName("foo::baz")}));
+  EXPECT_THAT(Symbols, UnorderedElementsAreArray(
+   {QName("Foo"),  QName("Foo::Foo"),
+QName("Foo::Foo"), QName("Foo::f"),
+QName("Foo::~Foo"),QName("Foo::operator="),
+QName("Foo::Nested"),  QName("Foo::Nested::f"),
+QName("Friend"),   QName("f1"),
+QName("f2"),   QName("KInt"),
+QName("kStr"), QName("foo"),
+QName("foo::bar"), QName("foo::int32"),
+QName("foo::int32_t"), QName("foo::v1"),
+QName("foo::bar::v2"), QName("foo::baz")}));
 }
 
 TEST_F(SymbolCollectorTest, Template) {
   Annotations Header(R"(
 // Template is indexed, specialization and instantiation is not.
-template  struct [[Tmpl]] {T x = 0;};
+template  struct [[Tmpl]] {T $xdecl[[x]] = 0;};
 template <> struct Tmpl {};
 extern template struct Tmpl;
 template struct Tmpl;
   )");
   runSymbolCollector(Header.code(), /*Main=*/"");
-  EXPECT_THAT(Symbols, UnorderedElementsAreArray({AllOf(
-   QName("Tmpl"), DeclRange(Header.range()))}));
+  EXPECT_THAT(Symbols,
+  UnorderedElementsAreArray(
+  {AllOf(QName("Tmpl"), DeclRange(Header.range())),
+   AllOf(QName("Tmpl::x"), DeclRange(Header.range("xdecl")))}));
+}
+
+TEST_F(SymbolCollectorTest, ObjCSymbols) {
+  const std::string Header = R"(
+@interface Person
+- (void)someMethodName:(void*)name1 lastName:(void*)lName;
+@end
+
+@implementation Person
+- (void)someMethodName:(void*)name1 lastName:(void*)lName{
+  int foo;
+  ^(int param){ int bar; };
+}
+@end
+
+@interface Person (MyCategory)
+- (void)someMethodName2:(void*)name2;
+@end
+
+@implementation Person (MyCategory)
+- (void)someMethodName2:(void*)name2 {
+  int foo2;
+}
+@end
+
+@protocol MyProtocol
+- (void)someMethodName3:(void*)name3;
+@end
+  )";
+  TestFileName = "test.m";
+  runSymbolCollector(Header, /*Main=*/"", {"-fblocks"});
+  EXPECT_THAT(Symbols,
+  UnorderedElementsAre(
+  QName("Person"), QName("Person::someMethodName:lastName:"),
+  QName("MyCategory"), QName("Person::someMethodName2:"),
+  QName("MyProtocol"), QName("MyProtocol::someMethodName3:")));
 }
 
 TEST_F(SymbolCollectorTest, Locations) {
@@ -334,29 +392,30 @@
   Green
 };
 enum class Color2 {
-  Yellow // ignore
+  Yellow
 };
 namespace ns {
 enum {
   Black
 };
 }
   )";
   runSymbolCollector(Header, /*Main=*/"");
-  EXPECT_THAT(Symbols, UnorderedElementsAre(QName("Red"), QName("Color"),
-QName("Green"), QName("Color2"),
-QName("ns"), QName("ns::Black")));
+  EXPECT_THAT(Symbols,
+  

[PATCH] D47305: [analyzer] pr37270: Fix binding constructed object to DeclStmt when ConstructionContext is already lost.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet, 
rnkovacs.
Herald added subscribers: cfe-commits, baloghadamsoftware.

As explained in http://lists.llvm.org/pipermail/cfe-dev/2018-May/058055.html, 
`findDirectConstructorForCurrentCFGElement()` was not working correctly in the 
current example and erroneously made us think that we've constructed into a 
dummy temporary region rather than into the variable. Instead, it was proposed 
to track it in the program state every time we are performing construction 
correctly.

Additionally this information will be used to maintain liveness of the 
variable's Store bindings, because previously an incorrect Environment binding 
of the target region to the construct-expression was used for that purpose. 
Such binding is incorrect because the constructor is a prvalue of an object 
type and should therefore have a NonLoc representing its symbolic value. 
Therefore the hack implemented by `isTemporaryPRValue()` can be safely removed.

`findDirectConstructorForCurrentCFGElement()` cannot be removed yet because it 
is also used for constructor member initializers for the same purpose. It 
doesn't seem to introduce bugs though.


Repository:
  rC Clang

https://reviews.llvm.org/D47305

Files:
  lib/StaticAnalyzer/Core/ExprEngineC.cpp
  lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
  test/Analysis/cxx17-mandatory-elision.cpp

Index: test/Analysis/cxx17-mandatory-elision.cpp
===
--- test/Analysis/cxx17-mandatory-elision.cpp
+++ test/Analysis/cxx17-mandatory-elision.cpp
@@ -3,6 +3,26 @@
 
 void clang_analyzer_eval(bool);
 
+namespace variable_functional_cast_crash {
+
+struct A {
+  A(int) {}
+};
+
+void foo() {
+  A a = A(0);
+}
+
+struct B {
+  A a;
+  B(): a(A(0)) {}
+};
+
+} // namespace variable_functional_cast_crash
+
+
+namespace address_vector_tests {
+
 template  struct AddressVector {
   T *buf[10];
   int len;
@@ -56,3 +76,5 @@
   clang_analyzer_eval(v.buf[4] == ); // expected-warning{{TRUE}}
 #endif
 }
+
+} // namespace address_vector_tests
Index: lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -193,23 +193,6 @@
   return RuntimeCallee->getCanonicalDecl() != StaticDecl->getCanonicalDecl();
 }
 
-/// Returns true if the CXXConstructExpr \p E was intended to construct a
-/// prvalue for the region in \p V.
-///
-/// Note that we can't just test for rvalue vs. glvalue because
-/// CXXConstructExprs embedded in DeclStmts and initializers are considered
-/// rvalues by the AST, and the analyzer would like to treat them as lvalues.
-static bool isTemporaryPRValue(const CXXConstructExpr *E, SVal V) {
-  if (E->isGLValue())
-return false;
-
-  const MemRegion *MR = V.getAsRegion();
-  if (!MR)
-return false;
-
-  return isa(MR);
-}
-
 /// The call exit is simulated with a sequence of nodes, which occur between
 /// CallExitBegin and CallExitEnd. The following operations occur between the
 /// two program points:
@@ -269,11 +252,7 @@
   loc::MemRegionVal This =
 svalBuilder.getCXXThis(CCE->getConstructor()->getParent(), calleeCtx);
   SVal ThisV = state->getSVal(This);
-
-  // If the constructed object is a temporary prvalue, get its bindings.
-  if (isTemporaryPRValue(CCE, ThisV))
-ThisV = state->getSVal(ThisV.castAs());
-
+  ThisV = state->getSVal(ThisV.castAs());
   state = state->BindExpr(CCE, callerCtx, ThisV);
 }
 
@@ -574,11 +553,7 @@
 }
   } else if (const CXXConstructorCall *C = dyn_cast()){
 SVal ThisV = C->getCXXThisVal();
-
-// If the constructed object is a temporary prvalue, get its bindings.
-if (isTemporaryPRValue(cast(E), ThisV))
-  ThisV = State->getSVal(ThisV.castAs());
-
+ThisV = State->getSVal(ThisV.castAs());
 return State->BindExpr(E, LCtx, ThisV);
   }
 
Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -125,9 +125,11 @@
   const auto *Var = cast(DS->getSingleDecl());
   SVal LValue = State->getLValue(Var, LCtx);
   QualType Ty = Var->getType();
-  return std::make_pair(
-  State,
-  makeZeroElementRegion(State, LValue, Ty, CallOpts.IsArrayCtorOrDtor));
+  LValue =
+  makeZeroElementRegion(State, LValue, Ty, CallOpts.IsArrayCtorOrDtor);
+  State =
+  addObjectUnderConstruction(State, DSCC->getDeclStmt(), LCtx, LValue);
+  return std::make_pair(State, LValue);
 }
 case ConstructionContext::SimpleConstructorInitializerKind: {
   const auto *ICC = cast(CC);
Index: 

[PATCH] D47303: [analyzer] NFC: Merge object construction related state traits into one.

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov requested changes to this revision.
george.karpenkov added a comment.
This revision now requires changes to proceed.

Definitely looks much cleaner, some nits inline. Can we protect against API 
misuse?




Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h:769
+  /// that tracks objects under construction.
+  static ProgramStateRef finishObjectConstruction(ProgramStateRef State,
+  const Stmt *S,

Should we somehow assert that those functions are called in the correct order? 
What will happen if e.g. `finishObjectConstruction` is called twice with the 
same statement? 



Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:101
 
-using InitializedTemporariesMap =
-llvm::ImmutableMap;
-
-// Keeps track of whether CXXBindTemporaryExpr nodes have been evaluated.
-// The StackFrameContext assures that nested calls due to inlined recursive
-// functions do not interfere.
-REGISTER_TRAIT_WITH_PROGRAMSTATE(InitializedTemporaries,
- InitializedTemporariesMap)
-
-using TemporaryMaterializationMap =
-llvm::ImmutableMap;
-
-// Keeps track of temporaries that will need to be materialized later.
-// The StackFrameContext assures that nested calls due to inlined recursive
-// functions do not interfere.
-REGISTER_TRAIT_WITH_PROGRAMSTATE(TemporaryMaterializations,
- TemporaryMaterializationMap)
-
-using CXXNewAllocatorValuesMap =
-llvm::ImmutableMap;
-
-// Keeps track of return values of various operator new() calls between
-// evaluation of the inlined operator new(), through the constructor call,
-// to the actual evaluation of the CXXNewExpr.
-// TODO: Refactor the key for this trait into a LocationContext sub-class,
-// which would be put on the stack of location contexts before operator new()
-// is evaluated, and removed from the stack when the whole CXXNewExpr
-// is fully evaluated.
-// Probably do something similar to the previous trait as well.
-REGISTER_TRAIT_WITH_PROGRAMSTATE(CXXNewAllocatorValues,
- CXXNewAllocatorValuesMap)
+// Keeps track of whether objects corresponding to the statement have been
+// fully constructed.

The comment seems incorrect, the map is not only answering the "whether" 
question, it also maps those pairs into --- (their regions where these objects 
are constructed into?)



Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:377
+  if (!State->contains(Key)) {
+return State->set(Key, V);
   }

nitpick: most C++ containers eliminate the need for two lookups by allowing the 
`get` method to return a reference. I'm not sure whether this can be done here 
though.


Repository:
  rC Clang

https://reviews.llvm.org/D47303



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

[PATCH] D47229: Make atomic non-member functions as nonnull

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.



Comment at: lib/Sema/SemaChecking.cpp:3497
+else if (Form == Copy || Form == Xchg) {
+  if (!IsC11 && !IsN)
+// The value pointer is always dereferenced, a nullptr is 
undefined.

Nit: might make more sense to check if `ByValType` is a pointer here instead of 
duplicating the `if` condition from above.


Repository:
  rC Clang

https://reviews.llvm.org/D47229



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

[PATCH] D47304: [analyzer] NFC: Merge the functions for obtaining constructed object location and storing this location for later use.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet, 
rnkovacs.
Herald added subscribers: cfe-commits, baloghadamsoftware.

`getLocationForConstructedObject()` looks at the construction context and 
figures out what region should represent the object.

`markStatementsCorrespondingToConstructedObject()` looks at the construction 
context and figures out what statements will need to retrieve that region 
directly later.

These functions are coupled and code is duplicated between them. They should be 
merged. The resulting function is large, so it'd probably later need to be 
split in a different manner (i.e. by construction context kinds). It'll also 
soon become recursive as we add better support for copy elision at return 
sites. I really hope we don't end up coding any sort of 
ConstructionContextVisitor.

No functional change intended here; this is a refactoring pass.


Repository:
  rC Clang

https://reviews.llvm.org/D47304

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp

Index: lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -587,18 +587,20 @@
   unsigned Count = currBldrCtx->blockCount();
   if (auto RTC = getCurrentCFGElement().getAs()) {
 // Conjure a temporary if the function returns an object by value.
-MemRegionManager  = svalBuilder.getRegionManager();
-const CXXTempObjectRegion *TR = MRMgr.getCXXTempObjectRegion(E, LCtx);
-State = markStatementsCorrespondingToConstructedObject(
-State, RTC->getConstructionContext(), LCtx, loc::MemRegionVal(TR));
-
+SVal Target;
+assert(RTC->getStmt() == Call.getOriginExpr());
+EvalCallOptions CallOpts; // FIXME: We won't really need those.
+std::tie(State, Target) =
+prepareForObjectConstruction(Call.getOriginExpr(), State, LCtx,
+ RTC->getConstructionContext(), CallOpts);
+assert(Target.getAsRegion());
 // Invalidate the region so that it didn't look uninitialized. Don't notify
 // the checkers.
-State = State->invalidateRegions(TR, E, Count, LCtx,
+State = State->invalidateRegions(Target.getAsRegion(), E, Count, LCtx,
  /* CausedByPointerEscape=*/false, nullptr,
  , nullptr);
 
-R = State->getSVal(TR, E->getType());
+R = State->getSVal(Target.castAs(), E->getType());
   } else {
 // Conjure a symbol if the return value is unknown.
 
Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -110,13 +110,9 @@
   return LValue;
 }
 
-
-SVal ExprEngine::getLocationForConstructedObject(const CXXConstructExpr *CE,
- ExplodedNode *Pred,
- const ConstructionContext *CC,
- EvalCallOptions ) {
-  const LocationContext *LCtx = Pred->getLocationContext();
-  ProgramStateRef State = Pred->getState();
+std::pair ExprEngine::prepareForObjectConstruction(
+const Expr *E, ProgramStateRef State, const LocationContext *LCtx,
+const ConstructionContext *CC, EvalCallOptions ) {
   MemRegionManager  = getSValBuilder().getRegionManager();
 
   // See if we're constructing an existing region by looking at the
@@ -129,8 +125,9 @@
   const auto *Var = cast(DS->getSingleDecl());
   SVal LValue = State->getLValue(Var, LCtx);
   QualType Ty = Var->getType();
-  return makeZeroElementRegion(State, LValue, Ty,
-   CallOpts.IsArrayCtorOrDtor);
+  return std::make_pair(
+  State,
+  makeZeroElementRegion(State, LValue, Ty, CallOpts.IsArrayCtorOrDtor));
 }
 case ConstructionContext::SimpleConstructorInitializerKind: {
   const auto *ICC = cast(CC);
@@ -154,7 +151,7 @@
   QualType Ty = Field->getType();
   FieldVal = makeZeroElementRegion(State, FieldVal, Ty,
CallOpts.IsArrayCtorOrDtor);
-  return FieldVal;
+  return std::make_pair(State, FieldVal);
 }
 case ConstructionContext::NewAllocatedObjectKind: {
   if (AMgr.getAnalyzerOptions().mayInlineCXXAllocator()) {
@@ -167,65 +164,97 @@
 // TODO: In fact, we need to call the constructor for every
 // allocated element, not just the first one!
 CallOpts.IsArrayCtorOrDtor = true;
-return loc::MemRegionVal(getStoreManager().GetElementZeroRegion(
-   

[PATCH] D47067: Update NRVO logic to support early return

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

rsmith added a comment.

Thanks! This looks like exactly the right way to compute when to apply NRVO. 
It'd be good to track (in your commit message at least) that this addresses 
PR13067.




Comment at: lib/Sema/SemaDecl.cpp:12760
   // to deduce an implicit return type.
-  if (FD->getReturnType()->isRecordType() &&
-  (!getLangOpts().CPlusPlus || !FD->isDependentContext()))
+  if (!FD->getReturnType()->isScalarType())
 computeNRVO(Body, getCurFunction());

tzik wrote:
> Quuxplusone wrote:
> > What is the purpose of this change?
> > If it's "no functional change" it should be done separately IMHO; if it is 
> > supposed to change codegen, then it needs some codegen tests. (From looking 
> > at the code: maybe this affects codegen on functions that return member 
> > function pointers by value?)
> I think the previous implementation was incorrect.
> Though computeNRVO clears ReturnStmt::NRVOCandidate when the corresponding 
> variable is not NRVO variable, CGStmt checks both of 
> ReturnStmt::NRVOCandidate and VarDecl::NRVOVariable anyway.
> So, computeNRVO took no effect in the previous code, and the absence of 
> computeNRVO here on function templates did not matter.
> Note that there was no chance to call computeNRVO on function template 
> elsewhere too.
> 
> OTOH in the new implementation, computeNRVO is necessary, and its absence on 
> function templates matters.
> 
> We can remove computeNRVO here separately as a NFC patch and readd the new 
> impl to the same place, but it's probably less readable, IMO.
What happens if we can't tell whether we have an NRVO candidate or not until 
instantiation:

```
template X f() {
  T v;
  return v; // is an NRVO candidate if T is X, otherwise is not
}
```

(I think this is not hard to handle: the dependent construct here can only 
affect whether you get NRVO at all, not which variable you perform NRVO on, but 
I want to check that it is handled properly.)



Comment at: test/CodeGenCXX/nrvo.cpp:139
   }
-  // FIXME: we should NRVO this variable too.
-  X x;
+  // CHECK: tail call {{.*}} @_ZN1XC1ERKS_
   return x;

tzik wrote:
> Quuxplusone wrote:
> > You've changed this function from testing one thing with a FIXME, to 
> > testing a completely different thing.  Could you add your new code as a new 
> > function, and leave the old FIXME alone until it's fixed?
> > Alternatively, if your patch actually fixes the FIXME, could you just 
> > replace the FIXME comment with a CHECK and leave the rest of this function 
> > alone?
> My patch fixes the FIXME.
> However, on the resulting code of NRVOing,
> ```
> X y;
> return y;
> ```
> and
> ```
> X x;
> return x;
> ```
> get to the same code and unified. And, the function is simplified as
> ```
> X test3(bool B) {
>   X x;
>   return x;
> }
> ```
> without the if statement.
> So, there will be nothing to CHECK left here if I leave the code as-is. I 
> think that does not fit to the test scenario.
... and this is one reason why we don't generally like clang's IR generation 
tests to enable optimizations. Please consider starting a new test file to test 
the output from clang's frontend rather than the output from -O1.


Repository:
  rC Clang

https://reviews.llvm.org/D47067



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

[PATCH] D47280: [Sema][ObjC] Do not DiagnoseUseOfDecl in LookupMemberExpr

[Steven Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC333148: [Sema][ObjC] Do not DiagnoseUseOfDecl in 
LookupMemberExpr (authored by steven_wu, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D47280?vs=148279=148327#toc

Repository:
  rC Clang

https://reviews.llvm.org/D47280

Files:
  lib/Sema/SemaExprMember.cpp
  test/SemaObjC/property-deprecated-warning.m


Index: test/SemaObjC/property-deprecated-warning.m
===
--- test/SemaObjC/property-deprecated-warning.m
+++ test/SemaObjC/property-deprecated-warning.m
@@ -167,3 +167,14 @@
foo.x = foo.x; // expected-error {{property access is using 'x' method 
which is unavailable}} \
   // expected-warning {{property access is using 'setX:' 
method which is deprecated}}
 }
+
+// test implicit property does not emit duplicated warning.
+@protocol Foo
+- (int)size __attribute__((availability(ios,deprecated=3.0))); // 
expected-note {{'size' has been explicitly marked deprecated here}}
+- (void)setSize: (int)x __attribute__((availability(ios,deprecated=2.0))); // 
expected-note {{'setSize:' has been explicitly marked deprecated here}}
+@end
+
+void testImplicitProperty(id object) {
+  object.size = object.size; // expected-warning {{'size' is deprecated: first 
deprecated in iOS 3.0}} \
+ // expected-warning {{'setSize:' is deprecated: 
first deprecated in iOS 2.0}}
+}
Index: lib/Sema/SemaExprMember.cpp
===
--- lib/Sema/SemaExprMember.cpp
+++ lib/Sema/SemaExprMember.cpp
@@ -1490,9 +1490,6 @@
 }
 
 if (ObjCMethodDecl *OMD = dyn_cast(PMDecl)) {
-  // Check the use of this method.
-  if (S.DiagnoseUseOfDecl(OMD, MemberLoc))
-return ExprError();
   Selector SetterSel =
 SelectorTable::constructSetterSelector(S.PP.getIdentifierTable(),
S.PP.getSelectorTable(),


Index: test/SemaObjC/property-deprecated-warning.m
===
--- test/SemaObjC/property-deprecated-warning.m
+++ test/SemaObjC/property-deprecated-warning.m
@@ -167,3 +167,14 @@
 	foo.x = foo.x; // expected-error {{property access is using 'x' method which is unavailable}} \
 		   // expected-warning {{property access is using 'setX:' method which is deprecated}}
 }
+
+// test implicit property does not emit duplicated warning.
+@protocol Foo
+- (int)size __attribute__((availability(ios,deprecated=3.0))); // expected-note {{'size' has been explicitly marked deprecated here}}
+- (void)setSize: (int)x __attribute__((availability(ios,deprecated=2.0))); // expected-note {{'setSize:' has been explicitly marked deprecated here}}
+@end
+
+void testImplicitProperty(id object) {
+  object.size = object.size; // expected-warning {{'size' is deprecated: first deprecated in iOS 3.0}} \
+ // expected-warning {{'setSize:' is deprecated: first deprecated in iOS 2.0}}
+}
Index: lib/Sema/SemaExprMember.cpp
===
--- lib/Sema/SemaExprMember.cpp
+++ lib/Sema/SemaExprMember.cpp
@@ -1490,9 +1490,6 @@
 }
 
 if (ObjCMethodDecl *OMD = dyn_cast(PMDecl)) {
-  // Check the use of this method.
-  if (S.DiagnoseUseOfDecl(OMD, MemberLoc))
-return ExprError();
   Selector SetterSel =
 SelectorTable::constructSetterSelector(S.PP.getIdentifierTable(),
S.PP.getSelectorTable(),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r333148 - [Sema][ObjC] Do not DiagnoseUseOfDecl in LookupMemberExpr

[Steven Wu via cfe-commits]

Author: steven_wu
Date: Wed May 23 18:01:43 2018
New Revision: 333148

URL: http://llvm.org/viewvc/llvm-project?rev=333148=rev
Log:
[Sema][ObjC] Do not DiagnoseUseOfDecl in LookupMemberExpr

Summary:
Remove the call to DiagnoseUseOfDecl in LookupMemberExpr because:
1. LookupMemberExpr eagerly lookup both getter and setter, reguardless
if they are used or not. It causes wrong diagnostics if you are only
using getter.
2. LookupMemberExpr only diagnoses getter, but not setter.
3. ObjCPropertyOpBuilder already DiagnoseUseOfDecl when building getter
and setter. Doing it again in LookupMemberExpr causes duplicated
diagnostics.

rdar://problem/38479756

Reviewers: erik.pilkington, arphaman, doug.gregor

Reviewed By: arphaman

Subscribers: cfe-commits

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

Modified:
cfe/trunk/lib/Sema/SemaExprMember.cpp
cfe/trunk/test/SemaObjC/property-deprecated-warning.m

Modified: cfe/trunk/lib/Sema/SemaExprMember.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprMember.cpp?rev=333148=333147=333148=diff
==
--- cfe/trunk/lib/Sema/SemaExprMember.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExprMember.cpp Wed May 23 18:01:43 2018
@@ -1490,9 +1490,6 @@ static ExprResult LookupMemberExpr(Sema
 }
 
 if (ObjCMethodDecl *OMD = dyn_cast(PMDecl)) {
-  // Check the use of this method.
-  if (S.DiagnoseUseOfDecl(OMD, MemberLoc))
-return ExprError();
   Selector SetterSel =
 SelectorTable::constructSetterSelector(S.PP.getIdentifierTable(),
S.PP.getSelectorTable(),

Modified: cfe/trunk/test/SemaObjC/property-deprecated-warning.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/property-deprecated-warning.m?rev=333148=333147=333148=diff
==
--- cfe/trunk/test/SemaObjC/property-deprecated-warning.m (original)
+++ cfe/trunk/test/SemaObjC/property-deprecated-warning.m Wed May 23 18:01:43 
2018
@@ -167,3 +167,14 @@ void testUserAccessorAttributes(Foo *foo
foo.x = foo.x; // expected-error {{property access is using 'x' method 
which is unavailable}} \
   // expected-warning {{property access is using 'setX:' 
method which is deprecated}}
 }
+
+// test implicit property does not emit duplicated warning.
+@protocol Foo
+- (int)size __attribute__((availability(ios,deprecated=3.0))); // 
expected-note {{'size' has been explicitly marked deprecated here}}
+- (void)setSize: (int)x __attribute__((availability(ios,deprecated=2.0))); // 
expected-note {{'setSize:' has been explicitly marked deprecated here}}
+@end
+
+void testImplicitProperty(id object) {
+  object.size = object.size; // expected-warning {{'size' is deprecated: first 
deprecated in iOS 3.0}} \
+ // expected-warning {{'setSize:' is deprecated: 
first deprecated in iOS 2.0}}
+}


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

[PATCH] D47280: [Sema][ObjC] Do not DiagnoseUseOfDecl in LookupMemberExpr

[Alex Lorenz via Phabricator via cfe-commits]

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

LGTM.


Repository:
  rC Clang

https://reviews.llvm.org/D47280



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

[PATCH] D47303: [analyzer] NFC: Merge object construction related state traits into one.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet, 
rnkovacs.
Herald added subscribers: cfe-commits, baloghadamsoftware.

As noticed in http://lists.llvm.org/pipermail/cfe-dev/2018-May/058055.html we 
need a path-sensitive program state trait that for, essentially, every 
constructed object maintains the region of that object until the statement that 
consumes the object is encountered.

We already have such trait for new-expressions and bind/materialize temporary 
expressions, which are three separate traits. Because we plan to add more, it 
doesn't make sense to maintain that many traits that do the same thing in 
different circumstances, so i guess it's better to merge them into a single 
multi-purpose trait. "Multi-purpose" is definitely not the top buzzword in 
programming, but here i believe it's worth it because the underlying reasoning 
for needing these traits and needing them to work in a particular manner is the 
same. We need them because our constructor expressions are turned inside out, 
and we need a better connection between "inside" and "out" in both directions. 
One of these directions is handled by the ConstructionContext; the other is 
path-sensitive.

No functional change intended here; this is a refactoring.


Repository:
  rC Clang

https://reviews.llvm.org/D47303

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp

Index: lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -288,8 +288,8 @@
   AllocV, CNE->getType(),
   getContext().getPointerType(getContext().VoidTy));
 
-  state =
-  setCXXNewAllocatorValue(state, CNE, calleeCtx->getParent(), AllocV);
+  state = addObjectUnderConstruction(state, CNE, calleeCtx->getParent(),
+ AllocV);
 }
   }
 
@@ -354,8 +354,9 @@
  /*WasInlined=*/true);
   for (auto I : DstPostPostCallCallback) {
 getCheckerManager().runCheckersForNewAllocator(
-CNE, getCXXNewAllocatorValue(I->getState(), CNE,
- calleeCtx->getParent()),
+CNE,
+*getObjectUnderConstruction(I->getState(), CNE,
+calleeCtx->getParent()),
 DstPostCall, I, *this,
 /*WasInlined=*/true);
   }
@@ -588,8 +589,8 @@
 // Conjure a temporary if the function returns an object by value.
 MemRegionManager  = svalBuilder.getRegionManager();
 const CXXTempObjectRegion *TR = MRMgr.getCXXTempObjectRegion(E, LCtx);
-State = addAllNecessaryTemporaryInfo(State, RTC->getConstructionContext(),
- LCtx, TR);
+State = markStatementsCorrespondingToConstructedObject(
+State, RTC->getConstructionContext(), LCtx, loc::MemRegionVal(TR));
 
 // Invalidate the region so that it didn't look uninitialized. Don't notify
 // the checkers.
Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -111,11 +111,10 @@
 }
 
 
-const MemRegion *
-ExprEngine::getRegionForConstructedObject(const CXXConstructExpr *CE,
-  ExplodedNode *Pred,
-  const ConstructionContext *CC,
-  EvalCallOptions ) {
+SVal ExprEngine::getLocationForConstructedObject(const CXXConstructExpr *CE,
+ ExplodedNode *Pred,
+ const ConstructionContext *CC,
+ EvalCallOptions ) {
   const LocationContext *LCtx = Pred->getLocationContext();
   ProgramStateRef State = Pred->getState();
   MemRegionManager  = getSValBuilder().getRegionManager();
@@ -130,9 +129,8 @@
   const auto *Var = cast(DS->getSingleDecl());
   SVal LValue = State->getLValue(Var, LCtx);
   QualType Ty = Var->getType();
-  LValue =
-  makeZeroElementRegion(State, LValue, Ty, CallOpts.IsArrayCtorOrDtor);
-  return LValue.getAsRegion();
+  return makeZeroElementRegion(State, LValue, Ty,
+   CallOpts.IsArrayCtorOrDtor);
 }
 case ConstructionContext::SimpleConstructorInitializerKind: {
   const auto *ICC = cast(CC);
@@ -156,25 +154,26 @@
   QualType Ty = Field->getType();
   FieldVal = makeZeroElementRegion(State, FieldVal, Ty,

[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added inline comments.



Comment at: include/clang/Basic/DiagnosticCommonKinds.td:172
+def err_fixed_point_only_allowed_in_c : Error<
+  "Fixed point types are only allowed in C">;
 

leonardchan wrote:
> leonardchan wrote:
> > rsmith wrote:
> > > Diagnostics should not be capitalized. Also, we generally allow 
> > > conforming C extensions to be used in other languages unless there is a 
> > > really good reason not to.
> > We decided not to allow fixed point types in other languages because there 
> > is no specification provided in N1169 for addressing some features in other 
> > languages. Using C++ as an example, N1169 does not provide recommended 
> > characters when name mangling so we do not allow this in C++.
> Actually, scratch that. We will be enabling it since GCC does. Will update 
> this and other relevant C++ related code appropriately.
Actually, the main thing that was preventing us from allowing this in C++ was 
no standardized characters for name mangling. GCC seems to use the same 
characters as some integral types (`short _Accum` uses `s`, `_Accum` uses `i`, 
...) but this would mean that a function that takes a `short _Accum` as a sole 
argument would also be mangled the same as a similarly named function that 
takes a `short`.

Would copying GCC take priority over not having characters specific for these 
types? This standard also proposes 24 different types, of which only 6 are 
included in this patch.


Repository:
  rC Clang

https://reviews.llvm.org/D46084



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

[PATCH] D47299: [CodeGenCXX] Emit strip.invariant.group with -fstrict-vtable-pointers

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

rsmith added inline comments.



Comment at: clang/include/clang/AST/DeclCXX.h:779
+  bool mayBeDynamicClass() const {
+return !isCompleteDefinition() || isDynamicClass();
+  }

`isCompleteDefinition` checks whether this declaration of the class is a 
definition, not whether it has a definition anywhere; the latter is what you 
need here. You can use `hasDefinition` to check that.

Please also check `hasAnyDependentBases()` (or add a comment to this function 
to indicate that it may return `false` for a templated class whose 
instantiations might be dynamic classes) -- if a class has dependent bases, we 
might not find out that it's a dynamic class until it's instantiated.



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:1626-1627
+
+  // Casting to pointer that does not carry dynamic information (provided 
by
+  // invariant.group) requires stripping it.
+  Src = Builder.CreateStripInvariantGroup(Src);

Are there any cases where we need a barrier when the destination type is a 
dynamic type here?


Repository:
  rL LLVM

https://reviews.llvm.org/D47299



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

[PATCH] D38708: [AST] Flag the typo-corrected nodes for better tooling

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.
Herald added a subscriber: llvm-commits.

Could you rebase this patch against ToT?

Also, it's not clear to me why only four Exprs (DeclRefExpr, MemberExpr, 
ObjCIvarRefExpr, and ObjCPropertyRefExpr) should have the IsTypoCorrected flag. 
Can you elaborate on how you chose that set of Exprs and whether you plan to 
add the flag to other Exprs?


Repository:
  rL LLVM

https://reviews.llvm.org/D38708



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

[PATCH] D47157: Warning for framework headers using double quote includes

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

In https://reviews.llvm.org/D47157#1110430, @bruno wrote:

> Hi Eugene,
>
> > You could just not include new warning into -Wall and -Wextra. Those who 
> > will want to check #include statements correctness could enable it 
> > explicitly.
>
> This is exactly what's happening in the patch, the new warning isn't part of 
> `-Wall` or `-Wextra`, and is marked `DefaultIgnore`, which means that will 
> fire only when `-Wquoted-include-in-framework-header` is passed to the 
> driver. Am I missing something from your explanation?
>
> Thanks,


Thank you for clarification! Sorry, I didn't know TableGen syntax well enough 
to deduce this from source.


Repository:
  rC Clang

https://reviews.llvm.org/D47157



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

[PATCH] D47273: [bash-completion] Fix tab separation on macOS

[Yuka Takahashi via Phabricator via cfe-commits]

yamaguchi accepted this revision.
yamaguchi added a comment.

LGTM


https://reviews.llvm.org/D47273



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

r333141 - Use zeroinitializer for (trailing zero portion of) large array initializers

[Richard Smith via cfe-commits]

Author: rsmith
Date: Wed May 23 16:41:38 2018
New Revision: 333141

URL: http://llvm.org/viewvc/llvm-project?rev=333141=rev
Log:
Use zeroinitializer for (trailing zero portion of) large array initializers
more reliably.

This re-commits r333044 with a fix for PR37560.

Modified:
cfe/trunk/lib/CodeGen/CGExprConstant.cpp
cfe/trunk/lib/Sema/SemaInit.cpp
cfe/trunk/test/CodeGen/init.c
cfe/trunk/test/CodeGenCXX/cxx11-initializer-aggregate.cpp
cfe/trunk/test/SemaCXX/aggregate-initialization.cpp

Modified: cfe/trunk/lib/CodeGen/CGExprConstant.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprConstant.cpp?rev=333141=333140=333141=diff
==
--- cfe/trunk/lib/CodeGen/CGExprConstant.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExprConstant.cpp Wed May 23 16:41:38 2018
@@ -635,6 +635,60 @@ static ConstantAddress tryEmitGlobalComp
   return ConstantAddress(GV, Align);
 }
 
+static llvm::Constant *
+EmitArrayConstant(CodeGenModule , const ConstantArrayType *DestType,
+  llvm::Type *CommonElementType, unsigned ArrayBound,
+  SmallVectorImpl ,
+  llvm::Constant *Filler) {
+  // Figure out how long the initial prefix of non-zero elements is.
+  unsigned NonzeroLength = ArrayBound;
+  if (Elements.size() < NonzeroLength && Filler->isNullValue())
+NonzeroLength = Elements.size();
+  if (NonzeroLength == Elements.size()) {
+while (NonzeroLength > 0 && Elements[NonzeroLength - 1]->isNullValue())
+  --NonzeroLength;
+  }
+
+  if (NonzeroLength == 0) {
+return llvm::ConstantAggregateZero::get(
+CGM.getTypes().ConvertType(QualType(DestType, 0)));
+  }
+
+  // Add a zeroinitializer array filler if we have lots of trailing zeroes.
+  unsigned TrailingZeroes = ArrayBound - NonzeroLength;
+  if (TrailingZeroes >= 8) {
+assert(Elements.size() >= NonzeroLength &&
+   "missing initializer for non-zero element");
+Elements.resize(NonzeroLength + 1);
+auto *FillerType =
+CommonElementType
+? CommonElementType
+: CGM.getTypes().ConvertType(DestType->getElementType());
+FillerType = llvm::ArrayType::get(FillerType, TrailingZeroes);
+Elements.back() = llvm::ConstantAggregateZero::get(FillerType);
+CommonElementType = nullptr;
+  } else if (Elements.size() != ArrayBound) {
+// Otherwise pad to the right size with the filler if necessary.
+Elements.resize(ArrayBound, Filler);
+if (Filler->getType() != CommonElementType)
+  CommonElementType = nullptr;
+  }
+
+  // If all elements have the same type, just emit an array constant.
+  if (CommonElementType)
+return llvm::ConstantArray::get(
+llvm::ArrayType::get(CommonElementType, ArrayBound), Elements);
+
+  // We have mixed types. Use a packed struct.
+  llvm::SmallVector Types;
+  Types.reserve(Elements.size());
+  for (llvm::Constant *Elt : Elements)
+Types.push_back(Elt->getType());
+  llvm::StructType *SType =
+  llvm::StructType::get(CGM.getLLVMContext(), Types, true);
+  return llvm::ConstantStruct::get(SType, Elements);
+}
+
 /// This class only needs to handle two cases:
 /// 1) Literals (this is used by APValue emission to emit literals).
 /// 2) Arrays, structs and unions (outside C++11 mode, we don't currently
@@ -832,68 +886,47 @@ public:
   }
 
   llvm::Constant *EmitArrayInitialization(InitListExpr *ILE, QualType T) {
-llvm::ArrayType *AType =
-cast(ConvertType(ILE->getType()));
-llvm::Type *ElemTy = AType->getElementType();
+auto *CAT = CGM.getContext().getAsConstantArrayType(ILE->getType());
+assert(CAT && "can't emit array init for non-constant-bound array");
 unsigned NumInitElements = ILE->getNumInits();
-unsigned NumElements = AType->getNumElements();
+unsigned NumElements = CAT->getSize().getZExtValue();
 
 // Initialising an array requires us to automatically
 // initialise any elements that have not been initialised explicitly
 unsigned NumInitableElts = std::min(NumInitElements, NumElements);
 
-QualType EltType = CGM.getContext().getAsArrayType(T)->getElementType();
+QualType EltType = CAT->getElementType();
 
 // Initialize remaining array elements.
-llvm::Constant *fillC;
-if (Expr *filler = ILE->getArrayFiller())
+llvm::Constant *fillC = nullptr;
+if (Expr *filler = ILE->getArrayFiller()) {
   fillC = Emitter.tryEmitAbstractForMemory(filler, EltType);
-else
-  fillC = Emitter.emitNullForMemory(EltType);
-if (!fillC)
-  return nullptr;
-
-// Try to use a ConstantAggregateZero if we can.
-if (fillC->isNullValue() && !NumInitableElts)
-  return llvm::ConstantAggregateZero::get(AType);
+  if (!fillC)
+return nullptr;
+}
 
 // Copy initializer elements.
 SmallVector Elts;
-Elts.reserve(std::max(NumInitableElts, NumElements));
+if 

[PATCH] D47157: Warning for framework headers using double quote includes

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno added a comment.

Hi Eugene,

> You could just not include new warning into -Wall and -Wextra. Those who will 
> want to check #include statements correctness could enable it explicitly.

This is exactly what's happening in the patch, the new warning isn't part of 
`-Wall` or `-Wextra`, and is marked `DefaultIgnore`, which means that will fire 
only when `-Wquoted-include-in-framework-header` is passed to the driver. Am I 
missing something from your explanation?

Thanks,


Repository:
  rC Clang

https://reviews.llvm.org/D47157



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

[PATCH] D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable.

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

rsmith added a comment.

In https://reviews.llvm.org/D45015#1109049, @ahatanak wrote:

> - Currently clang errors out when aligned operator new is selected but the 
> OS's version is too old to support it. What's the reason we want to change 
> this now to be a warning rather than an error?


I think it's fine to leave it as a `DefaultError` warning. I think it'd also be 
fine to change it to simply be an error (rather than an 
error-with-warning-flag) and remove the error recovery, and that actually seems 
a bit better: then our behavior under -faligned-alloc-unavailable would be 
equivalent to treating the implicit aligned forms of operator new/delete as if 
they had an availability attribute on them by default, which I think seems very 
reasonable.

> - So clang no longer needs to define macro 
> `__ALIGNED_ALLOCATION_UNAVAILABLE__` and libc++ will use `__cpp_aligned_new` 
> (I think you meant `__cpp_aligned_new`, not `__cpp_aligned_allocation`?) to 
> determine whether aligned allocation functions should be defined or made 
> available in the header?

Yes, that's the idea. `__cpp_aligned_new` exists to allow code to conditionally 
use aligned new if it's available, and we should do our best to honor that.


Repository:
  rC Clang

https://reviews.llvm.org/D45015



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

[PATCH] D47301: Warning for framework include violation from Headers to PrivateHeaders

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno updated this revision to Diff 148317.
bruno added a comment.

Update to a more recent version of the patch.


https://reviews.llvm.org/D47301

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticLexKinds.td
  lib/Lex/HeaderSearch.cpp
  test/Modules/Inputs/framework-public-includes-private/A.framework/Headers/A.h
  
test/Modules/Inputs/framework-public-includes-private/A.framework/Modules/module.modulemap
  
test/Modules/Inputs/framework-public-includes-private/A.framework/Modules/module.private.modulemap
  
test/Modules/Inputs/framework-public-includes-private/A.framework/PrivateHeaders/APriv.h
  
test/Modules/Inputs/framework-public-includes-private/A.framework/PrivateHeaders/APriv2.h
  test/Modules/Inputs/framework-public-includes-private/a.hmap.json
  test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.h
  
test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.modulemap
  
test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.private.modulemap
  test/Modules/Inputs/framework-public-includes-private/flat-header-path/ZPriv.h
  test/Modules/Inputs/framework-public-includes-private/z.hmap.json
  test/Modules/Inputs/framework-public-includes-private/z.yaml
  test/Modules/framework-public-includes-private.m

Index: test/Modules/framework-public-includes-private.m
===
--- /dev/null
+++ test/Modules/framework-public-includes-private.m
@@ -0,0 +1,37 @@
+// REQUIRES: shell
+
+// RUN: rm -rf %t
+// RUN: mkdir %t
+
+// RUN: hmaptool write %S/Inputs/framework-public-includes-private/a.hmap.json %t/a.hmap
+// RUN: hmaptool write %S/Inputs/framework-public-includes-private/z.hmap.json %t/z.hmap
+
+// RUN: sed -e "s:TEST_DIR:%S/Inputs/framework-public-includes-private:g" \
+// RUN:   %S/Inputs/framework-public-includes-private/z.yaml > %t/z.yaml
+
+// The output with and without modules should be the same, without modules first.
+// RUN: %clang_cc1 \
+// RUN:   -iquote %t/z.hmap -iquote %t/a.hmap -ivfsoverlay %t/z.yaml \
+// RUN:   -F%S/Inputs/framework-public-includes-private \
+// RUN:   -fsyntax-only %s -verify
+
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache \
+// RUN:   -iquote %t/z.hmap -iquote %t/a.hmap -ivfsoverlay %t/z.yaml \
+// RUN:   -F%S/Inputs/framework-public-includes-private \
+// RUN:   -fsyntax-only %s \
+// RUN:   2>%t/stderr
+
+// The same warnings show up when modules is on but -verify doesn't get it
+// because they only show up under the module A building context.
+// RUN: FileCheck --input-file=%t/stderr %s
+// CHECK: public framework header includes private framework header 'A/APriv.h'
+// CHECK: public framework header includes private framework header 'A/APriv2.h'
+// CHECK: public framework header includes private framework header 'Z/ZPriv.h'
+
+#import "A.h"
+
+int bar() { return foo(); }
+
+// expected-warning@Inputs/framework-public-includes-private/A.framework/Headers/A.h:2{{public framework header includes private framework header 'A/APriv.h'}}
+// expected-warning@Inputs/framework-public-includes-private/A.framework/Headers/A.h:3{{public framework header includes private framework header 'A/APriv2.h'}}
+// expected-warning@Inputs/framework-public-includes-private/flat-header-path/Z.h:1{{public framework header includes private framework header 'Z/ZPriv.h'}}
Index: test/Modules/Inputs/framework-public-includes-private/z.yaml
===
--- /dev/null
+++ test/Modules/Inputs/framework-public-includes-private/z.yaml
@@ -0,0 +1,45 @@
+{
+  'version': 0,
+  'case-sensitive': 'false',
+  'use-external-names' : 'false',
+  'roots': [
+{
+  'type': 'directory',
+  'name': "TEST_DIR/Z.framework/Headers",
+  'contents': [
+{
+  'type': 'file',
+  'name': "Z.h",
+  'external-contents': "TEST_DIR/flat-header-path/Z.h"
+}
+  ]
+},
+{
+  'type': 'directory',
+  'name': "TEST_DIR/Z.framework/PrivateHeaders",
+  'contents': [
+{
+  'type': 'file',
+  'name': "ZPriv.h",
+  'external-contents': "TEST_DIR/flat-header-path/ZPriv.h"
+}
+  ]
+},
+{
+  'type': 'directory',
+  'name': "TEST_DIR/Z.framework/Modules",
+  'contents': [
+{
+  'type': 'file',
+  'name': "module.modulemap",
+  'external-contents': "TEST_DIR/flat-header-path/Z.modulemap"
+},
+{
+  'type': 'file',
+  'name': "module.private.modulemap",
+  'external-contents': "TEST_DIR/flat-header-path/Z.private.modulemap"
+}
+  ]
+}
+  ]
+}
Index: test/Modules/Inputs/framework-public-includes-private/z.hmap.json
===
--- /dev/null
+++ test/Modules/Inputs/framework-public-includes-private/z.hmap.json
@@ -0,0 +1,7 

[PATCH] D47301: Warning for framework include violation from Headers to PrivateHeaders

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno created this revision.
bruno added reviewers: ributzka, vsapsai, dexonsmith.

Framework vendors usually layout their framework headers in the following way:

  Foo.framework/Headers -> "public" headers
  Foo.framework/PrivateHeader -> "private" headers

Since both headers in both directories can be found with #import 
, it's easy to make mistakes and include headers in 
Foo.framework/PrivateHeader from headers in Foo.framework/Headers, which 
usually configures a layering violation on Darwin ecosystems. One of the 
problem this causes is dep cycles when modules are used, since it's very common 
for "private" modules to include from the "public" ones; adding an edge the 
other way around will trigger cycles.

Add a warning to catch those cases such that:

In file included from test.m:1:
./A.framework/Headers/A.h:2:10: warning: public framework header includes 
private framework header 'A/APriv.h' [-Wframework-include-private-from-public]

  ^

This only works for the layering violation within a framework.

rdar://problem/38712182

Depends on https://reviews.llvm.org/D47157


Repository:
  rC Clang

https://reviews.llvm.org/D47301

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticLexKinds.td
  lib/Lex/HeaderSearch.cpp
  test/Modules/Inputs/framework-public-includes-private/A.framework/Headers/A.h
  
test/Modules/Inputs/framework-public-includes-private/A.framework/Modules/module.modulemap
  
test/Modules/Inputs/framework-public-includes-private/A.framework/Modules/module.private.modulemap
  
test/Modules/Inputs/framework-public-includes-private/A.framework/PrivateHeaders/APriv.h
  
test/Modules/Inputs/framework-public-includes-private/A.framework/PrivateHeaders/APriv2.h
  test/Modules/Inputs/framework-public-includes-private/a.hmap.json
  test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.h
  
test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.modulemap
  
test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.private.modulemap
  test/Modules/Inputs/framework-public-includes-private/flat-header-path/ZPriv.h
  test/Modules/Inputs/framework-public-includes-private/z.hmap.json
  test/Modules/Inputs/framework-public-includes-private/z.yaml
  test/Modules/framework-public-includes-private.m

Index: test/Modules/framework-public-includes-private.m
===
--- /dev/null
+++ test/Modules/framework-public-includes-private.m
@@ -0,0 +1,37 @@
+// REQUIRES: shell
+
+// RUN: rm -rf %t
+// RUN: mkdir %t
+
+// RUN: hmaptool write %S/Inputs/framework-public-includes-private/a.hmap.json %t/a.hmap
+// RUN: hmaptool write %S/Inputs/framework-public-includes-private/z.hmap.json %t/z.hmap
+
+// RUN: sed -e "s:TEST_DIR:%S/Inputs/framework-public-includes-private:g" \
+// RUN:   %S/Inputs/framework-public-includes-private/z.yaml > %t/z.yaml
+
+// The output with and without modules should be the same, without modules first.
+// RUN: %clang_cc1 \
+// RUN:   -iquote %t/z.hmap -iquote %t/a.hmap -ivfsoverlay %t/z.yaml \
+// RUN:   -F%S/Inputs/framework-public-includes-private \
+// RUN:   -fsyntax-only %s -verify
+
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache \
+// RUN:   -iquote %t/z.hmap -iquote %t/a.hmap -ivfsoverlay %t/z.yaml \
+// RUN:   -F%S/Inputs/framework-public-includes-private \
+// RUN:   -fsyntax-only %s \
+// RUN:   2>%t/stderr
+
+// The same warnings show up when modules is on but -verify doesn't get it
+// because they only show up under the module A building context.
+// RUN: FileCheck --input-file=%t/stderr %s
+// CHECK: public framework header includes private framework header 'A/APriv.h'
+// CHECK: public framework header includes private framework header 'A/APriv2.h'
+// CHECK: public framework header includes private framework header 'Z/ZPriv.h'
+
+#import "A.h"
+
+int bar() { return foo(); }
+
+// expected-warning@Inputs/framework-public-includes-private/A.framework/Headers/A.h:2{{public framework header includes private framework header 'A/APriv.h'}}
+// expected-warning@Inputs/framework-public-includes-private/A.framework/Headers/A.h:3{{public framework header includes private framework header 'A/APriv2.h'}}
+// expected-warning@Inputs/framework-public-includes-private/flat-header-path/Z.h:1{{public framework header includes private framework header 'Z/ZPriv.h'}}
Index: test/Modules/Inputs/framework-public-includes-private/z.yaml
===
--- /dev/null
+++ test/Modules/Inputs/framework-public-includes-private/z.yaml
@@ -0,0 +1,45 @@
+{
+  'version': 0,
+  'case-sensitive': 'false',
+  'use-external-names' : 'false',
+  'roots': [
+{
+  'type': 'directory',
+  'name': "TEST_DIR/Z.framework/Headers",
+  'contents': [
+{
+  'type': 'file',
+  'name': "Z.h",
+  'external-contents': "TEST_DIR/flat-header-path/Z.h"

[PATCH] D45897: Convert clang-interpreter to ORC JIT API

[Lang Hames via Phabricator via cfe-commits]

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

Looks good to me. :)


Repository:
  rC Clang

https://reviews.llvm.org/D45897



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

[PATCH] D47103: Implement strip.invariant.group

[Piotr Padlewski via Phabricator via cfe-commits]

Prazek updated this revision to Diff 148313.
Prazek marked 2 inline comments as done.
Prazek added a comment.

Slitted commit into defining and using intrinsic


Repository:
  rL LLVM

https://reviews.llvm.org/D47103

Files:
  llvm/docs/LangRef.rst
  llvm/include/llvm/IR/IRBuilder.h
  llvm/include/llvm/IR/Intrinsics.td
  llvm/lib/Analysis/BasicAliasAnalysis.cpp
  llvm/lib/Analysis/ConstantFolding.cpp
  llvm/lib/Analysis/ValueTracking.cpp
  llvm/lib/CodeGen/CodeGenPrepare.cpp
  llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/IR/Value.cpp
  llvm/lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp
  llvm/test/Analysis/ValueTracking/invariant.group.ll
  llvm/test/CodeGen/Generic/intrinsics.ll
  llvm/test/Other/invariant.group.ll
  llvm/test/Other/launder.invariant.group.ll
  llvm/test/Transforms/CodeGenPrepare/invariant.group.ll
  llvm/test/Transforms/DeadStoreElimination/launder.invariant.group.ll
  llvm/test/Transforms/FunctionAttrs/nocapture.ll
  llvm/test/Transforms/GVN/invariant.group.ll
  llvm/test/Transforms/GlobalOpt/invariant.group.barrier.ll
  llvm/test/Transforms/GlobalOpt/invariant.group.ll
  llvm/test/Transforms/InstCombine/invariant.group.ll
  llvm/test/Transforms/NewGVN/invariant.group.ll

Index: llvm/test/Transforms/NewGVN/invariant.group.ll
===
--- llvm/test/Transforms/NewGVN/invariant.group.ll
+++ llvm/test/Transforms/NewGVN/invariant.group.ll
@@ -52,6 +52,19 @@
 ret i8 %b
 }
 
+; CHECK-LABEL: define i1 @proveEqualityForStrip(
+define i1 @proveEqualityForStrip(i8* %a) {
+; FIXME: The first call could be also removed by GVN. Right now
+; DCE removes it. The second call is CSE'd with the first one.
+; CHECK: %b1 = call i8* @llvm.strip.invariant.group.p0i8(i8* %a)
+  %b1 = call i8* @llvm.strip.invariant.group.p0i8(i8* %a)
+; CHECK-NOT: llvm.strip.invariant.group
+  %b2 = call i8* @llvm.strip.invariant.group.p0i8(i8* %a)
+  %r = icmp eq i8* %b1, %b2
+; CHECK: ret i1 true
+  ret i1 %r
+}
+
 ; CHECK-LABEL: define i8 @unoptimizable1() {
 define i8 @unoptimizable1() {
 entry:
Index: llvm/test/Transforms/InstCombine/invariant.group.ll
===
--- llvm/test/Transforms/InstCombine/invariant.group.ll
+++ llvm/test/Transforms/InstCombine/invariant.group.ll
@@ -1,5 +1,6 @@
 ; RUN: opt -instcombine -S < %s | FileCheck %s
 
+
 ; CHECK-LABEL: define i8* @simplifyNullLaunder()
 define i8* @simplifyNullLaunder() {
 ; CHECK-NEXT: ret i8* null
@@ -29,6 +30,39 @@
   ret i8 addrspace(42)* %b2
 }
 
-
 declare i8* @llvm.launder.invariant.group.p0i8(i8*)
 declare i8 addrspace(42)* @llvm.launder.invariant.group.p42i8(i8 addrspace(42)*)
+
+
+; CHECK-LABEL: define i8* @simplifyNullStrip()
+define i8* @simplifyNullStrip() {
+; CHECK-NEXT: ret i8* null
+  %b2 = call i8* @llvm.strip.invariant.group.p0i8(i8* null)
+  ret i8* %b2
+}
+
+; CHECK-LABEL: define i8 addrspace(42)* @dontsimplifyNullStripForDifferentAddrspace()
+define i8 addrspace(42)* @dontsimplifyNullStripForDifferentAddrspace() {
+; CHECK: %b2 = call i8 addrspace(42)* @llvm.strip.invariant.group.p42i8(i8 addrspace(42)* null)
+; CHECK: ret i8 addrspace(42)* %b2
+  %b2 = call i8 addrspace(42)* @llvm.strip.invariant.group.p42i8(i8 addrspace(42)* null)
+  ret i8 addrspace(42)* %b2
+}
+
+; CHECK-LABEL: define i8* @simplifyUndefStrip()
+define i8* @simplifyUndefStrip() {
+; CHECK-NEXT: ret i8* undef
+  %b2 = call i8* @llvm.strip.invariant.group.p0i8(i8* undef)
+  ret i8* %b2
+}
+
+; CHECK-LABEL: define i8 addrspace(42)* @simplifyUndefStrip2()
+define i8 addrspace(42)* @simplifyUndefStrip2() {
+; CHECK-NEXT: ret i8 addrspace(42)* undef
+  %b2 = call i8 addrspace(42)* @llvm.strip.invariant.group.p42i8(i8 addrspace(42)* undef)
+  ret i8 addrspace(42)* %b2
+}
+
+declare i8* @llvm.strip.invariant.group.p0i8(i8*)
+declare i8 addrspace(42)* @llvm.strip.invariant.group.p42i8(i8 addrspace(42)*)
+
Index: llvm/test/Transforms/GlobalOpt/invariant.group.ll
===
--- llvm/test/Transforms/GlobalOpt/invariant.group.ll
+++ llvm/test/Transforms/GlobalOpt/invariant.group.ll
@@ -27,46 +27,46 @@
 define void @_optimizable() {
 enter:
   %valptr = alloca i32
-  
+
   %val = call i32 @TheAnswerToLifeTheUniverseAndEverything()
   store i32 %val, i32* @tmp
   store i32 %val, i32* %valptr
-  
+
   %0 = bitcast i32* %valptr to i8*
   %barr = call i8* @llvm.launder.invariant.group(i8* %0)
   %1 = bitcast i8* %barr to i32*
-  
+
   %val2 = load i32, i32* %1
   store i32 %val2, i32* @tmp2
   ret void
 }
 
 ; We can't step through launder.invariant.group here, because that would change
 ; this load in @usage_of_globals()
-; val = load i32, i32* %ptrVal, !invariant.group !0 
-; into 
+; val = load i32, i32* %ptrVal, !invariant.group !0
+; into
 ; %val = load i32, i32* @tmp3, !invariant.group !0
-; and then we could assume that %val and %val2 to be the same, 

[PATCH] D47103: Implement strip.invariant.group

[Piotr Padlewski via Phabricator via cfe-commits]

Prazek added inline comments.



Comment at: clang/lib/CodeGen/CGExpr.cpp:3858
+}
+  }
+

rjmccall wrote:
> Please add a comment explaining why this is necessary.  (I'm actually not 
> sure why it is, because surely the invariant groups we generate don't contain 
> assumptions about memory from fields, right?)
Short answer: you can only make virtual calls on a dynamic pointer that carries 
invariant.group and you can't do anything other because it could leak the 
information about this pointer (which when used with comparison could break 
devirtualization). 


Repository:
  rL LLVM

https://reviews.llvm.org/D47103



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

[PATCH] D47299: [CodeGenCXX] Emit strip.invariant.group with -fstrict-vtable-pointers

[Piotr Padlewski via Phabricator via cfe-commits]

Prazek created this revision.
Prazek added reviewers: rjmccall, rsmith, amharc, kuhar.
Herald added a subscriber: llvm-commits.

Emmiting new intrinsic that strips invariant.groups to make 
devirtulization sound, as described in RFC: Devirtualization v2.


Repository:
  rL LLVM

https://reviews.llvm.org/D47299

Files:
  clang/include/clang/AST/DeclCXX.h
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/test/CodeGenCXX/strict-vtable-pointers.cpp

Index: clang/test/CodeGenCXX/strict-vtable-pointers.cpp
===
--- clang/test/CodeGenCXX/strict-vtable-pointers.cpp
+++ clang/test/CodeGenCXX/strict-vtable-pointers.cpp
@@ -5,7 +5,8 @@
 // RUN: FileCheck --check-prefix=CHECK-LINK-REQ %s < %t.ll
 
 typedef __typeof__(sizeof(0)) size_t;
-void *operator new(size_t, void*) throw();
+void *operator new(size_t, void *) throw();
+using uintptr_t = unsigned long long;
 
 struct NotTrivialDtor {
   ~NotTrivialDtor();
@@ -17,7 +18,7 @@
 };
 
 struct DynamicDerived : DynamicBase1 {
-  void foo();
+  void foo() override;
 };
 
 struct DynamicBase2 {
@@ -28,8 +29,8 @@
 };
 
 struct DynamicDerivedMultiple : DynamicBase1, DynamicBase2 {
-  virtual void foo();
-  virtual void bar();
+  void foo() override;
+  void bar() override;
 };
 
 struct StaticBase {
@@ -47,9 +48,8 @@
 struct DynamicFromVirtualStatic2 : virtual StaticBase {
 };
 
-struct DynamicFrom2Virtuals :
-DynamicFromVirtualStatic1,
-DynamicFromVirtualStatic2 {
+struct DynamicFrom2Virtuals : DynamicFromVirtualStatic1,
+  DynamicFromVirtualStatic2 {
 };
 
 // CHECK-NEW-LABEL: define void @_Z12LocalObjectsv()
@@ -89,7 +89,6 @@
 // CHECK-CTORS: call i8* @llvm.launder.invariant.group.p0i8(
 // CHECK-CTORS-LABEL: {{^}}}
 
-
 // CHECK-NEW-LABEL: define void @_Z9Pointers1v()
 // CHECK-NEW-NOT: @llvm.launder.invariant.group.p0i8(
 // CHECK-NEW-LABEL: call void @_ZN12DynamicBase1C1Ev(
@@ -134,7 +133,6 @@
 // CHECK-CTORS-NOT: call i8* @llvm.launder.invariant.group.p0i8(
 // CHECK-CTORS-LABEL: {{^}}}
 
-
 struct DynamicDerived;
 
 // CHECK-CTORS-LABEL: define linkonce_odr void @_ZN14DynamicDerivedC2Ev(
@@ -164,14 +162,12 @@
 // CHECK-CTORS: call void @_ZN12DynamicBase2C2Ev(
 // CHECK-CTORS-NOT: @llvm.launder.invariant.group.p0i8
 
-
 // CHECK-CTORS: %[[THIS10:.*]] = bitcast %struct.DynamicDerivedMultiple* %[[THIS0]] to i32 (...)***
 // CHECK-CTORS: store {{.*}} @_ZTV22DynamicDerivedMultiple, i32 0, inrange i32 0, i32 2) {{.*}} %[[THIS10]]
 // CHECK-CTORS: %[[THIS11:.*]] = bitcast %struct.DynamicDerivedMultiple* %[[THIS0]] to i8*
 // CHECK-CTORS: %[[THIS_ADD:.*]] = getelementptr inbounds i8, i8* %[[THIS11]], i64 16
 // CHECK-CTORS: %[[THIS12:.*]]  = bitcast i8* %[[THIS_ADD]] to i32 (...)***
 
-
 // CHECK-CTORS: store {{.*}} @_ZTV22DynamicDerivedMultiple, i32 0, inrange i32 1, i32 2) {{.*}} %[[THIS12]]
 // CHECK-CTORS-LABEL: {{^}}}
 
@@ -182,9 +178,10 @@
 
 struct A {
   virtual void foo();
+  int m;
 };
 struct B : A {
-  virtual void foo();
+  void foo() override;
 };
 
 union U {
@@ -209,7 +206,7 @@
   // CHECK-NEW: call i8* @llvm.launder.invariant.group.p0i8(i8*
   // CHECK-NEW: call void @_Z2g2P1A(%struct.A*
   g2(>b);
-  // CHECK-NEW: call void @_Z9changeToAP1U(%union.U* 
+  // CHECK-NEW: call void @_Z9changeToAP1U(%union.U*
   changeToA(u);
   // CHECK-NEW: call i8* @llvm.launder.invariant.group.p0i8(i8*
   // call void @_Z2g2P1A(%struct.A* %a)
@@ -294,21 +291,206 @@
   take(u.v3);
 }
 
+// CHECK-NEW-LABEL: define void @_Z7comparev()
+void compare() {
+  A *a = new A;
+  a->foo();
+  // CHECK-NEW: call i8* @llvm.launder.invariant.group.p0i8(i8*
+  A *b = new (a) B;
+
+  // CHECK-NEW: %[[a:.*]] = call i8* @llvm.strip.invariant.group.p0i8(i8*
+  // CHECK-NEW: %[[a2:.*]] = bitcast i8* %[[a]] to %struct.A*
+  // CHECK-NEW: %[[b:.*]] = call i8* @llvm.strip.invariant.group.p0i8(i8*
+  // CHECK-NEW: %[[b2:.*]] = bitcast i8* %[[b]] to %struct.A*
+  // CHECK-NEW: %cmp = icmp eq %struct.A* %[[a2]], %[[b2]]
+  if (a == b)
+b->foo();
+}
+
+// CHECK-NEW-LABEL: compare2
+bool compare2(A *a, A *a2) {
+  // CHECK-NEW: %[[a:.*]] = call i8* @llvm.strip.invariant.group.p0i8(i8*
+  // CHECK-NEW: %[[a2:.*]] = bitcast i8* %[[a]] to %struct.A*
+  // CHECK-NEW: %[[b:.*]] = call i8* @llvm.strip.invariant.group.p0i8(i8*
+  // CHECK-NEW: %[[b2:.*]] = bitcast i8* %[[b]] to %struct.A*
+  // CHECK-NEW: %cmp = icmp ult %struct.A* %[[a2]], %[[b2]]
+  return a < a2;
+}
+// CHECK-NEW-LABEL: compareIntPointers
+bool compareIntPointers(int *a, int *b) {
+  // CHECK-NEW-NOT: call i8* @llvm.strip.invariant.group
+  return a == b;
+}
+
+struct HoldingOtherVirtuals {
+  B b;
+};
+
+// There is no need to add barriers for comparision of pointer to classes
+// that are not dynamic.
+// CHECK-NEW-LABEL: compare5
+bool compare5(HoldingOtherVirtuals *a, HoldingOtherVirtuals *b) {
+  // CHECK-NEW-NOT: call i8* @llvm.strip.invariant.group
+  return a == b;
+}
+// CHECK-NEW-LABEL: 

[PATCH] D47297: [Modules][ObjC] Add protocol redefinition to the current scope/context

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno created this revision.
bruno added a reviewer: rsmith.

Not doing so causes the AST writter to assert since the decl in question never 
gets emitted. This is fine when modules is not used, but otherwise we need to 
serialize something other than garbage.

rdar://problem/39844933


Repository:
  rC Clang

https://reviews.llvm.org/D47297

Files:
  lib/Sema/SemaDeclObjC.cpp
  test/Modules/Inputs/protocol-redefinition/Base.framework/Headers/Base.h
  
test/Modules/Inputs/protocol-redefinition/Base.framework/Modules/module.modulemap
  test/Modules/Inputs/protocol-redefinition/Kit.framework/Headers/Kit.h
  
test/Modules/Inputs/protocol-redefinition/Kit.framework/Modules/module.modulemap
  test/Modules/protocol-redefinition.m


Index: test/Modules/protocol-redefinition.m
===
--- /dev/null
+++ test/Modules/protocol-redefinition.m
@@ -0,0 +1,6 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t 
-F%S/Inputs/protocol-redefinition -fsyntax-only %s -Wno-private-module -verify
+
+// expected-no-diagnostics
+
+@import Kit;
Index: 
test/Modules/Inputs/protocol-redefinition/Kit.framework/Modules/module.modulemap
===
--- /dev/null
+++ 
test/Modules/Inputs/protocol-redefinition/Kit.framework/Modules/module.modulemap
@@ -0,0 +1,4 @@
+framework module Kit {
+  header "Kit.h"
+  export *
+}
\ No newline at end of file
Index: test/Modules/Inputs/protocol-redefinition/Kit.framework/Headers/Kit.h
===
--- /dev/null
+++ test/Modules/Inputs/protocol-redefinition/Kit.framework/Headers/Kit.h
@@ -0,0 +1,6 @@
+#import 
+
+// REDECLARATION
+@protocol Foo
+- (void)someMethodOnFoo;
+@end
Index: 
test/Modules/Inputs/protocol-redefinition/Base.framework/Modules/module.modulemap
===
--- /dev/null
+++ 
test/Modules/Inputs/protocol-redefinition/Base.framework/Modules/module.modulemap
@@ -0,0 +1,4 @@
+framework module Base {
+  header "Base.h"
+  export *
+}
\ No newline at end of file
Index: test/Modules/Inputs/protocol-redefinition/Base.framework/Headers/Base.h
===
--- /dev/null
+++ test/Modules/Inputs/protocol-redefinition/Base.framework/Headers/Base.h
@@ -0,0 +1,3 @@
+@protocol Foo
+- (void)someMethodOnFoo;
+@end
Index: lib/Sema/SemaDeclObjC.cpp
===
--- lib/Sema/SemaDeclObjC.cpp
+++ lib/Sema/SemaDeclObjC.cpp
@@ -1202,6 +1202,11 @@
 PDecl = ObjCProtocolDecl::Create(Context, CurContext, ProtocolName,
  ProtocolLoc, AtProtoInterfaceLoc,
  /*PrevDecl=*/nullptr);
+
+// If we are using modules, add the decl to the context in order to
+// serialize something meaningful.
+if (getLangOpts().Modules)
+  PushOnScopeChains(PDecl, TUScope);
 PDecl->startDefinition();
   } else {
 if (PrevDecl) {


Index: test/Modules/protocol-redefinition.m
===
--- /dev/null
+++ test/Modules/protocol-redefinition.m
@@ -0,0 +1,6 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -F%S/Inputs/protocol-redefinition -fsyntax-only %s -Wno-private-module -verify
+
+// expected-no-diagnostics
+
+@import Kit;
Index: test/Modules/Inputs/protocol-redefinition/Kit.framework/Modules/module.modulemap
===
--- /dev/null
+++ test/Modules/Inputs/protocol-redefinition/Kit.framework/Modules/module.modulemap
@@ -0,0 +1,4 @@
+framework module Kit {
+  header "Kit.h"
+  export *
+}
\ No newline at end of file
Index: test/Modules/Inputs/protocol-redefinition/Kit.framework/Headers/Kit.h
===
--- /dev/null
+++ test/Modules/Inputs/protocol-redefinition/Kit.framework/Headers/Kit.h
@@ -0,0 +1,6 @@
+#import 
+
+// REDECLARATION
+@protocol Foo
+- (void)someMethodOnFoo;
+@end
Index: test/Modules/Inputs/protocol-redefinition/Base.framework/Modules/module.modulemap
===
--- /dev/null
+++ test/Modules/Inputs/protocol-redefinition/Base.framework/Modules/module.modulemap
@@ -0,0 +1,4 @@
+framework module Base {
+  header "Base.h"
+  export *
+}
\ No newline at end of file
Index: test/Modules/Inputs/protocol-redefinition/Base.framework/Headers/Base.h
===
--- /dev/null
+++ test/Modules/Inputs/protocol-redefinition/Base.framework/Headers/Base.h
@@ -0,0 +1,3 @@
+@protocol Foo
+- (void)someMethodOnFoo;
+@end
Index: lib/Sema/SemaDeclObjC.cpp
===
--- 

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

[JF Bastien via Phabricator via cfe-commits]

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



Comment at: test/FixIt/fixit-format-ios-nopedantic.m:21
+  printf("test 4: %zd %zd", getNSInteger(), getNSInteger());
+}

alexshap wrote:
> maybe i'm missing smth, but i don't see any verification in this test.
`-Werror` makes the test fail if something is reported.


Repository:
  rC Clang

https://reviews.llvm.org/D47290



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

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

[Alexander Shaposhnikov via Phabricator via cfe-commits]

alexshap added inline comments.



Comment at: test/FixIt/fixit-format-ios-nopedantic.m:21
+  printf("test 4: %zd %zd", getNSInteger(), getNSInteger());
+}

maybe i'm missing smth, but i don't see any verification in this test.


Repository:
  rC Clang

https://reviews.llvm.org/D47290



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

[PATCH] D47291: Proposal to make rtti errors more generic.

[Sunil Srivastava via Phabricator via cfe-commits]

Sunil_Srivastava created this revision.
Herald added a subscriber: cfe-commits.

This patch changes the wording of two errors to make them more generic.

An attempt to use dynamic_cast while rtti is disabled, curently emits the error:

  cannot use dynamic_cast with -fno-rtti

and a similar one for typeid.

This patch proposes to change that to:

  use of dynamic_cast requires enabling RTTI

For targets where the default mode of RTTI is disabled, the current error
message is confusing because the user never used the -fno-rtti option.

This proposal is motivated by the PS4 compiler, whose default is to have RTTI
disabled. However, it is just as clear as the existing diagnostic, and it
may be applicable to other llvm compilers having the have the same default as 
the
PS4 compiler. It is also more appropriate in cases where the spelling of the
argument (to disable RTTI) is something other than -fno-rtti (for
example, /GR- is the switch to disable RTTI for cl).


Repository:
  rC Clang

https://reviews.llvm.org/D47291

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  test/SemaCXX/no-rtti.cpp


Index: test/SemaCXX/no-rtti.cpp
===
--- test/SemaCXX/no-rtti.cpp
+++ test/SemaCXX/no-rtti.cpp
@@ -6,7 +6,7 @@
 
 void f()
 {
-  (void)typeid(int); // expected-error {{cannot use typeid with -fno-rtti}}
+  (void)typeid(int); // expected-error {{use of typeid requires enabling RTTI}}
 }
 
 namespace {
@@ -20,7 +20,7 @@
 }
 
 bool isa_B(A *a) {
-  return dynamic_cast(a) != 0; // expected-error {{cannot use 
dynamic_cast with -fno-rtti}}
+  return dynamic_cast(a) != 0; // expected-error {{use of dynamic_cast 
requires enabling RTTI}}
 }
 
 void* getMostDerived(A* a) {
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -6724,9 +6724,9 @@
   "no %select{struct|interface|union|class|enum}0 named %1 in %2">;
 
 def err_no_typeid_with_fno_rtti : Error<
-  "cannot use typeid with -fno-rtti">;
+  "use of typeid requires enabling RTTI">;
 def err_no_dynamic_cast_with_fno_rtti : Error<
-  "cannot use dynamic_cast with -fno-rtti">;
+  "use of dynamic_cast requires enabling RTTI">;
 
 def err_cannot_form_pointer_to_member_of_reference_type : Error<
   "cannot form a pointer-to-member to member %0 of reference type %1">;


Index: test/SemaCXX/no-rtti.cpp
===
--- test/SemaCXX/no-rtti.cpp
+++ test/SemaCXX/no-rtti.cpp
@@ -6,7 +6,7 @@
 
 void f()
 {
-  (void)typeid(int); // expected-error {{cannot use typeid with -fno-rtti}}
+  (void)typeid(int); // expected-error {{use of typeid requires enabling RTTI}}
 }
 
 namespace {
@@ -20,7 +20,7 @@
 }
 
 bool isa_B(A *a) {
-  return dynamic_cast(a) != 0; // expected-error {{cannot use dynamic_cast with -fno-rtti}}
+  return dynamic_cast(a) != 0; // expected-error {{use of dynamic_cast requires enabling RTTI}}
 }
 
 void* getMostDerived(A* a) {
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -6724,9 +6724,9 @@
   "no %select{struct|interface|union|class|enum}0 named %1 in %2">;
 
 def err_no_typeid_with_fno_rtti : Error<
-  "cannot use typeid with -fno-rtti">;
+  "use of typeid requires enabling RTTI">;
 def err_no_dynamic_cast_with_fno_rtti : Error<
-  "cannot use dynamic_cast with -fno-rtti">;
+  "use of dynamic_cast requires enabling RTTI">;
 
 def err_cannot_form_pointer_to_member_of_reference_type : Error<
   "cannot form a pointer-to-member to member %0 of reference type %1">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

[JF Bastien via Phabricator via cfe-commits]

jfb created this revision.
jfb added reviewers: ahatanak, vsapsai, alexshap, aaron.ballman, javed.absar, 
jfb, rjmccall.
Herald added subscribers: cfe-commits, aheejin, kristof.beyls.

Pick https://reviews.llvm.org/D42933 back up, and make NSInteger/NSUInteger 
with %zu/%zi specifiers on Darwin warn only in pedantic mode. The default 
-Wformat recently started warning for the following code because of the added 
support for analysis for the '%zi' specifier.

  NSInteger i = NSIntegerMax;
  NSLog(@"max NSInteger = %zi", i);

The problem is that on armv7 %zi is 'long', and NSInteger is typedefed to 'int' 
in Foundation. We should avoid this warning as it's inconvenient to our users: 
it's target specific (happens only on armv7 and not arm64), and breaks their 
existing code. We should also silence the warning for the '%zu' specifier to 
ensure consistency. This is acceptable because Darwin guarantees that, despite 
the unfortunate choice of typedef, sizeof(size_t) == sizeof(NS[U]Integer), the 
warning is therefore noisy for pedantic reasons. Once this is in I'll update 
public documentation.

Related discussion on cfe-dev:
http://lists.llvm.org/pipermail/cfe-dev/2018-May/058050.html

rdar://36874921


Repository:
  rC Clang

https://reviews.llvm.org/D47290

Files:
  include/clang/Analysis/Analyses/FormatString.h
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Analysis/PrintfFormatString.cpp
  lib/Sema/SemaChecking.cpp
  test/FixIt/fixit-format-ios-nopedantic.m
  test/FixIt/fixit-format-ios.m
  test/SemaObjC/format-size-spec-nsinteger-nopedantic.m
  test/SemaObjC/format-size-spec-nsinteger.m

Index: test/SemaObjC/format-size-spec-nsinteger.m
===
--- /dev/null
+++ test/SemaObjC/format-size-spec-nsinteger.m
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -triple thumbv7-apple-ios -Wformat-pedantic -fsyntax-only -Wno-objc-root-class %s 2>&1 | FileCheck %s
+
+#if __LP64__
+typedef unsigned long NSUInteger;
+typedef long NSInteger;
+#else
+typedef unsigned int NSUInteger;
+typedef int NSInteger;
+#endif
+
+@class NSString;
+
+extern void NSLog(NSString *format, ...);
+
+void testSizeSpecifier() {
+  NSInteger i = 0;
+  NSUInteger j = 0;
+  NSLog(@"max NSInteger = %zi", i);  // CHECK: values of type 'NSInteger' should not be used as format arguments; add an explicit cast to 'long' instead
+  NSLog(@"max NSUinteger = %zu", j); // CHECK: values of type 'NSUInteger' should not be used as format arguments; add an explicit cast to 'unsigned long' instead
+}
Index: test/SemaObjC/format-size-spec-nsinteger-nopedantic.m
===
--- /dev/null
+++ test/SemaObjC/format-size-spec-nsinteger-nopedantic.m
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -triple thumbv7-apple-ios -Wformat -fsyntax-only -fblocks -verify -Wno-objc-root-class %s
+// RUN: %clang_cc1 -triple arm64-apple-ios -Wformat -fsyntax-only -fblocks -verify -Wno-objc-root-class %s
+// expected-no-diagnostics
+
+#if __LP64__
+typedef unsigned long NSUInteger;
+typedef long NSInteger;
+#else
+typedef unsigned int NSUInteger;
+typedef int NSInteger;
+#endif
+
+@class NSString;
+
+extern void NSLog(NSString *format, ...);
+
+void testSizeSpecifier() {
+  NSInteger i = 0;
+  NSUInteger j = 0;
+  NSLog(@"max NSInteger = %zi", i);
+  NSLog(@"max NSUinteger = %zu", j);
+}
Index: test/FixIt/fixit-format-ios.m
===
--- test/FixIt/fixit-format-ios.m
+++ test/FixIt/fixit-format-ios.m
@@ -1,5 +1,5 @@
 // RUN: cp %s %t
-// RUN: %clang_cc1 -triple thumbv7-apple-ios8.0.0 -fsyntax-only -Wformat -fixit %t
+// RUN: %clang_cc1 -triple thumbv7-apple-ios8.0.0 -fsyntax-only -Wformat-pedantic -fixit %t
 // RUN: grep -v CHECK %t | FileCheck %s
 
 int printf(const char * restrict, ...);
Index: test/FixIt/fixit-format-ios-nopedantic.m
===
--- /dev/null
+++ test/FixIt/fixit-format-ios-nopedantic.m
@@ -0,0 +1,21 @@
+// RUN: cp %s %t
+// RUN: %clang_cc1 -triple thumbv7-apple-ios8.0.0 -fsyntax-only -Wformat -Werror -fixit %t
+
+int printf(const char *restrict, ...);
+typedef unsigned int NSUInteger;
+typedef int NSInteger;
+NSUInteger getNSUInteger();
+NSInteger getNSInteger();
+
+void test() {
+  // For thumbv7-apple-ios8.0.0 the underlying type of ssize_t is long
+  // and the underlying type of size_t is unsigned long.
+
+  printf("test 1: %zu", getNSUInteger());
+
+  printf("test 2: %zu %zu", getNSUInteger(), getNSUInteger());
+
+  printf("test 3: %zd", getNSInteger());
+
+  printf("test 4: %zd %zd", getNSInteger(), getNSInteger());
+}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -6594,11 +6594,11 @@
 ExprTy = TET->getUnderlyingExpr()->getType();
   }
 
-  analyze_printf::ArgType::MatchKind match = AT.matchesType(S.Context, ExprTy);
-
-  if 

[PATCH] D46971: [DWARF] Get RA from RA register even if it appears unused

[whitequark via Phabricator via cfe-commits]

whitequark added a comment.

@compnerd ping


Repository:
  rUNW libunwind

https://reviews.llvm.org/D46971



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

[PATCH] D46652: [clang-cl, PCH] Implement support for MS-style PCH through headers

[Mike Rice via Phabricator via cfe-commits]

mikerice added a comment.

Ping. Still looking for a reviewer, mostly Lex and clang-cl driver changes.


https://reviews.llvm.org/D46652



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

r333126 - Rework __builtin_classify_type support to better match GCC and to not assert on

[Richard Smith via cfe-commits]

Author: rsmith
Date: Wed May 23 14:18:00 2018
New Revision: 333126

URL: http://llvm.org/viewvc/llvm-project?rev=333126=rev
Log:
Rework __builtin_classify_type support to better match GCC and to not assert on
unusual types.

Following the observed behavior of GCC, we now return -1 for vector types
(along with all of our extensions that GCC doesn't support), and for atomic
types we classify the underlying type.

GCC appears to have changed its classification for function and array arguments
between version 5 and version 6. Previously it would classify them as pointers
in C and as functions or arrays in C++, but from version 6 onwards, it
classifies them as pointers. We now follow the more recent GCC behavior rather
than emulating what I can only assume to be a historical bug in their C++
support for this builtin.

Finally, no version of GCC that I can find has ever used the "method"
classification for C++ pointers to member functions. Instead, GCC classifies
them as record types, presumably reflecting an internal implementation detail,
but whatever the reason we now produce compatible results.

Modified:
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/test/Sema/builtin-classify-type.c
cfe/trunk/test/SemaCXX/builtin-classify-type.cpp

Modified: cfe/trunk/lib/AST/ExprConstant.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=333126=333125=333126=diff
==
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)
+++ cfe/trunk/lib/AST/ExprConstant.cpp Wed May 23 14:18:00 2018
@@ -7277,30 +7277,43 @@ bool IntExprEvaluator::CheckReferencedDe
   return false;
 }
 
+/// Values returned by __builtin_classify_type, chosen to match the values
+/// produced by GCC's builtin.
+enum class GCCTypeClass {
+  None = -1,
+  Void = 0,
+  Integer = 1,
+  // GCC reserves 2 for character types, but instead classifies them as
+  // integers.
+  Enum = 3,
+  Bool = 4,
+  Pointer = 5,
+  // GCC reserves 6 for references, but appears to never use it (because
+  // expressions never have reference type, presumably).
+  PointerToDataMember = 7,
+  RealFloat = 8,
+  Complex = 9,
+  // GCC reserves 10 for functions, but does not use it since GCC version 6 due
+  // to decay to pointer. (Prior to version 6 it was only used in C++ mode).
+  // GCC claims to reserve 11 for pointers to member functions, but *actually*
+  // uses 12 for that purpose, same as for a class or struct. Maybe it
+  // internally implements a pointer to member as a struct?  Who knows.
+  PointerToMemberFunction = 12, // Not a bug, see above.
+  ClassOrStruct = 12,
+  Union = 13,
+  // GCC reserves 14 for arrays, but does not use it since GCC version 6 due to
+  // decay to pointer. (Prior to version 6 it was only used in C++ mode).
+  // GCC reserves 15 for strings, but actually uses 5 (pointer) for string
+  // literals.
+};
+
 /// EvaluateBuiltinClassifyType - Evaluate __builtin_classify_type the same way
 /// as GCC.
-static int EvaluateBuiltinClassifyType(const CallExpr *E,
-   const LangOptions ) {
-  // The following enum mimics the values returned by GCC.
-  // FIXME: Does GCC differ between lvalue and rvalue references here?
-  enum gcc_type_class {
-no_type_class = -1,
-void_type_class, integer_type_class, char_type_class,
-enumeral_type_class, boolean_type_class,
-pointer_type_class, reference_type_class, offset_type_class,
-real_type_class, complex_type_class,
-function_type_class, method_type_class,
-record_type_class, union_type_class,
-array_type_class, string_type_class,
-lang_type_class
-  };
-
-  // If no argument was supplied, default to "no_type_class". This isn't
-  // ideal, however it is what gcc does.
-  if (E->getNumArgs() == 0)
-return no_type_class;
+static GCCTypeClass
+EvaluateBuiltinClassifyType(QualType T, const LangOptions ) {
+  assert(!T->isDependentType() && "unexpected dependent type");
 
-  QualType CanTy = E->getArg(0)->getType().getCanonicalType();
+  QualType CanTy = T.getCanonicalType();
   const BuiltinType *BT = dyn_cast(CanTy);
 
   switch (CanTy->getTypeClass()) {
@@ -7309,37 +7322,41 @@ static int EvaluateBuiltinClassifyType(c
 #define NON_CANONICAL_TYPE(ID, BASE) case Type::ID:
 #define NON_CANONICAL_UNLESS_DEPENDENT_TYPE(ID, BASE) case Type::ID:
 #include "clang/AST/TypeNodes.def"
-  llvm_unreachable("CallExpr::isBuiltinClassifyType(): unimplemented 
type");
+  case Type::Auto:
+  case Type::DeducedTemplateSpecialization:
+  llvm_unreachable("unexpected non-canonical or dependent type");
 
   case Type::Builtin:
 switch (BT->getKind()) {
 #define BUILTIN_TYPE(ID, SINGLETON_ID)
-#define SIGNED_TYPE(ID, SINGLETON_ID) case BuiltinType::ID: return 
integer_type_class;
-#define FLOATING_TYPE(ID, SINGLETON_ID) case BuiltinType::ID: return 
real_type_class;
-#define PLACEHOLDER_TYPE(ID, SINGLETON_ID) case BuiltinType::ID: break;

[PATCH] D47108: [CodeGenCXX] Add -fforce-emit-vtables

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In https://reviews.llvm.org/D47108#1109145, @Prazek wrote:

> In https://reviews.llvm.org/D47108#1109014, @rjmccall wrote:
>
> > I thought we already had places in Sema that marked inline virtual methods 
> > as used, instantiated templates, etc. for devirtualization purposes when 
> > optimization was enabled.  Did we rip that out?
>
>
> I only recall the emitting available_externally vtables opportunistically, 
> that is emitting it only if all the inline virtual functions are present (and 
> they are not hidden).
>  (https://reviews.llvm.org/D33437)
>
> > The problem we've had over and over with devirtualization is that we have 
> > to emit a perfect v-table because LLVM lacks a lot of the key vocabulary 
> > for talking about incomplete information.  For example, if something weird 
> > happens and we don't have a definition for an inline virtual method, 
> > ideally we'd just say "well, you can't devirtualize this slot", then try to 
> > fix that as incremental progress; but instead we have to get everything 
> > just right or else disable the whole optimization.  Note that vague-linkage 
> > v-tables mean that we'd also need to be able to say things like "there is 
> > an object with a definition that looks like this, but its symbol is not 
> > available and you can't emit it yourself".
>
> That is correcty, my intention was that this flag would cause all inline 
> virtual functions to be emitted. Can you give a hint how to achieve this in 
> the sane way?


Well, this paragraph was really a call for extending LLVM IR, which might be 
outside a reasonable scope for your summer project.

Building a v-table should itself cause inline functions to be emitted in IRGen 
as long as they've been properly marked for instantiation and use by Sema.  I 
don't know the right place to ensure that that happens.


Repository:
  rL LLVM

https://reviews.llvm.org/D47108



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

r333124 - [X86] Move the include of clzerointrin.h from immintrin.h back to x86intrin.h.

[Craig Topper via cfe-commits]

Author: ctopper
Date: Wed May 23 14:04:26 2018
New Revision: 333124

URL: http://llvm.org/viewvc/llvm-project?rev=333124=rev
Log:
[X86] Move the include of clzerointrin.h from immintrin.h back to x86intrin.h.

This is an AMD intrinsic not an Intel intrinsic so it shouldn't be in 
immintrin.h

Modified:
cfe/trunk/lib/Headers/immintrin.h
cfe/trunk/lib/Headers/x86intrin.h

Modified: cfe/trunk/lib/Headers/immintrin.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/immintrin.h?rev=333124=333123=333124=diff
==
--- cfe/trunk/lib/Headers/immintrin.h (original)
+++ cfe/trunk/lib/Headers/immintrin.h Wed May 23 14:04:26 2018
@@ -347,10 +347,6 @@ _writegsbase_u64(unsigned long long __V)
 #include 
 #endif
 
-#if !defined(_MSC_VER) || __has_feature(modules) || defined(__CLZERO__)
-#include 
-#endif
-
 #if !defined(_MSC_VER) || __has_feature(modules) || defined(__WBNOINVD__)
 #include 
 #endif

Modified: cfe/trunk/lib/Headers/x86intrin.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/x86intrin.h?rev=333124=333123=333124=diff
==
--- cfe/trunk/lib/Headers/x86intrin.h (original)
+++ cfe/trunk/lib/Headers/x86intrin.h Wed May 23 14:04:26 2018
@@ -60,4 +60,9 @@
 #include 
 #endif
 
+#if !defined(_MSC_VER) || __has_feature(modules) || defined(__CLZERO__)
+#include 
+#endif
+
+
 #endif /* __X86INTRIN_H */


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

[PATCH] D47277: [modules] Mark __wmmintrin_pclmul.h/__wmmintrin_aes.h as textual

[Raphael Isemann via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL333123: [modules] Mark 
__wmmintrin_pclmul.h/__wmmintrin_aes.h as textual (authored by teemperor, 
committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D47277?vs=148272=148281#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D47277

Files:
  cfe/trunk/lib/Headers/module.modulemap


Index: cfe/trunk/lib/Headers/module.modulemap
===
--- cfe/trunk/lib/Headers/module.modulemap
+++ cfe/trunk/lib/Headers/module.modulemap
@@ -71,6 +71,9 @@
 textual header "sgxintrin.h"
 textual header "ptwriteintrin.h"
 
+textual header "__wmmintrin_aes.h"
+textual header "__wmmintrin_pclmul.h"
+
 explicit module mm_malloc {
   requires !freestanding
   header "mm_malloc.h"
@@ -136,14 +139,6 @@
   export aes
   export pclmul
 }
-
-explicit module aes {
-  header "__wmmintrin_aes.h"
-}
-
-explicit module pclmul {
-  header "__wmmintrin_pclmul.h"
-}
   }
 
   explicit module systemz {


Index: cfe/trunk/lib/Headers/module.modulemap
===
--- cfe/trunk/lib/Headers/module.modulemap
+++ cfe/trunk/lib/Headers/module.modulemap
@@ -71,6 +71,9 @@
 textual header "sgxintrin.h"
 textual header "ptwriteintrin.h"
 
+textual header "__wmmintrin_aes.h"
+textual header "__wmmintrin_pclmul.h"
+
 explicit module mm_malloc {
   requires !freestanding
   header "mm_malloc.h"
@@ -136,14 +139,6 @@
   export aes
   export pclmul
 }
-
-explicit module aes {
-  header "__wmmintrin_aes.h"
-}
-
-explicit module pclmul {
-  header "__wmmintrin_pclmul.h"
-}
   }
 
   explicit module systemz {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r333123 - [modules] Mark __wmmintrin_pclmul.h/__wmmintrin_aes.h as textual

[Raphael Isemann via cfe-commits]

Author: teemperor
Date: Wed May 23 13:59:46 2018
New Revision: 333123

URL: http://llvm.org/viewvc/llvm-project?rev=333123=rev
Log:
[modules] Mark __wmmintrin_pclmul.h/__wmmintrin_aes.h as textual

Summary:
Since clang r332929 these two headers throw errors when included from somewhere 
else than their wrapper header. It seems marking them as textual is the best 
way to fix the builds.

Fixes this new module build error:
While building module '_Builtin_intrinsics' imported from ...:
In file included from :2:
In file included from lib/clang/7.0.0/include/immintrin.h:54:
In file included from lib/clang/7.0.0/include/wmmintrin.h:29:
lib/clang/7.0.0/include/__wmmintrin_aes.h:25:2: error: "Never use 
<__wmmintrin_aes.h> directly; include  instead."
#error "Never use <__wmmintrin_aes.h> directly; include  
instead."

Reviewers: rsmith, v.g.vassilev, craig.topper

Reviewed By: craig.topper

Subscribers: craig.topper, cfe-commits

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

Modified:
cfe/trunk/lib/Headers/module.modulemap

Modified: cfe/trunk/lib/Headers/module.modulemap
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/module.modulemap?rev=333123=333122=333123=diff
==
--- cfe/trunk/lib/Headers/module.modulemap (original)
+++ cfe/trunk/lib/Headers/module.modulemap Wed May 23 13:59:46 2018
@@ -71,6 +71,9 @@ module _Builtin_intrinsics [system] [ext
 textual header "sgxintrin.h"
 textual header "ptwriteintrin.h"
 
+textual header "__wmmintrin_aes.h"
+textual header "__wmmintrin_pclmul.h"
+
 explicit module mm_malloc {
   requires !freestanding
   header "mm_malloc.h"
@@ -136,14 +139,6 @@ module _Builtin_intrinsics [system] [ext
   export aes
   export pclmul
 }
-
-explicit module aes {
-  header "__wmmintrin_aes.h"
-}
-
-explicit module pclmul {
-  header "__wmmintrin_pclmul.h"
-}
   }
 
   explicit module systemz {


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

[PATCH] D45212: Add HIP toolchain

[Yaxun Liu via Phabricator via cfe-commits]

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



Comment at: lib/Driver/ToolChains/HIP.cpp:44
+  }
+  if (!FoundLibDevice)
+C.getDriver().Diag(diag::err_drv_no_such_file) << BCName;

tra wrote:
> You don't need FoundLibDevice any more as you will  always return from inside 
> the loop if it is ever true.
will remove when committing. Thanks!


https://reviews.llvm.org/D45212



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

[PATCH] D47097: [DebugInfo] Preserve scope in auto generated StoreInst

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added inline comments.



Comment at: lib/CodeGen/CGExpr.cpp:72
+  // Set debug location in order to preserve the scope
+  Alloca->setDebugLoc(Builder.getCurrentDebugLocation());
   if (AllocaAddr)

vsk wrote:
> aprantl wrote:
> > vsk wrote:
> > > aprantl wrote:
> > > > vsk wrote:
> > > > > I think we need to be a bit more careful here. The current debug 
> > > > > location stored in the builder may not be an artificial 0-location. 
> > > > > This may cause non-linear single-stepping behavior. Consider this 
> > > > > example:
> > > > > 
> > > > > ```
> > > > > void foo() {
> > > > >   bar();
> > > > >   if (...) {
> > > > > int var = ...; //< Clang emits an alloca for "var".
> > > > >   }
> > > > > ...
> > > > > ```
> > > > > 
> > > > > The current debug location at the line "int var = ..." would be at 
> > > > > line 4. But the alloca is emitted in the entry block of the function. 
> > > > > In the debugger, this may result in strange single-stepping behavior 
> > > > > when stepping into foo(). You could step to line 4, then line 2, then 
> > > > > line 3, then line 4 again.
> > > > > 
> > > > > I think we can avoid that by setting an artificial location on 
> > > > > allocas.
> > > > > I think we can avoid that by setting an artificial location on 
> > > > > allocas.
> > > > An alloca doesn't really generate any code (or rather.. the code it 
> > > > generates is in the function prologue). In what situation would the 
> > > > debug location on an alloca influence stepping? Are you thinking about 
> > > > the alloca() function?
> > > An alloca instruction can lower to a subtraction (off the stack pointer) 
> > > though: https://godbolt.org/g/U4nGzJ.
> > > 
> > > `dwarfdump` shows that this subtraction instruction is actually assigned 
> > > a location -- it currently happens to be the first location in the body 
> > > of the function. I thought that assigning an artificial location to the 
> > > alloca would be a first step towards fixing this.
> > > 
> > > Also, using an artificial location could mitigate possible bad 
> > > interactions between code motion passes and IRBuilder. If, say, we were 
> > > to set the insertion point right after an alloca, we might infer some 
> > > arbitrary debug location. So long as this inference happens, it seems 
> > > safer to infer an artificial location.
> > > 
> > > 
> > This may have unintended side-effects: By assigning a debug location to an 
> > alloca you are moving the end of the function prolog to before the alloca 
> > instructions, since LLVM computes the end of the function prologue as the 
> > first instruction with a non-empty debug location. Moving the end of the 
> > function prologue to before that stack pointer is adjusted is wrong, since 
> > that's the whole point of the prologue_end marker.
> > 
> > To me it looks more like a bug in a much later stage. With the exception of 
> > shrink-wrapped code, the prologue_end should always be after the stack 
> > pointer adjustment, I think.
> Thanks for explaining, I didn't realize that's how the end of the function 
> prologue is computed! Should we leave out the any debug location changes for 
> allocas in this patch, then?
I think that would be better. You might want to double-check 
PrologueEpilogueInserter and how the FrameSetup attribute is attached to 
MachineInstrs in case my knowledge is out-of-date.


Repository:
  rC Clang

https://reviews.llvm.org/D47097



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

[PATCH] D47280: [Sema][ObjC] Do not DiagnoseUseOfDecl in LookupMemberExpr

[Steven Wu via Phabricator via cfe-commits]

steven_wu created this revision.
steven_wu added reviewers: erik.pilkington, arphaman, doug.gregor.

Remove the call to DiagnoseUseOfDecl in LookupMemberExpr because:

1. LookupMemberExpr eagerly lookup both getter and setter, reguardless

if they are used or not. It causes wrong diagnostics if you are only
using getter.

2. LookupMemberExpr only diagnoses getter, but not setter.
3. ObjCPropertyOpBuilder already DiagnoseUseOfDecl when building getter

and setter. Doing it again in LookupMemberExpr causes duplicated
diagnostics.

rdar://problem/38479756


Repository:
  rC Clang

https://reviews.llvm.org/D47280

Files:
  lib/Sema/SemaExprMember.cpp
  test/SemaObjC/property-deprecated-warning.m


Index: test/SemaObjC/property-deprecated-warning.m
===
--- test/SemaObjC/property-deprecated-warning.m
+++ test/SemaObjC/property-deprecated-warning.m
@@ -167,3 +167,14 @@
foo.x = foo.x; // expected-error {{property access is using 'x' method 
which is unavailable}} \
   // expected-warning {{property access is using 'setX:' 
method which is deprecated}}
 }
+
+// test implicit property does not emit duplicated warning.
+@protocol Foo
+- (int)size __attribute__((availability(ios,deprecated=3.0))); // 
expected-note {{'size' has been explicitly marked deprecated here}}
+- (void)setSize: (int)x __attribute__((availability(ios,deprecated=2.0))); // 
expected-note {{'setSize:' has been explicitly marked deprecated here}}
+@end
+
+void testImplicitProperty(id object) {
+  object.size = object.size; // expected-warning {{'size' is deprecated: first 
deprecated in iOS 3.0}} \
+ // expected-warning {{'setSize:' is deprecated: 
first deprecated in iOS 2.0}}
+}
Index: lib/Sema/SemaExprMember.cpp
===
--- lib/Sema/SemaExprMember.cpp
+++ lib/Sema/SemaExprMember.cpp
@@ -1490,9 +1490,6 @@
 }
 
 if (ObjCMethodDecl *OMD = dyn_cast(PMDecl)) {
-  // Check the use of this method.
-  if (S.DiagnoseUseOfDecl(OMD, MemberLoc))
-return ExprError();
   Selector SetterSel =
 SelectorTable::constructSetterSelector(S.PP.getIdentifierTable(),
S.PP.getSelectorTable(),


Index: test/SemaObjC/property-deprecated-warning.m
===
--- test/SemaObjC/property-deprecated-warning.m
+++ test/SemaObjC/property-deprecated-warning.m
@@ -167,3 +167,14 @@
 	foo.x = foo.x; // expected-error {{property access is using 'x' method which is unavailable}} \
 		   // expected-warning {{property access is using 'setX:' method which is deprecated}}
 }
+
+// test implicit property does not emit duplicated warning.
+@protocol Foo
+- (int)size __attribute__((availability(ios,deprecated=3.0))); // expected-note {{'size' has been explicitly marked deprecated here}}
+- (void)setSize: (int)x __attribute__((availability(ios,deprecated=2.0))); // expected-note {{'setSize:' has been explicitly marked deprecated here}}
+@end
+
+void testImplicitProperty(id object) {
+  object.size = object.size; // expected-warning {{'size' is deprecated: first deprecated in iOS 3.0}} \
+ // expected-warning {{'setSize:' is deprecated: first deprecated in iOS 2.0}}
+}
Index: lib/Sema/SemaExprMember.cpp
===
--- lib/Sema/SemaExprMember.cpp
+++ lib/Sema/SemaExprMember.cpp
@@ -1490,9 +1490,6 @@
 }
 
 if (ObjCMethodDecl *OMD = dyn_cast(PMDecl)) {
-  // Check the use of this method.
-  if (S.DiagnoseUseOfDecl(OMD, MemberLoc))
-return ExprError();
   Selector SetterSel =
 SelectorTable::constructSetterSelector(S.PP.getIdentifierTable(),
S.PP.getSelectorTable(),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45212: Add HIP toolchain

[Artem Belevich via Phabricator via cfe-commits]

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

One small nit. LGTM otherwise.




Comment at: lib/Driver/ToolChains/HIP.cpp:44
+  }
+  if (!FoundLibDevice)
+C.getDriver().Diag(diag::err_drv_no_such_file) << BCName;

You don't need FoundLibDevice any more as you will  always return from inside 
the loop if it is ever true.


https://reviews.llvm.org/D45212



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

[PATCH] D47273: [bash-completion] Fix tab separation on macOS

[Rui Ueyama via Phabricator via cfe-commits]

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

LGTM


https://reviews.llvm.org/D47273



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

[PATCH] D45212: Add HIP toolchain

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 148277.
yaxunl marked 6 inline comments as done.
yaxunl added a comment.

Revised by Artem's comments.


https://reviews.llvm.org/D45212

Files:
  include/clang/Driver/Options.td
  lib/Driver/CMakeLists.txt
  lib/Driver/Driver.cpp
  lib/Driver/ToolChains/HIP.cpp
  lib/Driver/ToolChains/HIP.h
  test/Driver/Inputs/hip_multiple_inputs/lib1/lib1.bc
  test/Driver/Inputs/hip_multiple_inputs/lib2/lib2.bc
  test/Driver/hip-toolchain.hip

Index: test/Driver/hip-toolchain.hip
===
--- /dev/null
+++ test/Driver/hip-toolchain.hip
@@ -0,0 +1,84 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+
+// RUN: %clang -### -target x86_64-linux-gnu \
+// RUN:   -x hip --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 \
+// RUN:   --hip-device-lib=lib1.bc --hip-device-lib=lib2.bc \
+// RUN:   --hip-device-lib-path=%S/Inputs/hip_multiple_inputs/lib1 \
+// RUN:   --hip-device-lib-path=%S/Inputs/hip_multiple_inputs/lib2 \
+// RUN:   -fuse-ld=lld \
+// RUN:   %S/Inputs/hip_multiple_inputs/a.cu \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
+// RUN: 2>&1 | FileCheck %s
+
+// CHECK: [[CLANG:".*clang.*"]] "-cc1" "-triple" "amdgcn-amd-amdhsa" 
+// CHECK-SAME: "-aux-triple" "x86_64--linux-gnu" "-emit-llvm-bc"
+// CHECK-SAME: {{.*}} "-main-file-name" "a.cu" {{.*}} "-fcuda-is-device"
+// CHECK-SAME: {{.*}} "-o" [[A_BC:".*bc"]] "-x" "hip"
+// CHECK-SAME: {{.*}} [[A_SRC:".*a.cu"]]
+
+// CHECK: [[CLANG]] "-cc1" "-triple" "amdgcn-amd-amdhsa"
+// CHECK-SAME: "-aux-triple" "x86_64--linux-gnu" "-emit-llvm-bc"
+// CHECK-SAME: {{.*}} "-main-file-name" "b.hip" {{.*}} "-fcuda-is-device"
+// CHECK-SAME: {{.*}} "-o" [[B_BC:".*bc"]] "-x" "hip"
+// CHECK-SAME: {{.*}} [[B_SRC:".*b.hip"]]
+
+// CHECK: [[LLVM_LINK:"*.llvm-link"]] [[A_BC]] [[B_BC]]
+// CHECK-SAME: "{{.*}}lib1.bc" "{{.*}}lib2.bc"
+// CHECK-SAME: "-o" [[LINKED_BC_DEV1:".*-gfx803-linked-.*bc"]]
+
+// CHECK: [[OPT:".*opt"]] [[LINKED_BC_DEV1]] "-mtriple=amdgcn-amd-amdhsa"
+// CHECK-SAME: "-mcpu=gfx803"
+// CHECK-SAME: "-o" [[OPT_BC_DEV1:".*-gfx803-optimized.*bc"]]
+
+// CHECK: [[LLC: ".*llc"]] [[OPT_BC_DEV1]] "-mtriple=amdgcn-amd-amdhsa"
+// CHECK-SAME: "-filetype=obj" "-mcpu=gfx803" "-o" [[OBJ_DEV1:".*-gfx803-.*o"]]
+
+// CHECK: [[LLD: ".*lld"]] "-flavor" "gnu" "--no-undefined" "-shared"
+// CHECK-SAME: "-o" "[[IMG_DEV1:.*out]]" [[OBJ_DEV1]]
+
+// CHECK: [[CLANG]] "-cc1" "-triple" "amdgcn-amd-amdhsa" 
+// CHECK-SAME: "-aux-triple" "x86_64--linux-gnu" "-emit-llvm-bc"
+// CHECK-SAME: {{.*}} "-main-file-name" "a.cu" {{.*}} "-fcuda-is-device"
+// CHECK-SAME: {{.*}} "-o" [[A_BC:".*bc"]] "-x" "hip"
+// CHECK-SAME: {{.*}} [[A_SRC]]
+
+// CHECK: [[CLANG]] "-cc1" "-triple" "amdgcn-amd-amdhsa"
+// CHECK-SAME: "-aux-triple" "x86_64--linux-gnu" "-emit-llvm-bc"
+// CHECK-SAME: {{.*}} "-main-file-name" "b.hip" {{.*}} "-fcuda-is-device"
+// CHECK-SAME: {{.*}} "-o" [[B_BC:".*bc"]] "-x" "hip"
+// CHECK-SAME: {{.*}} [[B_SRC]]
+
+// CHECK: [[LLVM_LINK]] [[A_BC]] [[B_BC]]
+// CHECK-SAME: "{{.*}}lib1.bc" "{{.*}}lib2.bc"
+// CHECK-SAME: "-o" [[LINKED_BC_DEV2:".*-gfx900-linked-.*bc"]]
+
+// CHECK: [[OPT]] [[LINKED_BC_DEV2]] "-mtriple=amdgcn-amd-amdhsa"
+// CHECK-SAME: "-mcpu=gfx900"
+// CHECK-SAME: "-o" [[OPT_BC_DEV2:".*-gfx900-optimized.*bc"]]
+
+// CHECK: [[LLC]] [[OPT_BC_DEV2]] "-mtriple=amdgcn-amd-amdhsa"
+// CHECK-SAME: "-filetype=obj" "-mcpu=gfx900" "-o" [[OBJ_DEV2:".*-gfx900-.*o"]]
+
+// CHECK: [[LLD]] "-flavor" "gnu" "--no-undefined" "-shared"
+// CHECK-SAME: "-o" "[[IMG_DEV2:.*out]]" [[OBJ_DEV2]]
+
+// CHECK: [[CLANG]] "-cc1" "-triple" "x86_64--linux-gnu" 
+// CHECK-SAME: "-aux-triple" "amdgcn-amd-amdhsa" "-emit-obj"
+// CHECK-SAME: {{.*}} "-main-file-name" "a.cu"
+// CHECK-SAME: {{.*}} "-o" [[A_OBJ_HOST:".*o"]] "-x" "hip"
+// CHECK-SAME: {{.*}} [[A_SRC]]
+
+// CHECK: [[CLANG]] "-cc1" "-triple" "x86_64--linux-gnu"
+// CHECK-SAME: "-aux-triple" "amdgcn-amd-amdhsa" "-emit-obj"
+// CHECK-SAME: {{.*}} "-main-file-name" "b.hip"
+// CHECK-SAME: {{.*}} "-o" [[B_OBJ_HOST:".*o"]] "-x" "hip"
+// CHECK-SAME: {{.*}} [[B_SRC]]
+
+// CHECK: [[BUNDLER:".*clang-offload-bundler"]] "-type=o"
+// CHECK-SAME: "-targets={{.*}},hip-amdgcn-amd-amdhsa-gfx803,hip-amdgcn-amd-amdhsa-gfx900"
+// CHECK-SAME: "-inputs={{.*}},[[IMG_DEV1]],[[IMG_DEV2]]" "-outputs=[[BUNDLE:.*o]]"
+
+// CHECK: [[LD:".*ld.lld"]] {{.*}} [[A_OBJ_HOST]] [[B_OBJ_HOST]]
+// CHECK-SAME: {{.*}} "-T" "{{.*}}.lk"
Index: lib/Driver/ToolChains/HIP.h
===
--- /dev/null
+++ lib/Driver/ToolChains/HIP.h
@@ -0,0 +1,123 @@
+//===--- HIP.h - HIP ToolChain Implementations --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//

[PATCH] D47273: [bash-completion] Fix tab separation on macOS

[Ben Langmuir via Phabricator via cfe-commits]

benlangmuir updated this revision to Diff 148278.
benlangmuir edited the summary of this revision.
benlangmuir added a comment.

Thanks for looking up  the supported bash version!  Updated diff.


https://reviews.llvm.org/D47273

Files:
  utils/bash-autocomplete.sh


Index: utils/bash-autocomplete.sh
===
--- utils/bash-autocomplete.sh
+++ utils/bash-autocomplete.sh
@@ -38,7 +38,8 @@
 
   # expand ~ to $HOME
   eval local path=${COMP_WORDS[0]}
-  flags=$( "$path" --autocomplete="$arg" 2>/dev/null | sed -e 's/\t.*//' )
+  # Use $'\t' so that bash expands the \t for older versions of sed.
+  flags=$( "$path" --autocomplete="$arg" 2>/dev/null | sed -e $'s/\t.*//' )
   # If clang is old that it does not support --autocomplete,
   # fall back to the filename completion.
   if [[ "$?" != 0 ]]; then


Index: utils/bash-autocomplete.sh
===
--- utils/bash-autocomplete.sh
+++ utils/bash-autocomplete.sh
@@ -38,7 +38,8 @@
 
   # expand ~ to $HOME
   eval local path=${COMP_WORDS[0]}
-  flags=$( "$path" --autocomplete="$arg" 2>/dev/null | sed -e 's/\t.*//' )
+  # Use $'\t' so that bash expands the \t for older versions of sed.
+  flags=$( "$path" --autocomplete="$arg" 2>/dev/null | sed -e $'s/\t.*//' )
   # If clang is old that it does not support --autocomplete,
   # fall back to the filename completion.
   if [[ "$?" != 0 ]]; then
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45212: Add HIP toolchain

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl marked 6 inline comments as done.
yaxunl added inline comments.



Comment at: lib/Driver/ToolChains/HIP.cpp:29-47
+static bool addBCLib(Compilation , const ArgList ,
+ ArgStringList , ArgStringList LibraryPaths,
+ StringRef BCName) {
+  StringRef FullName;
+  bool FoundLibDevice = false;
+  for (std::string LibraryPath : LibraryPaths) {
+SmallString<128> Path(LibraryPath);

tra wrote:
> FullName may remain uninitialized if LibraryPaths are empty which will 
> probably crash compiler when you attempt to pass it to MakeArgString.
> If empty LibraryPaths is not expected there should be an assert.
> 
> If the library is not found, we issue an error, but we still proceed to 
> append the FullName to the CmdArgs. I don't think we should do that. FullName 
> will be either NULL or pointing to the last directory in the LibraryPaths. 
> 
> You seem to be relying on diagnostics to deal with errors and are not using 
> return value of the function. You may as well make it void.
> 
> I'd move  `CmdArgs.push_back(...)` under `if(::exists(FullName))` and change 
> `break` to `return`;
> Then you can get rid of FoundLibDevice and just issue the error if we ever 
> reach the end of the function.
> 
Will CmdArgs.push_back(...) under if(::exists(FullName)) and change break to 
return; and change return type to void.



Comment at: lib/Driver/ToolChains/HIP.cpp:79-81
+std::string ISAVerBC = "oclc_isa_version_";
+ISAVerBC = ISAVerBC + SubArchName.drop_front(3).str();
+ISAVerBC = ISAVerBC + ".amdgcn.bc";

tra wrote:
> No need for intermediate values here -- just '+' all parts together. 
> 
will do



Comment at: lib/Driver/ToolChains/HIP.cpp:133
+}
+OptArgs.push_back(Args.MakeArgString(llvm::Twine("-O") + OOpt));
+  }

tra wrote:
> Nit: I think explicit llvm::Twine is unnecessary here. 
will remove



Comment at: lib/Driver/ToolChains/HIP.cpp:155-160
+  ArgStringList LlcArgs;
+  LlcArgs.push_back(InputFileName);
+  LlcArgs.push_back("-mtriple=amdgcn-amd-amdhsa");
+  LlcArgs.push_back("-filetype=obj");
+  LlcArgs.push_back(Args.MakeArgString("-mcpu=" + SubArchName));
+  LlcArgs.push_back("-o");

tra wrote:
> Nit: THis could be collapsed into `ArgStringList LlcArgs({...});`
will do



Comment at: lib/Driver/ToolChains/HIP.cpp:179-181
+  ArgStringList LldArgs;
+  // The output from ld.lld is an HSA code object file.
+  LldArgs.append({"-flavor", "gnu", "--no-undefined", "-shared", "-o"});

tra wrote:
> Same here: `ArgStringList LldArgs({"-flavor", "gnu", "--no-undefined", 
> "-shared", "-o"});`
will do



Comment at: lib/Driver/ToolChains/HIP.cpp:212-215
+  TempFile =
+  constructOptCommand(C, JA, Inputs, Args, SubArchName, Prefix, TempFile);
+  TempFile =
+  constructLlcCommand(C, JA, Inputs, Args, SubArchName, Prefix, TempFile);

tra wrote:
> Right now the code is structured as if you're appending to the same TempFile 
> string which is not the case here. I'd give intermediate variables their own 
> names -- `OptCommand`,`LlcCommand`.
> This would make it easier to see that you are **chaining** separate commands, 
> each producing its own temp output file.
will do


https://reviews.llvm.org/D45212



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

[PATCH] D47277: [modules] Mark __wmmintrin_pclmul.h/__wmmintrin_aes.h as textual

[Craig Topper via Phabricator via cfe-commits]

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

LGTM


https://reviews.llvm.org/D47277



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

[PATCH] D46863: [X86] Use __builtin_convertvector to implement some of the packed integer to packed float conversion intrinsics.

[Craig Topper via Phabricator via cfe-commits]

craig.topper added a comment.

Hi @aemerson, I'm not opposed to adding it back. But the clang policy for 
vector builtins has always been that we won't support all the builtins that gcc 
does and to encourage the use of the _mm_* wrappers which are guaranteed to 
work in both compilers. It possible to change your source code to use the 
portable intrinsic name?


Repository:
  rC Clang

https://reviews.llvm.org/D46863



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

[PATCH] D47097: [DebugInfo] Preserve scope in auto generated StoreInst

[Vedant Kumar via Phabricator via cfe-commits]

vsk added inline comments.



Comment at: lib/CodeGen/CGExpr.cpp:72
+  // Set debug location in order to preserve the scope
+  Alloca->setDebugLoc(Builder.getCurrentDebugLocation());
   if (AllocaAddr)

aprantl wrote:
> vsk wrote:
> > aprantl wrote:
> > > vsk wrote:
> > > > I think we need to be a bit more careful here. The current debug 
> > > > location stored in the builder may not be an artificial 0-location. 
> > > > This may cause non-linear single-stepping behavior. Consider this 
> > > > example:
> > > > 
> > > > ```
> > > > void foo() {
> > > >   bar();
> > > >   if (...) {
> > > > int var = ...; //< Clang emits an alloca for "var".
> > > >   }
> > > > ...
> > > > ```
> > > > 
> > > > The current debug location at the line "int var = ..." would be at line 
> > > > 4. But the alloca is emitted in the entry block of the function. In the 
> > > > debugger, this may result in strange single-stepping behavior when 
> > > > stepping into foo(). You could step to line 4, then line 2, then line 
> > > > 3, then line 4 again.
> > > > 
> > > > I think we can avoid that by setting an artificial location on allocas.
> > > > I think we can avoid that by setting an artificial location on allocas.
> > > An alloca doesn't really generate any code (or rather.. the code it 
> > > generates is in the function prologue). In what situation would the debug 
> > > location on an alloca influence stepping? Are you thinking about the 
> > > alloca() function?
> > An alloca instruction can lower to a subtraction (off the stack pointer) 
> > though: https://godbolt.org/g/U4nGzJ.
> > 
> > `dwarfdump` shows that this subtraction instruction is actually assigned a 
> > location -- it currently happens to be the first location in the body of 
> > the function. I thought that assigning an artificial location to the alloca 
> > would be a first step towards fixing this.
> > 
> > Also, using an artificial location could mitigate possible bad interactions 
> > between code motion passes and IRBuilder. If, say, we were to set the 
> > insertion point right after an alloca, we might infer some arbitrary debug 
> > location. So long as this inference happens, it seems safer to infer an 
> > artificial location.
> > 
> > 
> This may have unintended side-effects: By assigning a debug location to an 
> alloca you are moving the end of the function prolog to before the alloca 
> instructions, since LLVM computes the end of the function prologue as the 
> first instruction with a non-empty debug location. Moving the end of the 
> function prologue to before that stack pointer is adjusted is wrong, since 
> that's the whole point of the prologue_end marker.
> 
> To me it looks more like a bug in a much later stage. With the exception of 
> shrink-wrapped code, the prologue_end should always be after the stack 
> pointer adjustment, I think.
Thanks for explaining, I didn't realize that's how the end of the function 
prologue is computed! Should we leave out the any debug location changes for 
allocas in this patch, then?


Repository:
  rC Clang

https://reviews.llvm.org/D47097



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

[PATCH] D47097: [DebugInfo] Preserve scope in auto generated StoreInst

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added inline comments.



Comment at: lib/CodeGen/CGExpr.cpp:72
+  // Set debug location in order to preserve the scope
+  Alloca->setDebugLoc(Builder.getCurrentDebugLocation());
   if (AllocaAddr)

vsk wrote:
> aprantl wrote:
> > vsk wrote:
> > > I think we need to be a bit more careful here. The current debug location 
> > > stored in the builder may not be an artificial 0-location. This may cause 
> > > non-linear single-stepping behavior. Consider this example:
> > > 
> > > ```
> > > void foo() {
> > >   bar();
> > >   if (...) {
> > > int var = ...; //< Clang emits an alloca for "var".
> > >   }
> > > ...
> > > ```
> > > 
> > > The current debug location at the line "int var = ..." would be at line 
> > > 4. But the alloca is emitted in the entry block of the function. In the 
> > > debugger, this may result in strange single-stepping behavior when 
> > > stepping into foo(). You could step to line 4, then line 2, then line 3, 
> > > then line 4 again.
> > > 
> > > I think we can avoid that by setting an artificial location on allocas.
> > > I think we can avoid that by setting an artificial location on allocas.
> > An alloca doesn't really generate any code (or rather.. the code it 
> > generates is in the function prologue). In what situation would the debug 
> > location on an alloca influence stepping? Are you thinking about the 
> > alloca() function?
> An alloca instruction can lower to a subtraction (off the stack pointer) 
> though: https://godbolt.org/g/U4nGzJ.
> 
> `dwarfdump` shows that this subtraction instruction is actually assigned a 
> location -- it currently happens to be the first location in the body of the 
> function. I thought that assigning an artificial location to the alloca would 
> be a first step towards fixing this.
> 
> Also, using an artificial location could mitigate possible bad interactions 
> between code motion passes and IRBuilder. If, say, we were to set the 
> insertion point right after an alloca, we might infer some arbitrary debug 
> location. So long as this inference happens, it seems safer to infer an 
> artificial location.
> 
> 
This may have unintended side-effects: By assigning a debug location to an 
alloca you are moving the end of the function prolog to before the alloca 
instructions, since LLVM computes the end of the function prologue as the first 
instruction with a non-empty debug location. Moving the end of the function 
prologue to before that stack pointer is adjusted is wrong, since that's the 
whole point of the prologue_end marker.

To me it looks more like a bug in a much later stage. With the exception of 
shrink-wrapped code, the prologue_end should always be after the stack pointer 
adjustment, I think.


Repository:
  rC Clang

https://reviews.llvm.org/D47097



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

[PATCH] D47097: [DebugInfo] Preserve scope in auto generated StoreInst

[Vedant Kumar via Phabricator via cfe-commits]

vsk added inline comments.



Comment at: lib/CodeGen/CGExpr.cpp:72
+  // Set debug location in order to preserve the scope
+  Alloca->setDebugLoc(Builder.getCurrentDebugLocation());
   if (AllocaAddr)

aprantl wrote:
> vsk wrote:
> > I think we need to be a bit more careful here. The current debug location 
> > stored in the builder may not be an artificial 0-location. This may cause 
> > non-linear single-stepping behavior. Consider this example:
> > 
> > ```
> > void foo() {
> >   bar();
> >   if (...) {
> > int var = ...; //< Clang emits an alloca for "var".
> >   }
> > ...
> > ```
> > 
> > The current debug location at the line "int var = ..." would be at line 4. 
> > But the alloca is emitted in the entry block of the function. In the 
> > debugger, this may result in strange single-stepping behavior when stepping 
> > into foo(). You could step to line 4, then line 2, then line 3, then line 4 
> > again.
> > 
> > I think we can avoid that by setting an artificial location on allocas.
> > I think we can avoid that by setting an artificial location on allocas.
> An alloca doesn't really generate any code (or rather.. the code it generates 
> is in the function prologue). In what situation would the debug location on 
> an alloca influence stepping? Are you thinking about the alloca() function?
An alloca instruction can lower to a subtraction (off the stack pointer) 
though: https://godbolt.org/g/U4nGzJ.

`dwarfdump` shows that this subtraction instruction is actually assigned a 
location -- it currently happens to be the first location in the body of the 
function. I thought that assigning an artificial location to the alloca would 
be a first step towards fixing this.

Also, using an artificial location could mitigate possible bad interactions 
between code motion passes and IRBuilder. If, say, we were to set the insertion 
point right after an alloca, we might infer some arbitrary debug location. So 
long as this inference happens, it seems safer to infer an artificial location.




Repository:
  rC Clang

https://reviews.llvm.org/D47097



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

[PATCH] D46863: [X86] Use __builtin_convertvector to implement some of the packed integer to packed float conversion intrinsics.

[Amara Emerson via Phabricator via cfe-commits]

aemerson added a comment.

Hi Craig,

The `__builtin_ia32_cvtdq2ps` builtin seems to be supported by gcc, and this 
change breaks code which has been using this without problems for years. Can we 
restore it?

Amara


Repository:
  rC Clang

https://reviews.llvm.org/D46863



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

[PATCH] D47277: [modules] Mark __wmmintrin_pclmul.h/__wmmintrin_aes.h as textual

[Raphael Isemann via Phabricator via cfe-commits]

teemperor edited reviewers, added: v.g.vassilev; removed: slydiman.
teemperor added a comment.

(Wrong Vassil, sorry)


https://reviews.llvm.org/D47277



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

[PATCH] D47277: [modules] Mark __wmmintrin_pclmul.h/__wmmintrin_aes.h as textual

[Raphael Isemann via Phabricator via cfe-commits]

teemperor created this revision.
teemperor added reviewers: rsmith, slydiman.

Since clang r332929 these two headers throw errors when included from somewhere 
else than their wrapper header. It seems marking them as textual is the best 
way to fix the builds.

Fixes this new module build error:

  While building module '_Builtin_intrinsics' imported from ...:
  In file included from :2:
  In file included from lib/clang/7.0.0/include/immintrin.h:54:
  In file included from lib/clang/7.0.0/include/wmmintrin.h:29:
  lib/clang/7.0.0/include/__wmmintrin_aes.h:25:2: error: "Never use 
<__wmmintrin_aes.h> directly; include  instead."
  #error "Never use <__wmmintrin_aes.h> directly; include  
instead."


https://reviews.llvm.org/D47277

Files:
  lib/Headers/module.modulemap


Index: lib/Headers/module.modulemap
===
--- lib/Headers/module.modulemap
+++ lib/Headers/module.modulemap
@@ -71,6 +71,9 @@
 textual header "sgxintrin.h"
 textual header "ptwriteintrin.h"
 
+textual header "__wmmintrin_aes.h"
+textual header "__wmmintrin_pclmul.h"
+
 explicit module mm_malloc {
   requires !freestanding
   header "mm_malloc.h"
@@ -136,14 +139,6 @@
   export aes
   export pclmul
 }
-
-explicit module aes {
-  header "__wmmintrin_aes.h"
-}
-
-explicit module pclmul {
-  header "__wmmintrin_pclmul.h"
-}
   }
 
   explicit module systemz {


Index: lib/Headers/module.modulemap
===
--- lib/Headers/module.modulemap
+++ lib/Headers/module.modulemap
@@ -71,6 +71,9 @@
 textual header "sgxintrin.h"
 textual header "ptwriteintrin.h"
 
+textual header "__wmmintrin_aes.h"
+textual header "__wmmintrin_pclmul.h"
+
 explicit module mm_malloc {
   requires !freestanding
   header "mm_malloc.h"
@@ -136,14 +139,6 @@
   export aes
   export pclmul
 }
-
-explicit module aes {
-  header "__wmmintrin_aes.h"
-}
-
-explicit module pclmul {
-  header "__wmmintrin_pclmul.h"
-}
   }
 
   explicit module systemz {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47122: [clang-tidy] SimplifyBoolenExpr doesn't add parens if unary negotiation is of ExprWithCleanups type

[Malcolm Parsons via Phabricator via cfe-commits]

malcolm.parsons added inline comments.



Comment at: 
clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.cpp:198
   E = E->ignoreParenBaseCasts();
+  if (const auto *EC = dyn_cast(E))
+E = EC->getSubExpr();

zinovy.nis wrote:
> zinovy.nis wrote:
> > malcolm.parsons wrote:
> > > `E->IgnoreImplicit()` can be used to ignore `ExprWithCleanups`
> > Thanks. But it seems to be too agressive:
> > 
> > 
> > ```
> > return (i & 1) != 0;
> > ```
> > 
> > becomes 
> > 
> > ```
> > return static_cast(i & 1);
> > ```
> > 
> ```
>   if (!isa(E))
> E = E->IgnoreImplicit();
> 
> ```
> 
> works properly but looks a bit verbose. What do you think?
I think what you've committed is fine.


Repository:
  rL LLVM

https://reviews.llvm.org/D47122



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

[PATCH] D47273: [bash-completion] Fix tab separation on macOS

[Rui Ueyama via Phabricator via cfe-commits]

ruiu added a comment.

(I found that information at 
http://wiki.bash-hackers.org/scripting/bashchanges#quoting_expansions_substitutions_and_related)


Repository:
  rC Clang

https://reviews.llvm.org/D47273



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

[PATCH] D47273: [bash-completion] Fix tab separation on macOS

[Rui Ueyama via Phabricator via cfe-commits]

ruiu added a comment.

Looks like $'' is there since bash 2.0 which should be pretty safe to use in 
the script. So feel free to use $'' instead of $(echo ...) in this patch.


Repository:
  rC Clang

https://reviews.llvm.org/D47273



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

[PATCH] D47273: [bash-completion] Fix tab separation on macOS

[Rui Ueyama via Phabricator via cfe-commits]

ruiu added a comment.

Oh, I didn't know the existence of $''. Thank you for the suggestion! I wonder 
which version of bash started supporting that notation. Do you know if all 
recent versions of bash support it? Unfortunately `$'' bash` is very hard query 
to google...


Repository:
  rC Clang

https://reviews.llvm.org/D47273



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

[PATCH] D47273: [bash-completion] Fix tab separation on macOS

[Ben Langmuir via Phabricator via cfe-commits]

benlangmuir added a comment.

In https://reviews.llvm.org/D47273#1109985, @ruiu wrote:

>   sed -e "$(echo -e 's/\t.*//')"


Yep, that works!  Is there a reason you prefer that over the more succinct 
`$'s/\t.*//'`?


Repository:
  rC Clang

https://reviews.llvm.org/D47273



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

[PATCH] D45212: Add HIP toolchain

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: lib/Driver/ToolChains/HIP.cpp:29-47
+static bool addBCLib(Compilation , const ArgList ,
+ ArgStringList , ArgStringList LibraryPaths,
+ StringRef BCName) {
+  StringRef FullName;
+  bool FoundLibDevice = false;
+  for (std::string LibraryPath : LibraryPaths) {
+SmallString<128> Path(LibraryPath);

FullName may remain uninitialized if LibraryPaths are empty which will probably 
crash compiler when you attempt to pass it to MakeArgString.
If empty LibraryPaths is not expected there should be an assert.

If the library is not found, we issue an error, but we still proceed to append 
the FullName to the CmdArgs. I don't think we should do that. FullName will be 
either NULL or pointing to the last directory in the LibraryPaths. 

You seem to be relying on diagnostics to deal with errors and are not using 
return value of the function. You may as well make it void.

I'd move  `CmdArgs.push_back(...)` under `if(::exists(FullName))` and change 
`break` to `return`;
Then you can get rid of FoundLibDevice and just issue the error if we ever 
reach the end of the function.




Comment at: lib/Driver/ToolChains/HIP.cpp:79-81
+std::string ISAVerBC = "oclc_isa_version_";
+ISAVerBC = ISAVerBC + SubArchName.drop_front(3).str();
+ISAVerBC = ISAVerBC + ".amdgcn.bc";

No need for intermediate values here -- just '+' all parts together. 




Comment at: lib/Driver/ToolChains/HIP.cpp:133
+}
+OptArgs.push_back(Args.MakeArgString(llvm::Twine("-O") + OOpt));
+  }

Nit: I think explicit llvm::Twine is unnecessary here. 



Comment at: lib/Driver/ToolChains/HIP.cpp:155-160
+  ArgStringList LlcArgs;
+  LlcArgs.push_back(InputFileName);
+  LlcArgs.push_back("-mtriple=amdgcn-amd-amdhsa");
+  LlcArgs.push_back("-filetype=obj");
+  LlcArgs.push_back(Args.MakeArgString("-mcpu=" + SubArchName));
+  LlcArgs.push_back("-o");

Nit: THis could be collapsed into `ArgStringList LlcArgs({...});`



Comment at: lib/Driver/ToolChains/HIP.cpp:179-181
+  ArgStringList LldArgs;
+  // The output from ld.lld is an HSA code object file.
+  LldArgs.append({"-flavor", "gnu", "--no-undefined", "-shared", "-o"});

Same here: `ArgStringList LldArgs({"-flavor", "gnu", "--no-undefined", 
"-shared", "-o"});`



Comment at: lib/Driver/ToolChains/HIP.cpp:212-215
+  TempFile =
+  constructOptCommand(C, JA, Inputs, Args, SubArchName, Prefix, TempFile);
+  TempFile =
+  constructLlcCommand(C, JA, Inputs, Args, SubArchName, Prefix, TempFile);

Right now the code is structured as if you're appending to the same TempFile 
string which is not the case here. I'd give intermediate variables their own 
names -- `OptCommand`,`LlcCommand`.
This would make it easier to see that you are **chaining** separate commands, 
each producing its own temp output file.


https://reviews.llvm.org/D45212



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

[PATCH] D47273: [bash-completion] Fix tab separation on macOS

[Rui Ueyama via Phabricator via cfe-commits]

ruiu added a comment.

How about

  sed -e "$(echo -e 's/\t.*//')"

?


Repository:
  rC Clang

https://reviews.llvm.org/D47273



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

[PATCH] D47267: [UnrollAndJam] Add unroll_and_jam pragma handling

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur added a comment.

In https://reviews.llvm.org/D47267#1109912, @dmgreen wrote:

> > Could we maybe disable the #pragma clang loop unroll_and_jam spelling ftm 
> > to avoid compatibility issues?
>
> Sure, I'm not against. It sounds like you have opinions on how this should 
> work. That's good. If there are multiple clang loop pragma's, what is the 
> expected behaviour there?
>
> In the llvm side of this, in the unroll and jam pass, I made it so that a 
> loop with "llvm.loop.unroll" metadata without any "llvm.loop.unroll_and_jam" 
> metadata will not do unroll and jam, it will leave the loop for the unroller. 
> On the expectation that the use really wants to unroll (and it applies to 
> llvm.loop.unroll.disable too, disabling unroll and jam as well as unroll). I 
> haven't done anything with other loop metadata though.


I'd expect that transformations "stack up". E.g.

  #pragma unroll_and_jam
  #pragma unroll(4)

which I'd expect to unroll by a factor of 4 first, then try to unroll-and-jam 
(which I am not sure https://reviews.llvm.org/D41953 can do due to then 
containing 4 loops). On the other hand

  #pragma unroll(4)
  #pragma unroll_and_jam

should unroll-and-jam, and then unroll the outer loop by a factor of 4.


https://reviews.llvm.org/D47267



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

[PATCH] D47273: [bash-completion] Fix tab separation on macOS

[Ben Langmuir via Phabricator via cfe-commits]

benlangmuir added a comment.

In https://reviews.llvm.org/D47273#1109934, @ruiu wrote:

> I wonder if you could replace \t with \0x09. At least it works on my machine 
> which has GNU sed.


Unfortunately that doesn't work either on mac :-\


Repository:
  rC Clang

https://reviews.llvm.org/D47273



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

[PATCH] D47274: [clangd] Serve comments for headers decls from dynamic index only

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added a reviewer: sammccall.
Herald added subscribers: jkorous, MaskRay, ioeric, klimek.

To fix a crash in code completion that occurrs when reading doc
comments from files that were updated after the preamble was
computed. In that case, the files on disk could've been changed and we
can't rely on finding the comment text with the same range anymore.

The current workaround is to not provide comments from the headers at
all and rely on the dynamic index instead.

A more principled solution would be to store contents of the files
read inside the preamble, but it is way harder to implement properly,
given that it would definitely increase the sizes of the preamble.

Together with https://reviews.llvm.org/D47272, this should all preamble-related 
crashes we're
aware of.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D47274

Files:
  clangd/CodeCompletionStrings.cpp
  clangd/CodeCompletionStrings.h
  clangd/index/SymbolCollector.cpp
  unittests/clangd/CodeCompleteTests.cpp

Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -17,6 +17,7 @@
 #include "SyncAPI.h"
 #include "TestFS.h"
 #include "index/MemIndex.h"
+#include "llvm/Support/Error.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -962,6 +963,46 @@
   }
 }
 
+TEST(CompletionTest, DocumentationFromChangedFileCrash) {
+  MockFSProvider FS;
+  auto FooH = testPath("foo.h");
+  auto FooCpp = testPath("foo.cpp");
+  FS.Files[FooH] = R"cpp(
+// this is my documentation comment.
+int func();
+  )cpp";
+  FS.Files[FooCpp] = "";
+
+  MockCompilationDatabase CDB;
+  IgnoreDiagnostics DiagConsumer;
+  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
+
+  Annotations Source(R"cpp(
+#include "foo.h"
+int func() {
+  // This makes sure we have func from header in the AST.
+}
+int a = fun^
+  )cpp");
+  Server.addDocument(FooCpp, Source.code(), WantDiagnostics::Yes);
+  // We need to wait for preamble to build.
+  ASSERT_TRUE(Server.blockUntilIdleForTest());
+
+  // Change the header file. Completion will reuse the old preamble!
+  FS.Files[FooH] = R"cpp(
+int func();
+  )cpp";
+
+  clangd::CodeCompleteOptions Opts;
+  Opts.IncludeComments = true;
+  CompletionList Completions =
+  cantFail(runCodeComplete(Server, FooCpp, Source.point(), Opts));
+  // We shouldn't crash. Unfortunately, current workaround is to not produce
+  // comments for symbols from headers.
+  EXPECT_THAT(Completions.items,
+  Contains(AllOf(Not(IsDocumented()), Named("func";
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -381,7 +381,8 @@
 /*EnableSnippets=*/false);
   std::string FilterText = getFilterText(*CCS);
   std::string Documentation =
-  formatDocumentation(*CCS, getDocComment(Ctx, SymbolCompletion));
+  formatDocumentation(*CCS, getDocComment(Ctx, SymbolCompletion,
+  /*NoCommentsFromHeaders=*/false));
   std::string CompletionDetail = getDetail(*CCS);
 
   std::string Include;
Index: clangd/CodeCompletionStrings.h
===
--- clangd/CodeCompletionStrings.h
+++ clangd/CodeCompletionStrings.h
@@ -25,18 +25,25 @@
 /// markers stripped. See clang::RawComment::getFormattedText() for the detailed
 /// explanation of how the comment text is transformed.
 /// Returns empty string when no comment is available.
+/// If \p NoCommentsFromHeaders parameter is set, only comments from the main
+/// file will be returned. It is used to workaround crashes when parsing
+/// comments in the stale headers, coming from completion preamble.
 std::string getDocComment(const ASTContext ,
-  const CodeCompletionResult );
+  const CodeCompletionResult ,
+  bool NoCommentsFromHeaders = true);
 
 /// Gets a minimally formatted documentation for parameter of \p Result,
 /// corresponding to argument number \p ArgIndex.
 /// This currently looks for comments attached to the parameter itself, and
 /// doesn't extract them from function documentation.
 /// Returns empty string when no comment is available.
+/// If \p NoCommentsFromHeaders parameter is set, only comments from the main
+/// file will be returned. It is used to workaround crashes when parsing
+/// comments in the stale headers, coming from completion preamble.
 std::string
 getParameterDocComment(const ASTContext ,
const CodeCompleteConsumer::OverloadCandidate ,
-   unsigned ArgIndex);
+

[PATCH] D47273: [bash-completion] Fix tab separation on macOS

[Rui Ueyama via Phabricator via cfe-commits]

ruiu added a comment.

Even though Perl may be installed to 99.99% of machines that use this 
autocomplete script, using perl instead of sed is too much. If we could use 
perl, we'd have wrote this script entirely in perl in the first place. We 
shouldn't add a dependency to perl.

I wonder if you could replace \t with \0x09. At least it works on my machine 
which has GNU sed.


Repository:
  rC Clang

https://reviews.llvm.org/D47273



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

[PATCH] D47097: [DebugInfo] Preserve scope in auto generated StoreInst

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added inline comments.



Comment at: lib/CodeGen/CGExpr.cpp:72
+  // Set debug location in order to preserve the scope
+  Alloca->setDebugLoc(Builder.getCurrentDebugLocation());
   if (AllocaAddr)

vsk wrote:
> I think we need to be a bit more careful here. The current debug location 
> stored in the builder may not be an artificial 0-location. This may cause 
> non-linear single-stepping behavior. Consider this example:
> 
> ```
> void foo() {
>   bar();
>   if (...) {
> int var = ...; //< Clang emits an alloca for "var".
>   }
> ...
> ```
> 
> The current debug location at the line "int var = ..." would be at line 4. 
> But the alloca is emitted in the entry block of the function. In the 
> debugger, this may result in strange single-stepping behavior when stepping 
> into foo(). You could step to line 4, then line 2, then line 3, then line 4 
> again.
> 
> I think we can avoid that by setting an artificial location on allocas.
> I think we can avoid that by setting an artificial location on allocas.
An alloca doesn't really generate any code (or rather.. the code it generates 
is in the function prologue). In what situation would the debug location on an 
alloca influence stepping? Are you thinking about the alloca() function?


Repository:
  rC Clang

https://reviews.llvm.org/D47097



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

[PATCH] D47272: [clangd] Build index on preamble changes instead of the AST changes

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 148262.
ilya-biryukov added a comment.

- Added forgotten renames


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D47272

Files:
  clangd/ClangdServer.cpp
  clangd/ClangdUnit.cpp
  clangd/ClangdUnit.h
  clangd/TUScheduler.cpp
  clangd/TUScheduler.h
  clangd/index/FileIndex.cpp
  clangd/index/FileIndex.h
  unittests/clangd/FileIndexTests.cpp
  unittests/clangd/TUSchedulerTests.cpp
  unittests/clangd/TestTU.cpp

Index: unittests/clangd/TestTU.cpp
===
--- unittests/clangd/TestTU.cpp
+++ unittests/clangd/TestTU.cpp
@@ -43,7 +43,7 @@
 
 SymbolSlab TestTU::headerSymbols() const {
   auto AST = build();
-  return indexAST();
+  return indexAST(AST.getASTContext(), AST.getPreprocessorPtr());
 }
 
 std::unique_ptr TestTU::index() const {
Index: unittests/clangd/TUSchedulerTests.cpp
===
--- unittests/clangd/TUSchedulerTests.cpp
+++ unittests/clangd/TUSchedulerTests.cpp
@@ -42,7 +42,7 @@
 TEST_F(TUSchedulerTests, MissingFiles) {
   TUScheduler S(getDefaultAsyncThreadsCount(),
 /*StorePreamblesInMemory=*/true,
-/*ASTParsedCallback=*/nullptr,
+/*PreambleParsedCallback=*/nullptr,
 /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero());
 
   auto Added = testPath("added.cpp");
@@ -98,7 +98,7 @@
 TUScheduler S(
 getDefaultAsyncThreadsCount(),
 /*StorePreamblesInMemory=*/true,
-/*ASTParsedCallback=*/nullptr,
+/*PreambleParsedCallback=*/nullptr,
 /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero());
 auto Path = testPath("foo.cpp");
 S.update(Path, getInputs(Path, ""), WantDiagnostics::Yes,
@@ -126,7 +126,7 @@
   {
 TUScheduler S(getDefaultAsyncThreadsCount(),
   /*StorePreamblesInMemory=*/true,
-  /*ASTParsedCallback=*/nullptr,
+  /*PreambleParsedCallback=*/nullptr,
   /*UpdateDebounce=*/std::chrono::seconds(1));
 // FIXME: we could probably use timeouts lower than 1 second here.
 auto Path = testPath("foo.cpp");
@@ -157,7 +157,7 @@
   {
 TUScheduler S(getDefaultAsyncThreadsCount(),
   /*StorePreamblesInMemory=*/true,
-  /*ASTParsedCallback=*/nullptr,
+  /*PreambleParsedCallback=*/nullptr,
   /*UpdateDebounce=*/std::chrono::milliseconds(50));
 
 std::vector Files;
Index: unittests/clangd/FileIndexTests.cpp
===
--- unittests/clangd/FileIndexTests.cpp
+++ unittests/clangd/FileIndexTests.cpp
@@ -87,7 +87,7 @@
   File.HeaderFilename = (Basename + ".h").str();
   File.HeaderCode = Code;
   auto AST = File.build();
-  M.update(File.Filename, );
+  M.update(File.Filename, (), AST.getPreprocessorPtr());
 }
 
 TEST(FileIndexTest, IndexAST) {
@@ -129,13 +129,13 @@
   Req.Scopes = {"ns::"};
   EXPECT_THAT(match(M, Req), UnorderedElementsAre("ns::f", "ns::X"));
 
-  M.update("f1.cpp", nullptr);
+  M.update("f1.cpp", nullptr, nullptr);
   EXPECT_THAT(match(M, Req), UnorderedElementsAre());
 }
 
 TEST(FileIndexTest, RemoveNonExisting) {
   FileIndex M;
-  M.update("no.cpp", nullptr);
+  M.update("no.cpp", nullptr, nullptr);
   EXPECT_THAT(match(M, FuzzyFindRequest()), UnorderedElementsAre());
 }
 
Index: clangd/index/FileIndex.h
===
--- clangd/index/FileIndex.h
+++ clangd/index/FileIndex.h
@@ -19,6 +19,7 @@
 #include "../ClangdUnit.h"
 #include "Index.h"
 #include "MemIndex.h"
+#include "clang/Lex/Preprocessor.h"
 
 namespace clang {
 namespace clangd {
@@ -56,8 +57,10 @@
 class FileIndex : public SymbolIndex {
 public:
   /// \brief Update symbols in \p Path with symbols in \p AST. If \p AST is
-  /// nullptr, this removes all symbols in the file
-  void update(PathRef Path, ParsedAST *AST);
+  /// nullptr, this removes all symbols in the file.
+  /// If \p AST is not null, \p PP cannot be null and it should be the
+  /// preprocessor that was used to build \p AST.
+  void update(PathRef Path, ASTContext *AST, std::shared_ptr PP);
 
   bool
   fuzzyFind(const FuzzyFindRequest ,
@@ -73,7 +76,7 @@
 
 /// Retrieves namespace and class level symbols in \p AST.
 /// Exposed to assist in unit tests.
-SymbolSlab indexAST(ParsedAST *AST);
+SymbolSlab indexAST(ASTContext , std::shared_ptr PP);
 
 } // namespace clangd
 } // namespace clang
Index: clangd/index/FileIndex.cpp
===
--- clangd/index/FileIndex.cpp
+++ clangd/index/FileIndex.cpp
@@ -10,12 +10,12 @@
 #include "FileIndex.h"
 #include "SymbolCollector.h"
 #include "clang/Index/IndexingAction.h"
+#include "clang/Lex/Preprocessor.h"
 
 namespace clang {
 namespace clangd {
 
-SymbolSlab indexAST(ParsedAST *AST) {
-  assert(AST && "AST 

[PATCH] D47260: Testcase for dwarf 5 crash/assert when calculating a checksum for an expansion

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

Once we encounter a # directive we are (most likely) looking at some form of 
preprocessed source and that means that the checksum will inevitably be 
different than what we would have gotten were we reading the file directly, 
because of the preprocessing. At this point the value of the hash approaches 
zero. I think dropping all checksums is reasonable in that situation.


Repository:
  rC Clang

https://reviews.llvm.org/D47260



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

[PATCH] D47272: [clangd] Build index on preamble changes instead of the AST changes

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

@malaperle, a slight offtopic. I was wondering which completion options do you 
use for clangd? Defaults?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D47272



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

r333110 - [X86] Move all Intel defined intrinsic includes into immintrin.h

[Craig Topper via cfe-commits]

Author: ctopper
Date: Wed May 23 11:32:58 2018
New Revision: 333110

URL: http://llvm.org/viewvc/llvm-project?rev=333110=rev
Log:
[X86] Move all Intel defined intrinsic includes into immintrin.h

This matches the Intel documentation which shows them available by importing 
immintrin.h. x86intrin.h also includes immintrin.h so anyone including 
x86intrin.h will still get them.

This is different than gcc, but I don't think we were a perfect match there 
already. I'm unclear what gcc's policy is about how they choose which to add 
things to.

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

Modified:
cfe/trunk/lib/Headers/cldemoteintrin.h
cfe/trunk/lib/Headers/clzerointrin.h
cfe/trunk/lib/Headers/immintrin.h
cfe/trunk/lib/Headers/movdirintrin.h
cfe/trunk/lib/Headers/pconfigintrin.h
cfe/trunk/lib/Headers/ptwriteintrin.h
cfe/trunk/lib/Headers/rdseedintrin.h
cfe/trunk/lib/Headers/sgxintrin.h
cfe/trunk/lib/Headers/waitpkgintrin.h
cfe/trunk/lib/Headers/wbnoinvdintrin.h
cfe/trunk/lib/Headers/x86intrin.h

Modified: cfe/trunk/lib/Headers/cldemoteintrin.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/cldemoteintrin.h?rev=333110=333109=333110=diff
==
--- cfe/trunk/lib/Headers/cldemoteintrin.h (original)
+++ cfe/trunk/lib/Headers/cldemoteintrin.h Wed May 23 11:32:58 2018
@@ -21,7 +21,7 @@
  *===---===
  */
 
-#ifndef __X86INTRIN_H
+#if !defined __X86INTRIN_H && !defined __IMMINTRIN_H
 #error "Never use  directly; include  instead."
 #endif
 

Modified: cfe/trunk/lib/Headers/clzerointrin.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/clzerointrin.h?rev=333110=333109=333110=diff
==
--- cfe/trunk/lib/Headers/clzerointrin.h (original)
+++ cfe/trunk/lib/Headers/clzerointrin.h Wed May 23 11:32:58 2018
@@ -20,7 +20,7 @@
  *
  *===---===
  */
-#ifndef __X86INTRIN_H
+#if !defined __X86INTRIN_H && !defined __IMMINTRIN_H
 #error "Never use  directly; include  instead."
 #endif
 

Modified: cfe/trunk/lib/Headers/immintrin.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/immintrin.h?rev=333110=333109=333110=diff
==
--- cfe/trunk/lib/Headers/immintrin.h (original)
+++ cfe/trunk/lib/Headers/immintrin.h Wed May 23 11:32:58 2018
@@ -90,6 +90,10 @@
 #include 
 #endif
 
+#if !defined(_MSC_VER) || __has_feature(modules) || defined(__POPCNT__)
+#include 
+#endif
+
 #if !defined(_MSC_VER) || __has_feature(modules) || defined(__FMA__)
 #include 
 #endif
@@ -339,4 +343,41 @@ _writegsbase_u64(unsigned long long __V)
  * whereas others are also available at all times. */
 #include 
 
+#if !defined(_MSC_VER) || __has_feature(modules) || defined(__RDSEED__)
+#include 
+#endif
+
+#if !defined(_MSC_VER) || __has_feature(modules) || defined(__CLZERO__)
+#include 
+#endif
+
+#if !defined(_MSC_VER) || __has_feature(modules) || defined(__WBNOINVD__)
+#include 
+#endif
+
+#if !defined(_MSC_VER) || __has_feature(modules) || defined(__CLDEMOTE__)
+#include 
+#endif
+
+#if !defined(_MSC_VER) || __has_feature(modules) || defined(__WAITPKG__)
+#include 
+#endif
+
+#if !defined(_MSC_VER) || __has_feature(modules) || \
+  defined(__MOVDIRI__) || defined(__MOVDIR64B__)
+#include 
+#endif
+
+#if !defined(_MSC_VER) || __has_feature(modules) || defined(__PCONFIG__)
+#include 
+#endif
+
+#if !defined(_MSC_VER) || __has_feature(modules) || defined(__SGX__)
+#include 
+#endif
+
+#if !defined(_MSC_VER) || __has_feature(modules) || defined(__PTWRITE__)
+#include 
+#endif
+
 #endif /* __IMMINTRIN_H */

Modified: cfe/trunk/lib/Headers/movdirintrin.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/movdirintrin.h?rev=333110=333109=333110=diff
==
--- cfe/trunk/lib/Headers/movdirintrin.h (original)
+++ cfe/trunk/lib/Headers/movdirintrin.h Wed May 23 11:32:58 2018
@@ -20,7 +20,7 @@
  *
  *===---===
  */
-#ifndef __X86INTRIN_H
+#if !defined __X86INTRIN_H && !defined __IMMINTRIN_H
 #error "Never use  directly; include  instead."
 #endif
 

Modified: cfe/trunk/lib/Headers/pconfigintrin.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/pconfigintrin.h?rev=333110=333109=333110=diff
==
--- cfe/trunk/lib/Headers/pconfigintrin.h (original)
+++ cfe/trunk/lib/Headers/pconfigintrin.h Wed May 23 11:32:58 2018
@@ -21,7 +21,7 @@
  *===---===
  */
 
-#ifndef __X86INTRIN_H
+#if !defined __X86INTRIN_H && !defined 

[PATCH] D47272: [clangd] Build index on preamble changes instead of the AST changes

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

malaperle added a comment.

In https://reviews.llvm.org/D47272#1109914, @ilya-biryukov wrote:

> In https://reviews.llvm.org/D47272#1109909, @malaperle wrote:
>
> > > We do not to rely on symbols from the main file anyway, since any info 
> > > hat those provide can always be taken from the AST.
> >
> > I'll be adding those soon for workspace symbols... And also for document 
> > symbols.
>
>
> I can add extra code to build pieces for the AST later. This is not hard to 
> do, but would require rearranging some code a bit more.
>  Will try to send the change tomorrow for review tomorrow. Does that SG?


Sounds good. Doesn't have to be tomorrow :) Just making sure we're not on 
incompatible paths.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D47272



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

[PATCH] D47182: [X86] Move all Intel defined intrinsic includes into immintrin.h

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL333110: [X86] Move all Intel defined intrinsic includes into 
immintrin.h (authored by ctopper, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D47182?vs=148252=148259#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D47182

Files:
  cfe/trunk/lib/Headers/cldemoteintrin.h
  cfe/trunk/lib/Headers/clzerointrin.h
  cfe/trunk/lib/Headers/immintrin.h
  cfe/trunk/lib/Headers/movdirintrin.h
  cfe/trunk/lib/Headers/pconfigintrin.h
  cfe/trunk/lib/Headers/ptwriteintrin.h
  cfe/trunk/lib/Headers/rdseedintrin.h
  cfe/trunk/lib/Headers/sgxintrin.h
  cfe/trunk/lib/Headers/waitpkgintrin.h
  cfe/trunk/lib/Headers/wbnoinvdintrin.h
  cfe/trunk/lib/Headers/x86intrin.h

Index: cfe/trunk/lib/Headers/wbnoinvdintrin.h
===
--- cfe/trunk/lib/Headers/wbnoinvdintrin.h
+++ cfe/trunk/lib/Headers/wbnoinvdintrin.h
@@ -21,7 +21,7 @@
  *===---===
  */
 
-#ifndef __X86INTRIN_H
+#if !defined __X86INTRIN_H && !defined __IMMINTRIN_H
 #error "Never use  directly; include  instead."
 #endif
 
Index: cfe/trunk/lib/Headers/waitpkgintrin.h
===
--- cfe/trunk/lib/Headers/waitpkgintrin.h
+++ cfe/trunk/lib/Headers/waitpkgintrin.h
@@ -20,7 +20,7 @@
  *
  *===---===
  */
-#ifndef __X86INTRIN_H
+#if !defined __X86INTRIN_H && !defined __IMMINTRIN_H
 #error "Never use  directly; include  instead."
 #endif
 
Index: cfe/trunk/lib/Headers/movdirintrin.h
===
--- cfe/trunk/lib/Headers/movdirintrin.h
+++ cfe/trunk/lib/Headers/movdirintrin.h
@@ -20,7 +20,7 @@
  *
  *===---===
  */
-#ifndef __X86INTRIN_H
+#if !defined __X86INTRIN_H && !defined __IMMINTRIN_H
 #error "Never use  directly; include  instead."
 #endif
 
Index: cfe/trunk/lib/Headers/pconfigintrin.h
===
--- cfe/trunk/lib/Headers/pconfigintrin.h
+++ cfe/trunk/lib/Headers/pconfigintrin.h
@@ -21,7 +21,7 @@
  *===---===
  */
 
-#ifndef __X86INTRIN_H
+#if !defined __X86INTRIN_H && !defined __IMMINTRIN_H
 #error "Never use  directly; include  instead."
 #endif
 
Index: cfe/trunk/lib/Headers/immintrin.h
===
--- cfe/trunk/lib/Headers/immintrin.h
+++ cfe/trunk/lib/Headers/immintrin.h
@@ -90,6 +90,10 @@
 #include 
 #endif
 
+#if !defined(_MSC_VER) || __has_feature(modules) || defined(__POPCNT__)
+#include 
+#endif
+
 #if !defined(_MSC_VER) || __has_feature(modules) || defined(__FMA__)
 #include 
 #endif
@@ -339,4 +343,41 @@
  * whereas others are also available at all times. */
 #include 
 
+#if !defined(_MSC_VER) || __has_feature(modules) || defined(__RDSEED__)
+#include 
+#endif
+
+#if !defined(_MSC_VER) || __has_feature(modules) || defined(__CLZERO__)
+#include 
+#endif
+
+#if !defined(_MSC_VER) || __has_feature(modules) || defined(__WBNOINVD__)
+#include 
+#endif
+
+#if !defined(_MSC_VER) || __has_feature(modules) || defined(__CLDEMOTE__)
+#include 
+#endif
+
+#if !defined(_MSC_VER) || __has_feature(modules) || defined(__WAITPKG__)
+#include 
+#endif
+
+#if !defined(_MSC_VER) || __has_feature(modules) || \
+  defined(__MOVDIRI__) || defined(__MOVDIR64B__)
+#include 
+#endif
+
+#if !defined(_MSC_VER) || __has_feature(modules) || defined(__PCONFIG__)
+#include 
+#endif
+
+#if !defined(_MSC_VER) || __has_feature(modules) || defined(__SGX__)
+#include 
+#endif
+
+#if !defined(_MSC_VER) || __has_feature(modules) || defined(__PTWRITE__)
+#include 
+#endif
+
 #endif /* __IMMINTRIN_H */
Index: cfe/trunk/lib/Headers/ptwriteintrin.h
===
--- cfe/trunk/lib/Headers/ptwriteintrin.h
+++ cfe/trunk/lib/Headers/ptwriteintrin.h
@@ -21,7 +21,7 @@
  *===---===
  */
 
-#ifndef __X86INTRIN_H
+#if !defined __X86INTRIN_H && !defined __IMMINTRIN_H
 #error "Never use  directly; include  instead."
 #endif
 
Index: cfe/trunk/lib/Headers/sgxintrin.h
===
--- cfe/trunk/lib/Headers/sgxintrin.h
+++ cfe/trunk/lib/Headers/sgxintrin.h
@@ -21,7 +21,7 @@
  *===---===
  */
 
-#ifndef __X86INTRIN_H
+#if !defined __X86INTRIN_H && !defined __IMMINTRIN_H
 #error "Never use  directly; include  instead."
 #endif
 
Index: cfe/trunk/lib/Headers/cldemoteintrin.h
===
--- 

[PATCH] D47272: [clangd] Build index on preamble changes instead of the AST changes

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In https://reviews.llvm.org/D47272#1109909, @malaperle wrote:

> > We do not to rely on symbols from the main file anyway, since any info hat 
> > those provide can always be taken from the AST.
>
> I'll be adding those soon for workspace symbols... And also for document 
> symbols.


I can add extra code to build pieces for the AST later. This is not hard to do, 
but would require rearranging some code a bit more.
Will try to send the change tomorrow for review tomorrow. Does that SG?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D47272



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

[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

> Actually, scratch that. We will be enabling it since GCC does. Will update 
> this and other relevant C++ related code appropriately.

Could you also add tests which mix _Accum with volatile, _Atomic, _Complex, 
constexpr, inline?


Repository:
  rC Clang

https://reviews.llvm.org/D46084



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

[PATCH] D47267: [UnrollAndJam] Add unroll_and_jam pragma handling

[Dave Green via Phabricator via cfe-commits]

dmgreen added a comment.

> In my experience, they are used.

Good to know, cheers.

> Could we maybe disable the #pragma clang loop unroll_and_jam spelling ftm to 
> avoid compatibility issues?

Sure, I'm not against. It sounds like you have opinions on how this should 
work. That's good. If there are multiple clang loop pragma's, what is the 
expected behaviour there?

In the llvm side of this, in the unroll and jam pass, I made it so that a loop 
with "llvm.loop.unroll" metadata without any "llvm.loop.unroll_and_jam" 
metadata will not do unroll and jam, it will leave the loop for the unroller. 
On the expectation that the use really wants to unroll (and it applies to 
llvm.loop.unroll.disable too, disabling unroll and jam as well as unroll). I 
haven't done anything with other loop metadata though.


https://reviews.llvm.org/D47267



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

[PATCH] D47272: [clangd] Build index on preamble changes instead of the AST changes

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

malaperle added a comment.

> We do not to rely on symbols from the main file anyway, since any info hat 
> those provide can always be taken from the AST.

I'll be adding those soon for workspace symbols... And also for document 
symbols.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D47272



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

[PATCH] D47273: [bash-completion] Fix tab separation on macOS

[Ben Langmuir via Phabricator via cfe-commits]

benlangmuir created this revision.
benlangmuir added reviewers: yamaguchi, v.g.vassilev, ruiu, teemperor.
Herald added a subscriber: cfe-commits.

We have a regex that needs to match a tab character in the command output, but 
on macOS `sed` doesn't support '\t', causing it to split on the 't' character 
instead. Some options:

- use `perl -p -e 's/\t.*//'`, which does handle '\t'
- put a literal tab character in the string passed to sed
- use `sed -e $'s/\t.*//'`, which will cause bash to expand the '\t' before 
passing it to sed


Repository:
  rC Clang

https://reviews.llvm.org/D47273

Files:
  utils/bash-autocomplete.sh


Index: utils/bash-autocomplete.sh
===
--- utils/bash-autocomplete.sh
+++ utils/bash-autocomplete.sh
@@ -38,7 +38,7 @@
 
   # expand ~ to $HOME
   eval local path=${COMP_WORDS[0]}
-  flags=$( "$path" --autocomplete="$arg" 2>/dev/null | sed -e 's/\t.*//' )
+  flags=$( "$path" --autocomplete="$arg" 2>/dev/null | perl -p -e 's/\t.*//' )
   # If clang is old that it does not support --autocomplete,
   # fall back to the filename completion.
   if [[ "$?" != 0 ]]; then


Index: utils/bash-autocomplete.sh
===
--- utils/bash-autocomplete.sh
+++ utils/bash-autocomplete.sh
@@ -38,7 +38,7 @@
 
   # expand ~ to $HOME
   eval local path=${COMP_WORDS[0]}
-  flags=$( "$path" --autocomplete="$arg" 2>/dev/null | sed -e 's/\t.*//' )
+  flags=$( "$path" --autocomplete="$arg" 2>/dev/null | perl -p -e 's/\t.*//' )
   # If clang is old that it does not support --autocomplete,
   # fall back to the filename completion.
   if [[ "$?" != 0 ]]; then
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47272: [clangd] Build index on preamble changes instead of the AST changes

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added reviewers: ioeric, sammccall.
Herald added subscribers: jkorous, MaskRay, javed.absar, klimek.

This is more efficient and avoids data races when reading files that
come from the preamble. The staleness can occur when reading a file
from disk that changed after the preamble was built. This can lead to
crashes, e.g. when parsing comments.

We do not to rely on symbols from the main file anyway, since any info
that those provide can always be taken from the AST.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D47272

Files:
  clangd/ClangdServer.cpp
  clangd/ClangdUnit.cpp
  clangd/ClangdUnit.h
  clangd/TUScheduler.cpp
  clangd/TUScheduler.h
  clangd/index/FileIndex.cpp
  clangd/index/FileIndex.h
  unittests/clangd/FileIndexTests.cpp
  unittests/clangd/TestTU.cpp

Index: unittests/clangd/TestTU.cpp
===
--- unittests/clangd/TestTU.cpp
+++ unittests/clangd/TestTU.cpp
@@ -43,7 +43,7 @@
 
 SymbolSlab TestTU::headerSymbols() const {
   auto AST = build();
-  return indexAST();
+  return indexAST(AST.getASTContext(), AST.getPreprocessorPtr());
 }
 
 std::unique_ptr TestTU::index() const {
Index: unittests/clangd/FileIndexTests.cpp
===
--- unittests/clangd/FileIndexTests.cpp
+++ unittests/clangd/FileIndexTests.cpp
@@ -87,7 +87,7 @@
   File.HeaderFilename = (Basename + ".h").str();
   File.HeaderCode = Code;
   auto AST = File.build();
-  M.update(File.Filename, );
+  M.update(File.Filename, (), AST.getPreprocessorPtr());
 }
 
 TEST(FileIndexTest, IndexAST) {
@@ -129,13 +129,13 @@
   Req.Scopes = {"ns::"};
   EXPECT_THAT(match(M, Req), UnorderedElementsAre("ns::f", "ns::X"));
 
-  M.update("f1.cpp", nullptr);
+  M.update("f1.cpp", nullptr, nullptr);
   EXPECT_THAT(match(M, Req), UnorderedElementsAre());
 }
 
 TEST(FileIndexTest, RemoveNonExisting) {
   FileIndex M;
-  M.update("no.cpp", nullptr);
+  M.update("no.cpp", nullptr, nullptr);
   EXPECT_THAT(match(M, FuzzyFindRequest()), UnorderedElementsAre());
 }
 
Index: clangd/index/FileIndex.h
===
--- clangd/index/FileIndex.h
+++ clangd/index/FileIndex.h
@@ -19,6 +19,7 @@
 #include "../ClangdUnit.h"
 #include "Index.h"
 #include "MemIndex.h"
+#include "clang/Lex/Preprocessor.h"
 
 namespace clang {
 namespace clangd {
@@ -56,8 +57,10 @@
 class FileIndex : public SymbolIndex {
 public:
   /// \brief Update symbols in \p Path with symbols in \p AST. If \p AST is
-  /// nullptr, this removes all symbols in the file
-  void update(PathRef Path, ParsedAST *AST);
+  /// nullptr, this removes all symbols in the file.
+  /// If \p AST is not null, \p PP cannot be null and it should be the
+  /// preprocessor that was used to build \p AST.
+  void update(PathRef Path, ASTContext *AST, std::shared_ptr PP);
 
   bool
   fuzzyFind(const FuzzyFindRequest ,
@@ -73,7 +76,7 @@
 
 /// Retrieves namespace and class level symbols in \p AST.
 /// Exposed to assist in unit tests.
-SymbolSlab indexAST(ParsedAST *AST);
+SymbolSlab indexAST(ASTContext , std::shared_ptr PP);
 
 } // namespace clangd
 } // namespace clang
Index: clangd/index/FileIndex.cpp
===
--- clangd/index/FileIndex.cpp
+++ clangd/index/FileIndex.cpp
@@ -10,12 +10,12 @@
 #include "FileIndex.h"
 #include "SymbolCollector.h"
 #include "clang/Index/IndexingAction.h"
+#include "clang/Lex/Preprocessor.h"
 
 namespace clang {
 namespace clangd {
 
-SymbolSlab indexAST(ParsedAST *AST) {
-  assert(AST && "AST must not be nullptr!");
+SymbolSlab indexAST(ASTContext , std::shared_ptr PP) {
   SymbolCollector::Options CollectorOpts;
   // FIXME(ioeric): we might also want to collect include headers. We would need
   // to make sure all includes are canonicalized (with CanonicalIncludes), which
@@ -26,15 +26,18 @@
   CollectorOpts.CountReferences = false;
 
   SymbolCollector Collector(std::move(CollectorOpts));
-  Collector.setPreprocessor(AST->getPreprocessorPtr());
+  Collector.setPreprocessor(PP);
   index::IndexingOptions IndexOpts;
   // We only need declarations, because we don't count references.
   IndexOpts.SystemSymbolFilter =
   index::IndexingOptions::SystemSymbolFilterKind::DeclarationsOnly;
   IndexOpts.IndexFunctionLocals = false;
 
-  index::indexTopLevelDecls(AST->getASTContext(), AST->getTopLevelDecls(),
-Collector, IndexOpts);
+  std::vector TopLevelDecls(
+  AST.getTranslationUnitDecl()->decls().begin(),
+  AST.getTranslationUnitDecl()->decls().end());
+  index::indexTopLevelDecls(AST, TopLevelDecls, Collector, IndexOpts);
+
   return Collector.takeSymbols();
 }
 
@@ -69,12 +72,14 @@
   return {std::move(Snap), Pointers};
 }
 
-void FileIndex::update(PathRef Path, ParsedAST *AST) {
+void FileIndex::update(PathRef Path, ASTContext *AST,
+   

[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added inline comments.



Comment at: include/clang/Basic/DiagnosticCommonKinds.td:172
+def err_fixed_point_only_allowed_in_c : Error<
+  "Fixed point types are only allowed in C">;
 

leonardchan wrote:
> rsmith wrote:
> > Diagnostics should not be capitalized. Also, we generally allow conforming 
> > C extensions to be used in other languages unless there is a really good 
> > reason not to.
> We decided not to allow fixed point types in other languages because there is 
> no specification provided in N1169 for addressing some features in other 
> languages. Using C++ as an example, N1169 does not provide recommended 
> characters when name mangling so we do not allow this in C++.
Actually, scratch that. We will be enabling it since GCC does. Will update this 
and other relevant C++ related code appropriately.


Repository:
  rC Clang

https://reviews.llvm.org/D46084



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

[PATCH] D47182: [X86] Move all Intel defined intrinsic includes into immintrin.h

[David Kreitzer via Phabricator via cfe-commits]

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

Thanks, Craig. LGTM.


https://reviews.llvm.org/D47182



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

[PATCH] D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Somewhat tangential, in discussion with Duncan he mentioned that `-nostdinc++` 
should turn off assumptions about old Darwin. So if you build libc++ yourself, 
you don't care what does the system stdlib support. I agree with that and think 
it doesn't interfere with the latest proposal but adds more to it.


Repository:
  rC Clang

https://reviews.llvm.org/D45015



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

[PATCH] D47182: [X86] Move all Intel defined intrinsic includes into immintrin.h

[Craig Topper via Phabricator via cfe-commits]

craig.topper updated this revision to Diff 148252.
craig.topper added a comment.

Add back popcntintrin.h


https://reviews.llvm.org/D47182

Files:
  lib/Headers/cldemoteintrin.h
  lib/Headers/clzerointrin.h
  lib/Headers/immintrin.h
  lib/Headers/movdirintrin.h
  lib/Headers/pconfigintrin.h
  lib/Headers/ptwriteintrin.h
  lib/Headers/rdseedintrin.h
  lib/Headers/sgxintrin.h
  lib/Headers/waitpkgintrin.h
  lib/Headers/wbnoinvdintrin.h
  lib/Headers/x86intrin.h

Index: lib/Headers/x86intrin.h
===
--- lib/Headers/x86intrin.h
+++ lib/Headers/x86intrin.h
@@ -32,26 +32,6 @@
 #include 
 #endif
 
-#if !defined(_MSC_VER) || __has_feature(modules) || defined(__BMI__)
-#include 
-#endif
-
-#if !defined(_MSC_VER) || __has_feature(modules) || defined(__BMI2__)
-#include 
-#endif
-
-#if !defined(_MSC_VER) || __has_feature(modules) || defined(__LZCNT__)
-#include 
-#endif
-
-#if !defined(_MSC_VER) || __has_feature(modules) || defined(__POPCNT__)
-#include 
-#endif
-
-#if !defined(_MSC_VER) || __has_feature(modules) || defined(__RDSEED__)
-#include 
-#endif
-
 #if !defined(_MSC_VER) || __has_feature(modules) || defined(__PRFCHW__)
 #include 
 #endif
@@ -76,45 +56,8 @@
 #include 
 #endif
 
-#if !defined(_MSC_VER) || __has_feature(modules) || defined(__F16C__)
-#include 
-#endif
-
 #if !defined(_MSC_VER) || __has_feature(modules) || defined(__MWAITX__)
 #include 
 #endif
 
-#if !defined(_MSC_VER) || __has_feature(modules) || defined(__CLZERO__)
-#include 
-#endif
-
-#if !defined(_MSC_VER) || __has_feature(modules) || defined(__WBNOINVD__)
-#include 
-#endif
-
-#if !defined(_MSC_VER) || __has_feature(modules) || defined(__CLDEMOTE__)
-#include 
-#endif
-
-#if !defined(_MSC_VER) || __has_feature(modules) || defined(__WAITPKG__)
-#include 
-#endif
-
-#if !defined(_MSC_VER) || __has_feature(modules) || \
-  defined(__MOVDIRI__) || defined(__MOVDIR64B__)
-#include 
-#endif
-
-#if !defined(_MSC_VER) || __has_feature(modules) || defined(__PCONFIG__)
-#include 
-#endif
-
-#if !defined(_MSC_VER) || __has_feature(modules) || defined(__SGX__)
-#include 
-#endif
-
-#if !defined(_MSC_VER) || __has_feature(modules) || defined(__PTWRITE__)
-#include 
-#endif
-
 #endif /* __X86INTRIN_H */
Index: lib/Headers/wbnoinvdintrin.h
===
--- lib/Headers/wbnoinvdintrin.h
+++ lib/Headers/wbnoinvdintrin.h
@@ -21,7 +21,7 @@
  *===---===
  */
 
-#ifndef __X86INTRIN_H
+#if !defined __X86INTRIN_H && !defined __IMMINTRIN_H
 #error "Never use  directly; include  instead."
 #endif
 
Index: lib/Headers/waitpkgintrin.h
===
--- lib/Headers/waitpkgintrin.h
+++ lib/Headers/waitpkgintrin.h
@@ -20,7 +20,7 @@
  *
  *===---===
  */
-#ifndef __X86INTRIN_H
+#if !defined __X86INTRIN_H && !defined __IMMINTRIN_H
 #error "Never use  directly; include  instead."
 #endif
 
Index: lib/Headers/sgxintrin.h
===
--- lib/Headers/sgxintrin.h
+++ lib/Headers/sgxintrin.h
@@ -21,7 +21,7 @@
  *===---===
  */
 
-#ifndef __X86INTRIN_H
+#if !defined __X86INTRIN_H && !defined __IMMINTRIN_H
 #error "Never use  directly; include  instead."
 #endif
 
Index: lib/Headers/rdseedintrin.h
===
--- lib/Headers/rdseedintrin.h
+++ lib/Headers/rdseedintrin.h
@@ -21,7 +21,7 @@
  *===---===
  */
 
-#ifndef __X86INTRIN_H
+#if !defined __X86INTRIN_H && !defined __IMMINTRIN_H
 #error "Never use  directly; include  instead."
 #endif
 
Index: lib/Headers/ptwriteintrin.h
===
--- lib/Headers/ptwriteintrin.h
+++ lib/Headers/ptwriteintrin.h
@@ -21,7 +21,7 @@
  *===---===
  */
 
-#ifndef __X86INTRIN_H
+#if !defined __X86INTRIN_H && !defined __IMMINTRIN_H
 #error "Never use  directly; include  instead."
 #endif
 
Index: lib/Headers/pconfigintrin.h
===
--- lib/Headers/pconfigintrin.h
+++ lib/Headers/pconfigintrin.h
@@ -21,7 +21,7 @@
  *===---===
  */
 
-#ifndef __X86INTRIN_H
+#if !defined __X86INTRIN_H && !defined __IMMINTRIN_H
 #error "Never use  directly; include  instead."
 #endif
 
Index: lib/Headers/movdirintrin.h
===
--- lib/Headers/movdirintrin.h
+++ lib/Headers/movdirintrin.h
@@ -20,7 +20,7 @@
  *
  *===---===
  */
-#ifndef __X86INTRIN_H

[PATCH] D46187: [Analyzer] getRegisteredCheckers(): handle debug checkers too.

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri abandoned this revision.
lebedev.ri added a comment.

Sad, but ok.


Repository:
  rC Clang

https://reviews.llvm.org/D46187



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

[PATCH] D46188: [clang-tidy] Add a flag to enable debug checkers

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri abandoned this revision.
lebedev.ri added a comment.

https://reviews.llvm.org/D46187#1098571
Sad, but ok.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D46188



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

[libcxx] r333108 - Do not define template specialization __libcpp_is_floating_point<__fp16>

[Akira Hatanaka via cfe-commits]

Author: ahatanak
Date: Wed May 23 10:50:41 2018
New Revision: 333108

URL: http://llvm.org/viewvc/llvm-project?rev=333108=rev
Log:
Do not define template specialization __libcpp_is_floating_point<__fp16>
if the compiler is not clang.

gcc doesn't allow using __fp16 on non-ARM targets.

Modified:
libcxx/trunk/include/type_traits
libcxx/trunk/test/libcxx/type_traits/is_floating_point.pass.cpp

Modified: libcxx/trunk/include/type_traits
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/type_traits?rev=333108=333107=333108=diff
==
--- libcxx/trunk/include/type_traits (original)
+++ libcxx/trunk/include/type_traits Wed May 23 10:50:41 2018
@@ -733,7 +733,9 @@ _LIBCPP_INLINE_VAR _LIBCPP_CONSTEXPR boo
 // is_floating_point
 
 template  struct __libcpp_is_floating_point  : public 
false_type {};
+#ifdef __clang__
 template <>  struct __libcpp_is_floating_point<__fp16>  : public 
true_type {};
+#endif
 #ifdef __FLT16_MANT_DIG__
 template <>  struct __libcpp_is_floating_point<_Float16>: public 
true_type {};
 #endif

Modified: libcxx/trunk/test/libcxx/type_traits/is_floating_point.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/libcxx/type_traits/is_floating_point.pass.cpp?rev=333108=333107=333108=diff
==
--- libcxx/trunk/test/libcxx/type_traits/is_floating_point.pass.cpp (original)
+++ libcxx/trunk/test/libcxx/type_traits/is_floating_point.pass.cpp Wed May 23 
10:50:41 2018
@@ -14,7 +14,9 @@
 #include 
 
 int main() {
+#ifdef __clang__
   static_assert(std::is_floating_point<__fp16>::value, "");
+#endif
 #ifdef __FLT16_MANT_DIG__
   static_assert(std::is_floating_point<_Float16>::value, "");
 #endif


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

[PATCH] D46822: [RISCV] Add driver for riscv32-unknown-elf baremetal target

[Mandeep Singh Grang via Phabricator via cfe-commits]

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

LGTM.




Comment at: lib/Driver/ToolChains/Gnu.cpp:1885
+ "riscv64-unknown-linux-gnu",
+ "riscv32-unknown-elf"};
 

Alphabetically riscv32-unknown-elf should be the first in the list of triples.


Repository:
  rC Clang

https://reviews.llvm.org/D46822



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

[clang-tools-extra] r333104 - [Documentation] Fix link syntax in Release Notes.

[Eugene Zelenko via cfe-commits]

Author: eugenezelenko
Date: Wed May 23 10:39:46 2018
New Revision: 333104

URL: http://llvm.org/viewvc/llvm-project?rev=333104=rev
Log:
[Documentation] Fix link syntax in Release Notes.

Modified:
clang-tools-extra/trunk/docs/ReleaseNotes.rst

Modified: clang-tools-extra/trunk/docs/ReleaseNotes.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/ReleaseNotes.rst?rev=333104=333103=333104=diff
==
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst (original)
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst Wed May 23 10:39:46 2018
@@ -100,7 +100,7 @@ Improvements to clang-tidy
   only allowed in nested loops.
 
 - New :doc:`cppcoreguidelines-narrowing-conversions
-  `_ check
+  ` check
 
   Checks for narrowing conversions, e. g. ``int i = 0; i += 0.1;``.
 


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

[PATCH] D47097: [DebugInfo] Preserve scope in auto generated StoreInst

[Vedant Kumar via Phabricator via cfe-commits]

vsk added inline comments.



Comment at: lib/CodeGen/CGExpr.cpp:72
+  // Set debug location in order to preserve the scope
+  Alloca->setDebugLoc(Builder.getCurrentDebugLocation());
   if (AllocaAddr)

I think we need to be a bit more careful here. The current debug location 
stored in the builder may not be an artificial 0-location. This may cause 
non-linear single-stepping behavior. Consider this example:

```
void foo() {
  bar();
  if (...) {
int var = ...; //< Clang emits an alloca for "var".
  }
...
```

The current debug location at the line "int var = ..." would be at line 4. But 
the alloca is emitted in the entry block of the function. In the debugger, this 
may result in strange single-stepping behavior when stepping into foo(). You 
could step to line 4, then line 2, then line 3, then line 4 again.

I think we can avoid that by setting an artificial location on allocas.



Comment at: lib/CodeGen/CGExpr.cpp:105
   return new llvm::AllocaInst(Ty, CGM.getDataLayout().getAllocaAddrSpace(),
   ArraySize, Name, AllocaInsertPt);
 }

Why not apply the location here to cover more cases?



Comment at: test/CodeGen/debug-info-preserve-scope.c:11
+
+// CHECK: [[B:%.*]] = alloca i32 {{.*}} !dbg ![[artificialDbgLoc:[0-9]+]]
+// CHECK: store i32 {{.*}} !dbg ![[artificialDbgLoc]]

Why is "B" captured?


Repository:
  rC Clang

https://reviews.llvm.org/D47097



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

[PATCH] D46867: [ASTImporter] Add unit tests for structural equivalence

[Aleksei Sidorin via Phabricator via cfe-commits]

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

Hi Gabor,

LGTM, thank you for addressing the comments! Just a minor nit, it's OK to fix 
it before committing without a separate review.




Comment at: unittests/AST/StructuralEquivalenceTest.cpp:67
+  bool testStructuralMatch(std::tuple t) {
+using std::get;
+return testStructuralMatch(get<0>(t), get<1>(t));

Not sure we need this using: we can move up the `using` below or just write 
std::get twice.


Repository:
  rC Clang

https://reviews.llvm.org/D46867



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

[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a subscriber: sammccall.
leonardchan added inline comments.



Comment at: include/clang/Basic/DiagnosticCommonKinds.td:172
+def err_fixed_point_only_allowed_in_c : Error<
+  "Fixed point types are only allowed in C">;
 

rsmith wrote:
> Diagnostics should not be capitalized. Also, we generally allow conforming C 
> extensions to be used in other languages unless there is a really good reason 
> not to.
We decided not to allow fixed point types in other languages because there is 
no specification provided in N1169 for addressing some features in other 
languages. Using C++ as an example, N1169 does not provide recommended 
characters when name mangling so we do not allow this in C++.



Comment at: lib/AST/ItaniumMangle.cpp:2552
+  case BuiltinType::ULongAccum:
+llvm_unreachable("Fixed point types are disabled for c++");
   case BuiltinType::Half:

rsmith wrote:
> Please check what GCC uses to mangle these, and follow suit; if GCC doesn't 
> have a mangling, you can use a vendor mangling (`u6_Accum`) or produce an 
> error for now, but please open an issue at 
> https://github.com/itanium-cxx-abi/cxx-abi/ to pick a real mangling.
It seems that GCC uses the characters for each fixed point type's corresponding 
integral type (https://github.com/gcc-mirror/gcc/blob/master/gcc/cp/mangle.c). 
Will follow up on this if we end up enabling fixed point types for C++.



Comment at: lib/CodeGen/CGDebugInfo.cpp:678-681
+  case BuiltinType::UShortAccum:
+  case BuiltinType::UAccum:
+  case BuiltinType::ULongAccum:
 Encoding = llvm::dwarf::DW_ATE_unsigned;

rsmith wrote:
> @echristo @dblaikie Is this appropriate?
My bad, this should be changed to `DW_ATE_signed_fixed` and 
`DW_ATE_unsigned_fixed`



Comment at: lib/Index/USRGeneration.cpp:691
+case BuiltinType::ULongAccum:
+  llvm_unreachable("No USR name mangling for fixed point types.");
 case BuiltinType::Float16:

rsmith wrote:
> leonardchan wrote:
> > phosek wrote:
> > > We need some solution for fixed point types.
> > Added character ~ to indicate fixed point type followed by string detailing 
> > the type. I have not added a test to it because logically, I do not think 
> > we will ever reach that point. This logic is implemented in the VisitType 
> > method, which mostly gets called by visitors to c++ nodes like 
> > VisitTemplateParameterList, but we have disabled the use of fixed point 
> > types in c++. VisitType does get called in VisitFunctionDecl but the 
> > function exits early since we are not reading c++ (line 
> > lib/Index/USRGeneration.cpp:238).
> @rjmccall Is this an acceptable USR encoding? (Is our USR encoding scheme 
> documented anywhere?)
I chatted with @sammccall about this who said it was ok to add these types if 
no one opposed this. I posted this on cfe-dev also and it seemed that no one 
spoke up about it, so I thought this was ok.

I also couldn't find any standard or documentation about reserving characters 
for USR. It doesn't seem that USR is also parsed in any way, so I don't think 
I'm breaking anything (running ninja check-all passes).

And unless we also do enable this for C++, this code actually may not be run 
since this method will not be visited if limited to C.


Repository:
  rC Clang

https://reviews.llvm.org/D46084



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

r333094 - Test Commit. Fix namespace comment

[Mikhail R. Gadelha via cfe-commits]

Author: mramalho
Date: Wed May 23 08:49:12 2018
New Revision: 333094

URL: http://llvm.org/viewvc/llvm-project?rev=333094=rev
Log:
Test Commit. Fix namespace comment

Signed-off-by: Mikhail Ramalho 

Modified:

cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SimpleConstraintManager.h

Modified: 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SimpleConstraintManager.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SimpleConstraintManager.h?rev=333094=333093=333094=diff
==
--- 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SimpleConstraintManager.h
 (original)
+++ 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SimpleConstraintManager.h
 Wed May 23 08:49:12 2018
@@ -85,8 +85,8 @@ private:
 bool Assumption);
 };
 
-} // end GR namespace
+} // end namespace ento
 
-} // end clang namespace
+} // end namespace clang
 
 #endif


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

[libcxx] r333103 - Teach __libcpp_is_floating_point that __fp16 and _Float16 are

[Akira Hatanaka via cfe-commits]

Author: ahatanak
Date: Wed May 23 10:31:09 2018
New Revision: 333103

URL: http://llvm.org/viewvc/llvm-project?rev=333103=rev
Log:
Teach __libcpp_is_floating_point that __fp16 and _Float16 are
floating-point types.

rdar://problem/40377353

Added:
libcxx/trunk/test/libcxx/type_traits/is_floating_point.pass.cpp
Modified:
libcxx/trunk/include/type_traits

Modified: libcxx/trunk/include/type_traits
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/type_traits?rev=333103=333102=333103=diff
==
--- libcxx/trunk/include/type_traits (original)
+++ libcxx/trunk/include/type_traits Wed May 23 10:31:09 2018
@@ -733,6 +733,10 @@ _LIBCPP_INLINE_VAR _LIBCPP_CONSTEXPR boo
 // is_floating_point
 
 template  struct __libcpp_is_floating_point  : public 
false_type {};
+template <>  struct __libcpp_is_floating_point<__fp16>  : public 
true_type {};
+#ifdef __FLT16_MANT_DIG__
+template <>  struct __libcpp_is_floating_point<_Float16>: public 
true_type {};
+#endif
 template <>  struct __libcpp_is_floating_point   : public 
true_type {};
 template <>  struct __libcpp_is_floating_point  : public 
true_type {};
 template <>  struct __libcpp_is_floating_point : public 
true_type {};

Added: libcxx/trunk/test/libcxx/type_traits/is_floating_point.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/libcxx/type_traits/is_floating_point.pass.cpp?rev=333103=auto
==
--- libcxx/trunk/test/libcxx/type_traits/is_floating_point.pass.cpp (added)
+++ libcxx/trunk/test/libcxx/type_traits/is_floating_point.pass.cpp Wed May 23 
10:31:09 2018
@@ -0,0 +1,22 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// 
+//
+// Test that is_floating_point::value is true when T=__fp16 or T=_Float16.
+
+#include 
+
+int main() {
+  static_assert(std::is_floating_point<__fp16>::value, "");
+#ifdef __FLT16_MANT_DIG__
+  static_assert(std::is_floating_point<_Float16>::value, "");
+#endif
+  return 0;
+}


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

[PATCH] D46944: [analyzer] Use sufficiently large types for index/size calculation.

[Aleksei Sidorin via Phabricator via cfe-commits]

a.sidorin added a comment.

Hi Bevin,

Could you please address these comments?




Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h:89
 SymMgr(context, BasicVals, alloc), MemMgr(context, alloc),
-StateMgr(stateMgr), ArrayIndexTy(context.LongLongTy),
+StateMgr(stateMgr), ArrayIndexTy(context.getSignedSizeType()),
 ArrayIndexWidth(context.getTypeSize(ArrayIndexTy)) {}

As you correctly pointed, ssize_t is 32-bit on 32-bit systems. Therefore, it is 
too short. So, we can leave this line as-is.



Comment at: test/Analysis/index-type.c:13
   char arr[X86_ARRAY_SIZE];
-  char *ptr = arr + UINT_MAX/2;
+  char *ptr = arr + UINT_MAX/4;
   ptr += 2;  // index shouldn't overflow

We don't need to fix the test - it is correct. We have to fix the type instead.



Comment at: test/Analysis/index-type.c:25
+void testOutOfBounds() {
+  // not out of bounds
+  buf[SIZE-1] = 1; // no-warning

The comments should be normal sentences: "Not out of bounds."


https://reviews.llvm.org/D46944



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

[clang-tools-extra] r333100 - [Documentation] Move some Clang-tidy changes to proper sections.

[Eugene Zelenko via cfe-commits]

Author: eugenezelenko
Date: Wed May 23 10:25:22 2018
New Revision: 333100

URL: http://llvm.org/viewvc/llvm-project?rev=333100=rev
Log:
[Documentation] Move some Clang-tidy changes to proper sections.

Modified:
clang-tools-extra/trunk/docs/ReleaseNotes.rst

Modified: clang-tools-extra/trunk/docs/ReleaseNotes.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/ReleaseNotes.rst?rev=333100=333099=333100=diff
==
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst (original)
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst Wed May 23 10:25:22 2018
@@ -99,10 +99,10 @@ Improvements to clang-tidy
   with looping constructs. Every backward jump is rejected. Forward jumps are
   only allowed in nested loops.
 
-- New alias :doc:`fuchsia-header-anon-namespaces
-  ` to 
:doc:`google-build-namespaces
-  `
-  added.
+- New :doc:`cppcoreguidelines-narrowing-conversions
+  `_ check
+
+  Checks for narrowing conversions, e. g. ``int i = 0; i += 0.1;``.
 
 - New :doc:`fuchsia-multiple-inheritance
   ` check.
@@ -180,6 +180,11 @@ Improvements to clang-tidy
 - The `AnalyzeTemporaryDtors` option was removed, since the corresponding
   `cfg-temporary-dtors` option of the Static Analyzer now defaults to `true`.
 
+- New alias :doc:`fuchsia-header-anon-namespaces
+  ` to 
:doc:`google-build-namespaces
+  `
+  added.
+
 - New alias :doc:`hicpp-avoid-goto
   ` to :doc:`cppcoreguidelines-avoid-goto
   `
@@ -241,11 +246,6 @@ Improvements to clang-tidy
 
 - The 'google-runtime-member-string-references' check was removed.
 
-- New `cppcoreguidelines-narrowing-conversions
-  
`_
 check
-
-  Checks for narrowing conversions, e.g. ``int i = 0; i += 0.1;``.
-
 Improvements to include-fixer
 -
 


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

[PATCH] D46943: [clangd] Boost scores for decls from current file in completion

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In https://reviews.llvm.org/D46943#1109199, @ioeric wrote:

> > - Symbols store their paths as URIs ⇒ we need to parse them in order to 
> > apply heuristics. We could avoid that by writing a version of 
> > header-matching that also works on URIs, but that would mean more 
> > complexity.
>
> Yeah, this is a bit unfortunate. It's probably OK to parse URIs; they are not 
> that expensive after all (we might want to do some measurement though).


Yeah. I really wish we had microbenchmarks for things like completion.

>> - Merging quality signals from headers now requires an extra paramater: name 
>> of the source file. I wonder if it's worth extracting builders for symbol 
>> qualities into separate classes to keep the current nice signatures, i.e. 
>> `merge(Symbol& IndexResult)`.
> 
> I'm not very familiar with `SymbolQualitySignals`. But if we decide to use 
> main file name as a signal, it might make sense to pass it in through the 
> constructor?

That's possible, but that essentially turns `SymbolQualitySignals` from a data 
class to a stateful builder.

>> - How should we match the header with the main file?  Our options are:
>>   - (proposed by Eric) use main file regex from clang-format for that. I'm 
>> not entirely sure it suits us well, since the regex is designed to work on 
>> paths inside #include directive, but we're getting ours from the Symbols and 
>> Sema AST Decls. Moreover, it means we're gonna read .clang-format files to 
>> get that style.
> 
> I think the ".clang-format problem" is not specific to the header matching 
> here. We would eventually need proper format style support in clangd anyway, 
> as clangd provides formatting features (e.g. reformat and include insertion).

Yeah, `IncludeMainRegex` does look like a useful setting from clang-format. And 
maybe using clang-format settings is a good idea there. I'm just a bit weary of 
adding this logic in this change along with the completion changes.
So I'd go with a simple heuristic for starters to solve a problem at hand. 
Happy to improve it to use clang-format regex with a follow-up change if 
everyone agrees that's a good idea. I mostly feel uneasy about the added 
complexity to the current change.




Comment at: clangd/MatchingHeader.cpp:44
+  auto HeaderNoExt = llvm::sys::path::filename(Header);
+  return SourceNoExt.equals_lower(HeaderNoExt);
+}

ioeric wrote:
> Why `equals_lower`?
A former-windows-developer habbit. I don't think it hurts in any way if we do 
`equals_lower` here, we'll merely work in those strange cases where the 
extensions are not lower-case.
This is also consistent with isHeaderFile/isSourceFile (moved from ClangdServer)




Comment at: clangd/MatchingHeader.h:1
+//===--- MatchingHeader.h - Match source and header files *- 
C++-*-===//
+//

ioeric wrote:
> I wonder if we could merge this into Headers.h
Thanks, I totally forgot we have `Headers.h`.  Will move the helpers there.



Comment at: clangd/MatchingHeader.h:24
+/// header for a \p Source.
+bool isMainHeaderFor(PathRef Header, PathRef Source);
+

ioeric wrote:
> We might want to briefly explain what a matching header is and what the 
> heuristics are.
My idea was to not elaborate before we agree on what those heuristics should be.
Given the heuristics are so simple and there are suggestions to change them, 
documenting the current behavior seems like a bad idea. I'd rather keep them a 
black box for now.
Does that make sense? Maybe add a comment that the heuristics are likely to 
change, so the users shouldn't rely on them too much?



Comment at: clangd/Quality.cpp:28-29
+struct DeclFiles {
+  bool AllDeclsInMainFile = false;
+  bool HasDeclsInMatchingHeader = false;
+};

ioeric wrote:
> Could we merge these two flags into something like 
> `IsDeclaredInMainHeaderOrFile`?
The two flags are built differently.
**Any** decl in the matching header gives a boost. Otherwise, **all** decls 
should be in the main file to get a boost.
The second one is built differently for the reasons outlined in the previous 
comments on why it might not be the best idea to boost completion items that 
have one of the inside the current file:
- It gives inconsistent ranking for different completion points (before/after 
declaration)
- The fact that a function has definition in the current file does not 
necessarily mean I want to call it more often than the others.



Comment at: clangd/Quality.cpp:40
+  assert(MainFile);
+  for (auto *Redecl : D->redecls()) {
+auto Loc = SM.getSpellingLoc(Redecl->getLocation());

ioeric wrote:
> I wonder if it's still necessary to check all `redecls`. Would it be 
> sufficient to check `D`, if `D` is the decl we referencing to?
I don't think it's sufficient. How could we compute the flags that this 
function returns by looking at 

[PATCH] D47267: [UnrollAndJam] Add unroll_and_jam pragma handling

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur added a comment.

This is straightforward in that it clones the implementation of `#pragma 
unroll` for `unroll_and_jam`.

Hence, it also shares the same problem of clang's LoopHints, namely that the 
order of loop transformations (if there are multiple on the same loop: loop 
distribution, vectorization, etc) is defined by the order of the passes in the 
pass pipeline, which should be an implementation detail.

I am currently working on this topic. Could we maybe disable the `#pragma clang 
loop unroll_and_jam` spelling ftm to avoid compatibility issues? However, the 
problem already exists for the other loop hints, so I will have to ensure 
compatibility with those anyway.


https://reviews.llvm.org/D47267



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

[PATCH] D47063: [clangd] Keep only a limited number of idle ASTs in memory

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Having taken a closer look, I think the cache can be simplified/separated a bit 
more cleanly by returning shared pointers and not allowing lookups, instead 
restoring limited ownership in CppFile...

Happy to discuss more, esp if you might disagree :)




Comment at: clangd/ClangdUnit.h:132
 
-/// Manages resources, required by clangd. Allows to rebuild file with new
-/// contents, and provides AST and Preamble for it.
-class CppFile {
+/// A helper class that handles building preambles and ASTs for a file. Also
+/// adds some logging.

This may be change aversion, but I'm not sure this class does enough after this 
change - it doesn't store the inputs or the outputs/cache, so it kind of seems 
like it wants to be a function.

I guess the motivation here is that storing the outputs means dealing with the 
cache, and the cache is local to TUScheduler.
But `CppFile` is only used in TUScheduler, so we could move this there too? It 
feels like expanding the scope more than I'd like.

The upside is that I think it's a more natural division of responsibility: 
`CppFile` could continue to be the "main" holder of the `shared_ptr` 
(which we don't limit, but share), and instead of `Optional` it'd 
have a `weak_ptr` which is controlled and can be refreshed through 
the cache.

As discussed offline, the cache could look something like:
```
class Cache {
   shared_ptr put(ParsedAST);
   void hintUsed(ParsedAST*); // optional, bumps LRU when client reads
   void hintUnused(ParsedAST*); // optional, releases when client abandons
}

shared_ptr CppFile::getAST() {
  shared_ptr AST = WeakAST.lock();
  if (AST)
Cache.hintUsed(AST.get());
  else
WeakAST = AST = Cache.put(build...);
  return AST;
}
```


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D47063



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