[PATCH] D60850: [sema][objc] Minor refactor to OverrideSearch. NFCI.

[Matt Davis via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC358898: [sema][objc] Minor refactor to OverrideSearch. NFCI. 
(authored by mattd, committed by ).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D60850?vs=195876=196077#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D60850

Files:
  lib/Sema/SemaDeclObjC.cpp

Index: lib/Sema/SemaDeclObjC.cpp
===
--- lib/Sema/SemaDeclObjC.cpp
+++ lib/Sema/SemaDeclObjC.cpp
@@ -4166,13 +4166,12 @@
 /// overrides.
 class OverrideSearch {
 public:
-  Sema 
-  ObjCMethodDecl *Method;
+  const ObjCMethodDecl *Method;
   llvm::SmallSetVector Overridden;
   bool Recursive;
 
 public:
-  OverrideSearch(Sema , ObjCMethodDecl *method) : S(S), Method(method) {
+  OverrideSearch(Sema , const ObjCMethodDecl *method) : Method(method) {
 Selector selector = method->getSelector();
 
 // Bypass this search if we've never seen an instance/class method
@@ -4186,19 +4185,20 @@
   if (it == S.MethodPool.end())
 return;
 }
-ObjCMethodList  =
+const ObjCMethodList  =
   method->isInstanceMethod() ? it->second.first : it->second.second;
 if (!list.getMethod()) return;
 
-ObjCContainerDecl *container
+const ObjCContainerDecl *container
   = cast(method->getDeclContext());
 
 // Prevent the search from reaching this container again.  This is
 // important with categories, which override methods from the
 // interface and each other.
-if (ObjCCategoryDecl *Category = dyn_cast(container)) {
+if (const ObjCCategoryDecl *Category =
+dyn_cast(container)) {
   searchFromContainer(container);
-  if (ObjCInterfaceDecl *Interface = Category->getClassInterface())
+  if (const ObjCInterfaceDecl *Interface = Category->getClassInterface())
 searchFromContainer(Interface);
 } else {
   searchFromContainer(container);
@@ -4210,7 +4210,7 @@
   iterator end() const { return Overridden.end(); }
 
 private:
-  void searchFromContainer(ObjCContainerDecl *container) {
+  void searchFromContainer(const ObjCContainerDecl *container) {
 if (container->isInvalidDecl()) return;
 
 switch (container->getDeclKind()) {
@@ -4226,7 +4226,7 @@
 }
   }
 
-  void searchFrom(ObjCProtocolDecl *protocol) {
+  void searchFrom(const ObjCProtocolDecl *protocol) {
 if (!protocol->hasDefinition())
   return;
 
@@ -4235,14 +4235,14 @@
 search(protocol->getReferencedProtocols());
   }
 
-  void searchFrom(ObjCCategoryDecl *category) {
+  void searchFrom(const ObjCCategoryDecl *category) {
 // A method in a category declaration overrides declarations from
 // the main class and from protocols the category references.
 // The main class is handled in the constructor.
 search(category->getReferencedProtocols());
   }
 
-  void searchFrom(ObjCCategoryImplDecl *impl) {
+  void searchFrom(const ObjCCategoryImplDecl *impl) {
 // A method in a category definition that has a category
 // declaration overrides declarations from the category
 // declaration.
@@ -4252,12 +4252,12 @@
 search(Interface);
 
 // Otherwise it overrides declarations from the class.
-} else if (ObjCInterfaceDecl *Interface = impl->getClassInterface()) {
+} else if (const auto *Interface = impl->getClassInterface()) {
   search(Interface);
 }
   }
 
-  void searchFrom(ObjCInterfaceDecl *iface) {
+  void searchFrom(const ObjCInterfaceDecl *iface) {
 // A method in a class declaration overrides declarations from
 if (!iface->hasDefinition())
   return;
@@ -4274,20 +4274,19 @@
 search(iface->getReferencedProtocols());
   }
 
-  void searchFrom(ObjCImplementationDecl *impl) {
+  void searchFrom(const ObjCImplementationDecl *impl) {
 // A method in a class implementation overrides declarations from
 // the class interface.
-if (ObjCInterfaceDecl *Interface = impl->getClassInterface())
+if (const auto *Interface = impl->getClassInterface())
   search(Interface);
   }
 
   void search(const ObjCProtocolList ) {
-for (ObjCProtocolList::iterator i = protocols.begin(), e = protocols.end();
- i != e; ++i)
-  search(*i);
+for (const auto *Proto : protocols)
+  search(Proto);
   }
 
-  void search(ObjCContainerDecl *container) {
+  void search(const ObjCContainerDecl *container) {
 // Check for a method in this container which matches this selector.
 ObjCMethodDecl *meth = container->getMethod(Method->getSelector(),
 Method->isInstanceMethod(),
@@ -4313,6 +4312,8 @@
 void Sema::CheckObjCMethodOverrides(ObjCMethodDecl *ObjCMethod,
 ObjCInterfaceDecl *CurrentClass,
 

[PATCH] D55039: [sema] Warn of mangling change if function parameters are noexcept.

[Matt Davis via Phabricator via cfe-commits]

mattd added a comment.

*Ping* to see if anyone else has any feedback towards this patch.


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

https://reviews.llvm.org/D55039



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

[PATCH] D55039: [sema] Warn of mangling change if function parameters are noexcept.

[Matt Davis via Phabricator via cfe-commits]

mattd marked an inline comment as not done.
mattd added a comment.

In D55039#1334195 , @Rakete wrote:

> Apart from the comments I think your patch LGTM, but I'll let someone else 
> hav the final say.


Thanks for taking a peek!


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

https://reviews.llvm.org/D55039



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

[PATCH] D55039: [sema] Warn of mangling change if function parameters are noexcept.

[Matt Davis via Phabricator via cfe-commits]

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



Comment at: lib/Sema/SemaDecl.cpp:10266
   auto *FPT = NewFD->getType()->castAs();
-  bool AnyNoexcept = HasNoexcept(FPT->getReturnType());
-  for (QualType T : FPT->param_types())
-AnyNoexcept |= HasNoexcept(T);
-  if (AnyNoexcept)
+  if (hasNoexcept(FPT->getReturnType()) ||
+  llvm::any_of(FPT->param_types(),

mattd wrote:
> Rakete wrote:
> > What you're doing here is a subset of what `hasNoexcept` is doing for 
> > function types. Do you think it would make sense to use `!FPT->isNothrow() 
> > && hasNoexcept(NewFW->getType())` or something like that?
> Thanks for the comments @Rakete.
> 
> I do see the similarity and was trying to implement a solution to take 
> advantage of that.  However, I was running into trouble on assert/debug 
> builds.  `llvm_unreachable`, called in `FunctionProtoType::canThrow`, will 
> trigger on FunctionProtoType instances that have not yet had their 
> exception-spec type evaluated.  `canThrow` is called on behalf of 
> `isNothrow`.  Some of the test cases will fail in assert/debug builds if we 
> place a call to to `FPT->isNothrow` here. 
> 
> The original code avoided this assertion by not calling `isNothrow` on FPT 
> here, but instead just evaluating the return and parameter types of FPT.  I 
> made this patch similar to what was already in place, but with the more  
> thorough check in `hasNoexcept.`  I'd be pleased to remove this 
> duplication/similarity if I can avoid this assertion.
I just got back into looking at this issue.  Given that I was unable to 
implement `!FPT->isNoThrow() && hasNoexcept(NewFW->getType())` without 
triggering an assertion (discussed in my reply immediately above), is there 
anything we should be doing differently here?  The change, as it sits now (with 
the duplication), seems to work fine.


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

https://reviews.llvm.org/D55039



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

[PATCH] D55039: [sema] Warn of mangling change if function parameters are noexcept.

[Matt Davis via Phabricator via cfe-commits]

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



Comment at: lib/Sema/SemaDecl.cpp:10266
   auto *FPT = NewFD->getType()->castAs();
-  bool AnyNoexcept = HasNoexcept(FPT->getReturnType());
-  for (QualType T : FPT->param_types())
-AnyNoexcept |= HasNoexcept(T);
-  if (AnyNoexcept)
+  if (hasNoexcept(FPT->getReturnType()) ||
+  llvm::any_of(FPT->param_types(),

Rakete wrote:
> What you're doing here is a subset of what `hasNoexcept` is doing for 
> function types. Do you think it would make sense to use `!FPT->isNothrow() && 
> hasNoexcept(NewFW->getType())` or something like that?
Thanks for the comments @Rakete.

I do see the similarity and was trying to implement a solution to take 
advantage of that.  However, I was running into trouble on assert/debug builds. 
 `llvm_unreachable`, called in `FunctionProtoType::canThrow`, will trigger on 
FunctionProtoType instances that have not yet had their exception-spec type 
evaluated.  `canThrow` is called on behalf of `isNothrow`.  Some of the test 
cases will fail in assert/debug builds if we place a call to to 
`FPT->isNothrow` here. 

The original code avoided this assertion by not calling `isNothrow` on FPT 
here, but instead just evaluating the return and parameter types of FPT.  I 
made this patch similar to what was already in place, but with the more  
thorough check in `hasNoexcept.`  I'd be pleased to remove this 
duplication/similarity if I can avoid this assertion.


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

https://reviews.llvm.org/D55039



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

[PATCH] D55039: [sema] Warn of mangling change if function parameters are noexcept.

[Matt Davis via Phabricator via cfe-commits]

mattd updated this revision to Diff 176515.
mattd added a comment.

- Move the HasNoexcept lambda to its own static function.
- Added an additional check in HasNoexcept to recursively check return values.
- Modified the parameter check in Sema::CheckFunctionDeclaration to use 
llvm::any_of, all we need to know is if any of the parameters might have a 
mangling change.
- Updated the test to include the example provided by @Rakete.


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

https://reviews.llvm.org/D55039

Files:
  lib/Sema/SemaDecl.cpp
  test/CXX/except/except.spec/p2-places.cpp
  test/SemaCXX/cxx1z-noexcept-function-type.cpp

Index: test/SemaCXX/cxx1z-noexcept-function-type.cpp
===
--- test/SemaCXX/cxx1z-noexcept-function-type.cpp
+++ test/SemaCXX/cxx1z-noexcept-function-type.cpp
@@ -83,14 +83,18 @@
   auto f5() -> void (*)() throw();
   auto f6() -> void (&)() throw();
   auto f7() -> void (X::*)() throw();
+  void f8(int, void (*)(int, void (*)() noexcept));
+  void f9(void (*g())() noexcept);
 #if __cplusplus <= 201402L && !defined(NO_COMPAT_MANGLING)
-  // expected-warning@-8 {{mangled name of 'f1' will change in C++17 due to non-throwing exception specification in function signature}}
-  // expected-warning@-8 {{mangled name of 'f2' will change in C++17 due to non-throwing exception specification in function signature}}
-  // expected-warning@-8 {{mangled name of 'f3' will change in C++17 due to non-throwing exception specification in function signature}}
-  // expected-warning@-8 {{mangled name of 'f4' will change in C++17 due to non-throwing exception specification in function signature}}
-  // expected-warning@-8 {{mangled name of 'f5' will change in C++17 due to non-throwing exception specification in function signature}}
-  // expected-warning@-8 {{mangled name of 'f6' will change in C++17 due to non-throwing exception specification in function signature}}
-  // expected-warning@-8 {{mangled name of 'f7' will change in C++17 due to non-throwing exception specification in function signature}}
+  // expected-warning@-10 {{mangled name of 'f1' will change in C++17 due to non-throwing exception specification in function signature}}
+  // expected-warning@-10 {{mangled name of 'f2' will change in C++17 due to non-throwing exception specification in function signature}}
+  // expected-warning@-10 {{mangled name of 'f3' will change in C++17 due to non-throwing exception specification in function signature}}
+  // expected-warning@-10 {{mangled name of 'f4' will change in C++17 due to non-throwing exception specification in function signature}}
+  // expected-warning@-10 {{mangled name of 'f5' will change in C++17 due to non-throwing exception specification in function signature}}
+  // expected-warning@-10 {{mangled name of 'f6' will change in C++17 due to non-throwing exception specification in function signature}}
+  // expected-warning@-10 {{mangled name of 'f7' will change in C++17 due to non-throwing exception specification in function signature}}
+  // expected-warning@-10 {{mangled name of 'f8' will change in C++17 due to non-throwing exception specification in function signature}}
+  // expected-warning@-10 {{mangled name of 'f9' will change in C++17 due to non-throwing exception specification in function signature}}
 #endif
 
   // An instantiation-dependent exception specification needs to be mangled in
Index: test/CXX/except/except.spec/p2-places.cpp
===
--- test/CXX/except/except.spec/p2-places.cpp
+++ test/CXX/except/except.spec/p2-places.cpp
@@ -55,7 +55,7 @@
 
   void (*h())() noexcept(false);
 
-  void (*i() noexcept(false))(void (*)() noexcept(true)) noexcept(false);
+  void (*i() noexcept(false))(void (*)() noexcept(true)) noexcept(false); // expected-warning {{mangled name of 'i' will change in C++17 due to non-throwing exception specification in function signature}}
 
   void (**k)(void pfa() noexcept(false)); // no-error
 
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -9914,6 +9914,28 @@
   OldDecl, MergeTypeWithPrevious, Previous);
 }
 
+/// Returns true if the type T is a function type that contains a noexcept specifier.
+/// This check recursively checks the params and return types of T if T happens
+/// to be a function.
+static bool hasNoexcept(QualType T) {
+  // Strip off declarator chunks that could be between us and a function
+  // type. We don't need to look far, exception specifications are very
+  // restricted prior to C++17.
+  if (auto *RT = T->getAs())
+T = RT->getPointeeType();
+  else if (T->isAnyPointerType())
+T = T->getPointeeType();
+  else if (auto *MPT = T->getAs())
+T = MPT->getPointeeType();
+  if (auto *FPT = T->getAs()) {
+if (FPT->isNothrow() || 

[PATCH] D55039: [sema] Warn of mangling change if function parameters are noexcept.

[Matt Davis via Phabricator via cfe-commits]

mattd marked an inline comment as done.
mattd added a comment.

Thanks for the reviews and added example.  I'll update the logic to account for 
the return type, and add @Rakete 's example to the existing test.




Comment at: lib/Sema/SemaDecl.cpp:10225
+  llvm::any_of(FPT->param_types(),
+   [&](QualType Q) { return HasNoexcept(Q); }))
 return true;

erik.pilkington wrote:
> Can you just make this lambda a static function? Recursive lambdas are a bit 
> crazy :)
Not a problem!  Actually, in my original fix I was hacking on,  I moved the 
lambda into its own static function.  But after looking at the patch, the least 
invasive thing, to keep the patch as small as possible, was just to make the 
lambda recursive... and part of me really liked the idea of a recursive lambda. 
 I'm working on an update, and will move the lambda to a static function.


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

https://reviews.llvm.org/D55039



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

[PATCH] D55039: [sema] Warn of mangling change if function parameters are noexcept.

[Matt Davis via Phabricator via cfe-commits]

mattd created this revision.
mattd added reviewers: erik.pilkington, rsmith.

The flag: `-Wc++17-compat-mangling` was not emitting a warning diagnostic in 
all cases, specifically if a function has parameters that happen to have 
function pointers that have  noexecept parameters.  For example:

  void fn(void (*)(void (*)() noexcept)){}

That example will produce different symbol names between c++17 and earlier 
dialects.  I believe that clang should be emitting the compatibility warning in 
this case.  This patch modifies the existing logic to recursively check the 
parameters of a function and warn if they too are noexcept.

This fixes PR39656.


https://reviews.llvm.org/D55039

Files:
  lib/Sema/SemaDecl.cpp
  test/CXX/except/except.spec/p2-places.cpp
  test/SemaCXX/cxx1z-noexcept-function-type.cpp


Index: test/SemaCXX/cxx1z-noexcept-function-type.cpp
===
--- test/SemaCXX/cxx1z-noexcept-function-type.cpp
+++ test/SemaCXX/cxx1z-noexcept-function-type.cpp
@@ -83,14 +83,16 @@
   auto f5() -> void (*)() throw();
   auto f6() -> void (&)() throw();
   auto f7() -> void (X::*)() throw();
+  void f8(int, void (*)(int, void (*)() noexcept));
 #if __cplusplus <= 201402L && !defined(NO_COMPAT_MANGLING)
-  // expected-warning@-8 {{mangled name of 'f1' will change in C++17 due to 
non-throwing exception specification in function signature}}
-  // expected-warning@-8 {{mangled name of 'f2' will change in C++17 due to 
non-throwing exception specification in function signature}}
-  // expected-warning@-8 {{mangled name of 'f3' will change in C++17 due to 
non-throwing exception specification in function signature}}
-  // expected-warning@-8 {{mangled name of 'f4' will change in C++17 due to 
non-throwing exception specification in function signature}}
-  // expected-warning@-8 {{mangled name of 'f5' will change in C++17 due to 
non-throwing exception specification in function signature}}
-  // expected-warning@-8 {{mangled name of 'f6' will change in C++17 due to 
non-throwing exception specification in function signature}}
-  // expected-warning@-8 {{mangled name of 'f7' will change in C++17 due to 
non-throwing exception specification in function signature}}
+  // expected-warning@-9 {{mangled name of 'f1' will change in C++17 due to 
non-throwing exception specification in function signature}}
+  // expected-warning@-9 {{mangled name of 'f2' will change in C++17 due to 
non-throwing exception specification in function signature}}
+  // expected-warning@-9 {{mangled name of 'f3' will change in C++17 due to 
non-throwing exception specification in function signature}}
+  // expected-warning@-9 {{mangled name of 'f4' will change in C++17 due to 
non-throwing exception specification in function signature}}
+  // expected-warning@-9 {{mangled name of 'f5' will change in C++17 due to 
non-throwing exception specification in function signature}}
+  // expected-warning@-9 {{mangled name of 'f6' will change in C++17 due to 
non-throwing exception specification in function signature}}
+  // expected-warning@-9 {{mangled name of 'f7' will change in C++17 due to 
non-throwing exception specification in function signature}}
+  // expected-warning@-9 {{mangled name of 'f8' will change in C++17 due to 
non-throwing exception specification in function signature}}
 #endif
 
   // An instantiation-dependent exception specification needs to be mangled in
Index: test/CXX/except/except.spec/p2-places.cpp
===
--- test/CXX/except/except.spec/p2-places.cpp
+++ test/CXX/except/except.spec/p2-places.cpp
@@ -55,7 +55,7 @@
 
   void (*h())() noexcept(false);
 
-  void (*i() noexcept(false))(void (*)() noexcept(true)) noexcept(false);
+  void (*i() noexcept(false))(void (*)() noexcept(true)) noexcept(false); // 
expected-warning {{mangled name of 'i' will change in C++17 due to non-throwing 
exception specification in function signature}}
 
   void (**k)(void pfa() noexcept(false)); // no-error
 
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -10209,7 +10209,7 @@
 // most cases, and exception specifications are not permitted in most other
 // contexts where they could make it into a mangling.)
 if (!getLangOpts().CPlusPlus17 && !NewFD->getPrimaryTemplate()) {
-  auto HasNoexcept = [&](QualType T) -> bool {
+  std::function HasNoexcept = [&](QualType T) -> bool {
 // Strip off declarator chunks that could be between us and a function
 // type. We don't need to look far, exception specifications are very
 // restricted prior to C++17.
@@ -10219,9 +10219,12 @@
   T = T->getPointeeType();
 else if (auto *MPT = T->getAs())
   T = MPT->getPointeeType();
-if (auto *FPT = T->getAs())
-  if (FPT->isNothrow())
+if (auto *FPT = 

[PATCH] D54048: [AST] Get aliased type info from an aliased TemplateSpecialization.

[Matt Davis via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC346146: [AST] Get aliased type info from an aliased 
TemplateSpecialization. (authored by mattd, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D54048

Files:
  include/clang/AST/Type.h
  test/SemaCXX/alignof.cpp


Index: include/clang/AST/Type.h
===
--- include/clang/AST/Type.h
+++ include/clang/AST/Type.h
@@ -4901,7 +4901,9 @@
 return !isDependentType() || isCurrentInstantiation() || isTypeAlias();
   }
 
-  QualType desugar() const { return getCanonicalTypeInternal(); }
+  QualType desugar() const {
+return isTypeAlias() ? getAliasedType() : getCanonicalTypeInternal();
+  }
 
   void Profile(llvm::FoldingSetNodeID , const ASTContext ) {
 Profile(ID, Template, template_arguments(), Ctx);
Index: test/SemaCXX/alignof.cpp
===
--- test/SemaCXX/alignof.cpp
+++ test/SemaCXX/alignof.cpp
@@ -97,3 +97,8 @@
   typedef __attribute__((aligned(N))) int Field[sizeof(N)]; // expected-error 
{{requested alignment is dependent but declaration is not dependent}}
 };
 }
+
+typedef int __attribute__((aligned(16))) aligned_int;
+template 
+using template_alias = aligned_int;
+static_assert(alignof(template_alias) == 16, "Expected alignment of 16" 
);


Index: include/clang/AST/Type.h
===
--- include/clang/AST/Type.h
+++ include/clang/AST/Type.h
@@ -4901,7 +4901,9 @@
 return !isDependentType() || isCurrentInstantiation() || isTypeAlias();
   }
 
-  QualType desugar() const { return getCanonicalTypeInternal(); }
+  QualType desugar() const {
+return isTypeAlias() ? getAliasedType() : getCanonicalTypeInternal();
+  }
 
   void Profile(llvm::FoldingSetNodeID , const ASTContext ) {
 Profile(ID, Template, template_arguments(), Ctx);
Index: test/SemaCXX/alignof.cpp
===
--- test/SemaCXX/alignof.cpp
+++ test/SemaCXX/alignof.cpp
@@ -97,3 +97,8 @@
   typedef __attribute__((aligned(N))) int Field[sizeof(N)]; // expected-error {{requested alignment is dependent but declaration is not dependent}}
 };
 }
+
+typedef int __attribute__((aligned(16))) aligned_int;
+template 
+using template_alias = aligned_int;
+static_assert(alignof(template_alias) == 16, "Expected alignment of 16" );
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54048: [AST] Get aliased type info from an aliased TemplateSpecialization.

[Matt Davis via Phabricator via cfe-commits]

mattd added a comment.

In https://reviews.llvm.org/D54048#1286654, @rjmccall wrote:

> That looks a lot better, thanks.  Did you check whether any other tests 
> needed adjustment?  That's not uncommon from this kind of desugaring change.
>
> If not, LGTM.


Thanks again.  I ran the typical 'check-all' in my working tree which included 
clang, clang-extras, compiler-rt, lld.   Everything passed as expected.


https://reviews.llvm.org/D54048



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

[PATCH] D54048: [AST] Get aliased type info from an aliased TemplateSpecialization.

[Matt Davis via Phabricator via cfe-commits]

mattd updated this revision to Diff 172503.
mattd added a comment.

Thanks for the review @rjmccall.  I have moved the type-alias check into 
TemplateSpecializationType's desugar method.  I like this solution better.


https://reviews.llvm.org/D54048

Files:
  include/clang/AST/Type.h
  test/SemaCXX/alignof.cpp


Index: test/SemaCXX/alignof.cpp
===
--- test/SemaCXX/alignof.cpp
+++ test/SemaCXX/alignof.cpp
@@ -97,3 +97,8 @@
   typedef __attribute__((aligned(N))) int Field[sizeof(N)]; // expected-error 
{{requested alignment is dependent but declaration is not dependent}}
 };
 }
+
+typedef int __attribute__((aligned(16))) aligned_int;
+template 
+using template_alias = aligned_int;
+static_assert(alignof(template_alias) == 16, "Expected alignment of 16" 
);
Index: include/clang/AST/Type.h
===
--- include/clang/AST/Type.h
+++ include/clang/AST/Type.h
@@ -4901,7 +4901,9 @@
 return !isDependentType() || isCurrentInstantiation() || isTypeAlias();
   }
 
-  QualType desugar() const { return getCanonicalTypeInternal(); }
+  QualType desugar() const {
+return isTypeAlias() ? getAliasedType() : getCanonicalTypeInternal();
+  }
 
   void Profile(llvm::FoldingSetNodeID , const ASTContext ) {
 Profile(ID, Template, template_arguments(), Ctx);


Index: test/SemaCXX/alignof.cpp
===
--- test/SemaCXX/alignof.cpp
+++ test/SemaCXX/alignof.cpp
@@ -97,3 +97,8 @@
   typedef __attribute__((aligned(N))) int Field[sizeof(N)]; // expected-error {{requested alignment is dependent but declaration is not dependent}}
 };
 }
+
+typedef int __attribute__((aligned(16))) aligned_int;
+template 
+using template_alias = aligned_int;
+static_assert(alignof(template_alias) == 16, "Expected alignment of 16" );
Index: include/clang/AST/Type.h
===
--- include/clang/AST/Type.h
+++ include/clang/AST/Type.h
@@ -4901,7 +4901,9 @@
 return !isDependentType() || isCurrentInstantiation() || isTypeAlias();
   }
 
-  QualType desugar() const { return getCanonicalTypeInternal(); }
+  QualType desugar() const {
+return isTypeAlias() ? getAliasedType() : getCanonicalTypeInternal();
+  }
 
   void Profile(llvm::FoldingSetNodeID , const ASTContext ) {
 Profile(ID, Template, template_arguments(), Ctx);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54048: [AST] Get aliased type info from an aliased TemplateSpecialization.

[Matt Davis via Phabricator via cfe-commits]

mattd created this revision.
mattd added reviewers: aaron.ballman, rjmccall.

Previously the TemplateSpecialization instance for 'template_alias', in the 
example below, returned the type info of the  canonical type (int).  This 
ignored the type alias if the template type happen to be aliased.

Before this patch, the assert would trigger with an  alignment of 4:

  typedef int __attribute__(( aligned( 16 ) )) aligned_int;
  template < typename >
  using template_alias = aligned_int;
  static_assert( alignof( template_alias) == 16, "" );

This patch checks if the TemplateSpecialization type has an alias, and if so 
will return the type information for the aliased type, else the canonical 
type's info is returned (original behavior).  I believe that this is the 
desired behavior.


https://reviews.llvm.org/D54048

Files:
  include/clang/AST/TypeNodes.def
  lib/AST/ASTContext.cpp
  lib/AST/Type.cpp
  test/SemaCXX/alignof.cpp


Index: test/SemaCXX/alignof.cpp
===
--- test/SemaCXX/alignof.cpp
+++ test/SemaCXX/alignof.cpp
@@ -97,3 +97,8 @@
   typedef __attribute__((aligned(N))) int Field[sizeof(N)]; // expected-error 
{{requested alignment is dependent but declaration is not dependent}}
 };
 }
+
+typedef int __attribute__(( aligned( 16 ) )) aligned_int;
+template < typename >
+using template_alias = aligned_int;
+static_assert( alignof( template_alias) == 16, 
"alignof(template_alias<>) is incorrect" );
Index: lib/AST/Type.cpp
===
--- lib/AST/Type.cpp
+++ lib/AST/Type.cpp
@@ -3481,6 +3481,7 @@
 
   case Type::Auto:
   case Type::DeducedTemplateSpecialization:
+  case Type::TemplateSpecialization:
 // Give non-deduced 'auto' types external linkage. We should only see them
 // here in error recovery.
 return CachedProperties(ExternalLinkage, false);
@@ -3587,6 +3588,7 @@
 
   case Type::Auto:
   case Type::DeducedTemplateSpecialization:
+  case Type::TemplateSpecialization:
 return LinkageInfo::external();
 
   case Type::Record:
Index: lib/AST/ASTContext.cpp
===
--- lib/AST/ASTContext.cpp
+++ lib/AST/ASTContext.cpp
@@ -1990,6 +1990,12 @@
 return getTypeInfo(A->getDeducedType().getTypePtr());
   }
 
+  case Type::TemplateSpecialization: {
+const auto *TS = cast(T);
+return TS->isTypeAlias() ? getTypeInfo(TS->getAliasedType().getTypePtr())
+ : getTypeInfo(TS->desugar().getTypePtr());
+  }
+
   case Type::Paren:
 return getTypeInfo(cast(T)->getInnerType().getTypePtr());
 
@@ -6926,6 +6932,7 @@
   // We could see an undeduced auto type here during error recovery.
   // Just ignore it.
   case Type::Auto:
+  case Type::TemplateSpecialization:
   case Type::DeducedTemplateSpecialization:
 return;
 
@@ -8738,6 +8745,7 @@
 llvm_unreachable("Non-canonical and dependent types shouldn't get here");
 
   case Type::Auto:
+  case Type::TemplateSpecialization:
   case Type::DeducedTemplateSpecialization:
   case Type::LValueReference:
   case Type::RValueReference:
Index: include/clang/AST/TypeNodes.def
===
--- include/clang/AST/TypeNodes.def
+++ include/clang/AST/TypeNodes.def
@@ -97,7 +97,7 @@
 DEPENDENT_TYPE(TemplateTypeParm, Type)
 NON_CANONICAL_TYPE(SubstTemplateTypeParm, Type)
 DEPENDENT_TYPE(SubstTemplateTypeParmPack, Type)
-NON_CANONICAL_UNLESS_DEPENDENT_TYPE(TemplateSpecialization, Type)
+TYPE(TemplateSpecialization, Type)
 ABSTRACT_TYPE(Deduced, Type)
 TYPE(Auto, DeducedType)
 TYPE(DeducedTemplateSpecialization, DeducedType)


Index: test/SemaCXX/alignof.cpp
===
--- test/SemaCXX/alignof.cpp
+++ test/SemaCXX/alignof.cpp
@@ -97,3 +97,8 @@
   typedef __attribute__((aligned(N))) int Field[sizeof(N)]; // expected-error {{requested alignment is dependent but declaration is not dependent}}
 };
 }
+
+typedef int __attribute__(( aligned( 16 ) )) aligned_int;
+template < typename >
+using template_alias = aligned_int;
+static_assert( alignof( template_alias) == 16, "alignof(template_alias<>) is incorrect" );
Index: lib/AST/Type.cpp
===
--- lib/AST/Type.cpp
+++ lib/AST/Type.cpp
@@ -3481,6 +3481,7 @@
 
   case Type::Auto:
   case Type::DeducedTemplateSpecialization:
+  case Type::TemplateSpecialization:
 // Give non-deduced 'auto' types external linkage. We should only see them
 // here in error recovery.
 return CachedProperties(ExternalLinkage, false);
@@ -3587,6 +3588,7 @@
 
   case Type::Auto:
   case Type::DeducedTemplateSpecialization:
+  case Type::TemplateSpecialization:
 return LinkageInfo::external();
 
   case Type::Record:
Index: lib/AST/ASTContext.cpp
===
--- lib/AST/ASTContext.cpp
+++ 

[PATCH] D50410: Removing -debug-info-macros from option suggestions test

[Matt Davis via Phabricator via cfe-commits]

mattd added a comment.

In https://reviews.llvm.org/D50410#1202597, @acoomans wrote:

> @mattd thanks for confirming. I tried `clang -cc1as -target x86_64-scei-ps4 
> -debug-info-macros` and got suggestions.
>
> @modocache @mattd Should we try to first land https://reviews.llvm.org/D50515 
> to see if it fails again?


I took a look at https://reviews.llvm.org/D50515, and do not see anything 
target specific in there.  I also applied your patch to two of my local builds 
using the ps4 triple.  I didn't see any errors in the lit tests (yay!)  If you 
can successfully build your tests as x86_64-scei-ps4, and get the expected 
results, then I'd say it is worth seeing how the build-bot responds.


Repository:
  rC Clang

https://reviews.llvm.org/D50410



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

[PATCH] D50410: Removing -debug-info-macros from option suggestions test

[Matt Davis via Phabricator via cfe-commits]

mattd added a comment.

In https://reviews.llvm.org/D50410#1201206, @acoomans wrote:

> @mattd @Sunil_Srivastava if you have access to the PS4 SDK, could you tell if 
> the `-debug-info-macro` command line clang option is available? Thank you


Hi @acoomans,

The ps4 buildbot should be running with the same code that is available in the 
clang repo.  As far as I know, that bot uses the ps4 triple: x86_64-scei-ps4, 
and there is no special treatment for the -debug-info-macro flag under that 
triple in the codebase.  So, the flag should be available when testing, and you 
should be getting the "suggestion" in your tests.  You can try adding 
`x86_64-scei-ps4` triple to your RUN line, and see if things still pass.  As 
far as I can tell having the test for the -debug-macro-info suggestion is fine. 
I am curious, that failure was back in May, if you try a more recent build does 
the issue still show up?


Repository:
  rC Clang

https://reviews.llvm.org/D50410



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

[PATCH] D50408: [analyzer] Avoid querying this-pointers for static-methods.

[Matt Davis via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC339201: [analyzer] Avoid querying this-pointers for 
static-methods. (authored by mattd, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D50408

Files:
  lib/StaticAnalyzer/Core/LoopWidening.cpp
  test/Analysis/loop-widening-ignore-static-methods.cpp


Index: test/Analysis/loop-widening-ignore-static-methods.cpp
===
--- test/Analysis/loop-widening-ignore-static-methods.cpp
+++ test/Analysis/loop-widening-ignore-static-methods.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-config 
widen-loops=true -analyzer-max-loop 2 %s
+// REQUIRES: asserts
+// expected-no-diagnostics
+//
+// This test checks that the loop-widening code ignores static methods.  If 
that is not the
+// case, then an assertion will trigger.
+
+class Test {
+  static void foo() {
+for (;;) {}
+  }
+};
Index: lib/StaticAnalyzer/Core/LoopWidening.cpp
===
--- lib/StaticAnalyzer/Core/LoopWidening.cpp
+++ lib/StaticAnalyzer/Core/LoopWidening.cpp
@@ -81,8 +81,10 @@
 
   // 'this' pointer is not an lvalue, we should not invalidate it. If the loop
   // is located in a method, constructor or destructor, the value of 'this'
-  // pointer shoule remain unchanged.
-  if (const CXXMethodDecl *CXXMD = dyn_cast(STC->getDecl())) {
+  // pointer should remain unchanged.  Ignore static methods, since they do not
+  // have 'this' pointers.
+  const CXXMethodDecl *CXXMD = dyn_cast(STC->getDecl());
+  if (CXXMD && !CXXMD->isStatic()) {
 const CXXThisRegion *ThisR = MRMgr.getCXXThisRegion(
 CXXMD->getThisType(STC->getAnalysisDeclContext()->getASTContext()),
 STC);


Index: test/Analysis/loop-widening-ignore-static-methods.cpp
===
--- test/Analysis/loop-widening-ignore-static-methods.cpp
+++ test/Analysis/loop-widening-ignore-static-methods.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-config widen-loops=true -analyzer-max-loop 2 %s
+// REQUIRES: asserts
+// expected-no-diagnostics
+//
+// This test checks that the loop-widening code ignores static methods.  If that is not the
+// case, then an assertion will trigger.
+
+class Test {
+  static void foo() {
+for (;;) {}
+  }
+};
Index: lib/StaticAnalyzer/Core/LoopWidening.cpp
===
--- lib/StaticAnalyzer/Core/LoopWidening.cpp
+++ lib/StaticAnalyzer/Core/LoopWidening.cpp
@@ -81,8 +81,10 @@
 
   // 'this' pointer is not an lvalue, we should not invalidate it. If the loop
   // is located in a method, constructor or destructor, the value of 'this'
-  // pointer shoule remain unchanged.
-  if (const CXXMethodDecl *CXXMD = dyn_cast(STC->getDecl())) {
+  // pointer should remain unchanged.  Ignore static methods, since they do not
+  // have 'this' pointers.
+  const CXXMethodDecl *CXXMD = dyn_cast(STC->getDecl());
+  if (CXXMD && !CXXMD->isStatic()) {
 const CXXThisRegion *ThisR = MRMgr.getCXXThisRegion(
 CXXMD->getThisType(STC->getAnalysisDeclContext()->getASTContext()),
 STC);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50408: [analyzer] Avoid querying this-pointers for static-methods.

[Matt Davis via Phabricator via cfe-commits]

mattd created this revision.
mattd added a reviewer: dcoughlin.
Herald added subscribers: mikhail.ramalho, a.sidorin, szepet, xazax.hun.
Herald added a reviewer: george.karpenkov.

The loop-widening code processes c++ methods looking for `this` pointers.  In
the case of static methods (which do not have `this` pointers), an assertion
was triggering.   This patch avoids trying to process `this` pointers for
static methods, and thus avoids triggering the assertion .


https://reviews.llvm.org/D50408

Files:
  lib/StaticAnalyzer/Core/LoopWidening.cpp
  test/Analysis/loop-widening-ignore-static-methods.cpp


Index: test/Analysis/loop-widening-ignore-static-methods.cpp
===
--- test/Analysis/loop-widening-ignore-static-methods.cpp
+++ test/Analysis/loop-widening-ignore-static-methods.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-config 
widen-loops=true -analyzer-max-loop 2 %s
+// REQUIRES: asserts
+// expected-no-diagnostics
+//
+// This test checks that the loop-widening code ignores static methods.  If 
that is not the
+// case, then an assertion will trigger.
+
+class Test {
+  static void foo() {
+for (;;) {}
+  }
+};
Index: lib/StaticAnalyzer/Core/LoopWidening.cpp
===
--- lib/StaticAnalyzer/Core/LoopWidening.cpp
+++ lib/StaticAnalyzer/Core/LoopWidening.cpp
@@ -81,8 +81,10 @@
 
   // 'this' pointer is not an lvalue, we should not invalidate it. If the loop
   // is located in a method, constructor or destructor, the value of 'this'
-  // pointer shoule remain unchanged.
-  if (const CXXMethodDecl *CXXMD = dyn_cast(STC->getDecl())) {
+  // pointer should remain unchanged.  Ignore static methods, since they do not
+  // have 'this' pointers.
+  const CXXMethodDecl *CXXMD = dyn_cast(STC->getDecl());
+  if (CXXMD && !CXXMD->isStatic()) {
 const CXXThisRegion *ThisR = MRMgr.getCXXThisRegion(
 CXXMD->getThisType(STC->getAnalysisDeclContext()->getASTContext()),
 STC);


Index: test/Analysis/loop-widening-ignore-static-methods.cpp
===
--- test/Analysis/loop-widening-ignore-static-methods.cpp
+++ test/Analysis/loop-widening-ignore-static-methods.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-config widen-loops=true -analyzer-max-loop 2 %s
+// REQUIRES: asserts
+// expected-no-diagnostics
+//
+// This test checks that the loop-widening code ignores static methods.  If that is not the
+// case, then an assertion will trigger.
+
+class Test {
+  static void foo() {
+for (;;) {}
+  }
+};
Index: lib/StaticAnalyzer/Core/LoopWidening.cpp
===
--- lib/StaticAnalyzer/Core/LoopWidening.cpp
+++ lib/StaticAnalyzer/Core/LoopWidening.cpp
@@ -81,8 +81,10 @@
 
   // 'this' pointer is not an lvalue, we should not invalidate it. If the loop
   // is located in a method, constructor or destructor, the value of 'this'
-  // pointer shoule remain unchanged.
-  if (const CXXMethodDecl *CXXMD = dyn_cast(STC->getDecl())) {
+  // pointer should remain unchanged.  Ignore static methods, since they do not
+  // have 'this' pointers.
+  const CXXMethodDecl *CXXMD = dyn_cast(STC->getDecl());
+  if (CXXMD && !CXXMD->isStatic()) {
 const CXXThisRegion *ThisR = MRMgr.getCXXThisRegion(
 CXXMD->getThisType(STC->getAnalysisDeclContext()->getASTContext()),
 STC);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49953: [compiler-rt] Add a routine to specify the mode used when creating profile dirs.

[Matt Davis via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
mattd marked an inline comment as done.
Closed by commit rCRT338456: [compiler-rt] Add a routine to specify the mode 
used when creating profile dirs. (authored by mattd, committed by ).
Herald added subscribers: Sanitizers, llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D49953?vs=158298=158419#toc

Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D49953

Files:
  lib/profile/InstrProfilingUtil.c
  lib/profile/InstrProfilingUtil.h
  test/profile/instrprof-set-dir-mode.c

Index: test/profile/instrprof-set-dir-mode.c
===
--- test/profile/instrprof-set-dir-mode.c
+++ test/profile/instrprof-set-dir-mode.c
@@ -0,0 +1,48 @@
+// UNSUPPORTED: windows
+// RUN: %clang_pgogen -o %t.bin %s -DTESTPATH=\"%t.dir\"
+// RUN: rm -rf %t.dir
+// RUN: %run %t.bin
+
+#include 
+#include 
+#include 
+#include 
+
+void __llvm_profile_set_dir_mode(unsigned Mode);
+unsigned __llvm_profile_get_dir_mode(void);
+void __llvm_profile_recursive_mkdir(char *Path);
+
+static int test(unsigned Mode, const char *TestDir) {
+  int Ret = 0;
+
+  /* Create a dir and set the mode accordingly. */
+  char *Dir = strdup(TestDir);
+  if (!Dir)
+return -1;
+  __llvm_profile_set_dir_mode(Mode);
+  __llvm_profile_recursive_mkdir(Dir);
+
+  if (Mode != __llvm_profile_get_dir_mode())
+Ret = -1;
+  else {
+const unsigned Expected = ~umask(0) & Mode;
+struct stat DirSt;
+if (stat(Dir, ) == -1)
+  Ret = -1;
+else if (DirSt.st_mode != Expected) {
+  printf("Modes do not match: Expected %o but found %o (%s)\n", Expected,
+ DirSt.st_mode, Dir);
+  Ret = -1;
+}
+  }
+
+  free(Dir);
+  return Ret;
+}
+
+int main(void) {
+  if (test(S_IFDIR | 0777, TESTPATH "/foo/bar/baz/") ||
+  test(S_IFDIR | 0666, TESTPATH "/foo/bar/qux/"))
+return -1;
+  return 0;
+}
Index: lib/profile/InstrProfilingUtil.h
===
--- lib/profile/InstrProfilingUtil.h
+++ lib/profile/InstrProfilingUtil.h
@@ -16,6 +16,12 @@
 /*! \brief Create a directory tree. */
 void __llvm_profile_recursive_mkdir(char *Pathname);
 
+/*! Set the mode used when creating profile directories. */
+void __llvm_profile_set_dir_mode(unsigned Mode);
+
+/*! Return the directory creation mode. */
+unsigned __llvm_profile_get_dir_mode(void);
+
 int lprofLockFd(int fd);
 int lprofUnlockFd(int fd);
 
Index: lib/profile/InstrProfilingUtil.c
===
--- lib/profile/InstrProfilingUtil.c
+++ lib/profile/InstrProfilingUtil.c
@@ -35,6 +35,8 @@
 #include "InstrProfiling.h"
 #include "InstrProfilingUtil.h"
 
+COMPILER_RT_WEAK unsigned lprofDirMode = 0755;
+
 COMPILER_RT_VISIBILITY
 void __llvm_profile_recursive_mkdir(char *path) {
   int i;
@@ -47,12 +49,19 @@
 #ifdef _WIN32
 _mkdir(path);
 #else
-mkdir(path, 0755); /* Some of these will fail, ignore it. */
+/* Some of these will fail, ignore it. */
+mkdir(path, __llvm_profile_get_dir_mode());
 #endif
 path[i] = save;
   }
 }
 
+COMPILER_RT_VISIBILITY
+void __llvm_profile_set_dir_mode(unsigned Mode) { lprofDirMode = Mode; }
+
+COMPILER_RT_VISIBILITY
+unsigned __llvm_profile_get_dir_mode(void) { return lprofDirMode; }
+
 #if COMPILER_RT_HAS_ATOMICS != 1
 COMPILER_RT_VISIBILITY
 uint32_t lprofBoolCmpXchg(void **Ptr, void *OldV, void *NewV) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49953: [compiler-rt] Add a routine to specify the mode used when creating profile dirs.

[Matt Davis via Phabricator via cfe-commits]

mattd updated this revision to Diff 158298.
mattd added a comment.

- Simplify the test case, let the compiler join the literals for us.


https://reviews.llvm.org/D49953

Files:
  lib/profile/InstrProfilingUtil.c
  lib/profile/InstrProfilingUtil.h
  test/profile/instrprof-set-dir-mode.c
  test/profile/instrprof-set-dir-mode2.c

Index: test/profile/instrprof-set-dir-mode2.c
===
--- /dev/null
+++ test/profile/instrprof-set-dir-mode2.c
@@ -0,0 +1,48 @@
+// UNSUPPORTED: windows
+// RUN: %clang_pgogen -o %t.bin %s -DTESTPATH=\"%t.dir\"
+// rm -rf %t.dir
+// RUN: %run %t.bin
+
+#include 
+#include 
+#include 
+#include 
+
+void __llvm_profile_set_dir_mode(unsigned Mode);
+unsigned __llvm_profile_get_dir_mode(void);
+void __llvm_profile_recursive_mkdir(char *Path);
+
+static int test(unsigned Mode, const char *TestDir) {
+  int Ret = 0;
+
+  /* Create a dir and set the mode accordingly. */
+  char *Dir = strdup(TestDir);
+  if (!Dir)
+return -1;
+  __llvm_profile_set_dir_mode(Mode);
+  __llvm_profile_recursive_mkdir(Dir);
+
+  if (Mode != __llvm_profile_get_dir_mode())
+Ret = -1;
+  else {
+const unsigned Expected = ~umask(0) & Mode;
+struct stat DirSt;
+if (stat(Dir, ) == -1)
+  Ret = -1;
+else if (DirSt.st_mode != Expected) {
+  printf("Modes do not match: Expected %o but found %o (%s)\n", Expected,
+ DirSt.st_mode, Dir);
+  Ret = -1;
+}
+  }
+
+  free(Dir);
+  return Ret;
+}
+
+int main(void) {
+  if (test(S_IFDIR | 0777, TESTPATH "/foo/bar/baz/") ||
+  test(S_IFDIR | 0666, TESTPATH "/foo/bar/qux/"))
+return -1;
+  return 0;
+}
Index: test/profile/instrprof-set-dir-mode.c
===
--- /dev/null
+++ test/profile/instrprof-set-dir-mode.c
@@ -0,0 +1,17 @@
+// RUN: %clang_pgogen -o %t %s
+// RUN: %run %t
+
+void __llvm_profile_set_dir_mode(unsigned Mode);
+unsigned __llvm_profile_get_dir_mode(void);
+
+int main(void) {
+  __llvm_profile_set_dir_mode(0777);
+  if (__llvm_profile_get_dir_mode() != 0777)
+return -1;
+
+  __llvm_profile_set_dir_mode(0666);
+  if (__llvm_profile_get_dir_mode() != 0666)
+return -1;
+
+  return 0;
+}
Index: lib/profile/InstrProfilingUtil.h
===
--- lib/profile/InstrProfilingUtil.h
+++ lib/profile/InstrProfilingUtil.h
@@ -16,6 +16,12 @@
 /*! \brief Create a directory tree. */
 void __llvm_profile_recursive_mkdir(char *Pathname);
 
+/*! Set the mode used when creating profile directories. */
+void __llvm_profile_set_dir_mode(unsigned Mode);
+
+/*! Return the directory creation mode. */
+unsigned __llvm_profile_get_dir_mode(void);
+
 int lprofLockFd(int fd);
 int lprofUnlockFd(int fd);
 
Index: lib/profile/InstrProfilingUtil.c
===
--- lib/profile/InstrProfilingUtil.c
+++ lib/profile/InstrProfilingUtil.c
@@ -35,6 +35,8 @@
 #include "InstrProfiling.h"
 #include "InstrProfilingUtil.h"
 
+COMPILER_RT_WEAK unsigned lprofDirMode = 0755;
+
 COMPILER_RT_VISIBILITY
 void __llvm_profile_recursive_mkdir(char *path) {
   int i;
@@ -47,12 +49,19 @@
 #ifdef _WIN32
 _mkdir(path);
 #else
-mkdir(path, 0755); /* Some of these will fail, ignore it. */
+/* Some of these will fail, ignore it. */
+mkdir(path, __llvm_profile_get_dir_mode());
 #endif
 path[i] = save;
   }
 }
 
+COMPILER_RT_VISIBILITY
+void __llvm_profile_set_dir_mode(unsigned Mode) { lprofDirMode = Mode; }
+
+COMPILER_RT_VISIBILITY
+unsigned __llvm_profile_get_dir_mode(void) { return lprofDirMode; }
+
 #if COMPILER_RT_HAS_ATOMICS != 1
 COMPILER_RT_VISIBILITY
 uint32_t lprofBoolCmpXchg(void **Ptr, void *OldV, void *NewV) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49953: [compiler-rt] Add a routine to specify the mode used when creating profile dirs.

[Matt Davis via Phabricator via cfe-commits]

mattd updated this revision to Diff 158272.
mattd added a comment.

- Renamed the test output and test directory to clean up the more specific test 
example.


https://reviews.llvm.org/D49953

Files:
  lib/profile/InstrProfilingUtil.c
  lib/profile/InstrProfilingUtil.h
  test/profile/instrprof-set-dir-mode.c
  test/profile/instrprof-set-dir-mode2.c

Index: test/profile/instrprof-set-dir-mode2.c
===
--- /dev/null
+++ test/profile/instrprof-set-dir-mode2.c
@@ -0,0 +1,58 @@
+// UNSUPPORTED: windows
+// RUN: %clang_pgogen -o %t.bin %s -DTESTPATH=\"%t.dir\"
+// rm -rf %t.dir
+// RUN: %run %t.bin
+
+#include 
+#include 
+#include 
+#include 
+
+void __llvm_profile_set_dir_mode(unsigned Mode);
+unsigned __llvm_profile_get_dir_mode(void);
+void __llvm_profile_recursive_mkdir(char *Path);
+
+static char *join(const char *Dir, const char *Subdir) {
+  char *Joined = malloc(strlen(Dir) + strlen(Subdir) + 2);
+  if (!Joined)
+return NULL;
+  sprintf(Joined, "%s/%s", Dir, Subdir);
+  return Joined;
+}
+
+static int test(unsigned Mode, const char *Subdir) {
+  int Ret = 0;
+
+  /* Truncate the path removing the filename and adding Subdir. */
+  char *Dir;
+  if (!(Dir = join(TESTPATH, Subdir)))
+return -1;
+
+  /* Create a dir and set the mode accordingly. */
+  __llvm_profile_set_dir_mode(Mode);
+  __llvm_profile_recursive_mkdir(Dir);
+
+  if (Mode != __llvm_profile_get_dir_mode())
+Ret = -1;
+  else {
+const unsigned Expected = ~umask(0) & Mode;
+struct stat DirSt;
+if (stat(Dir, ) == -1)
+  Ret = -1;
+else if (DirSt.st_mode != Expected) {
+  printf("Modes do not match: Expected %o but found %o (%s)\n", Expected,
+ DirSt.st_mode, Dir);
+  Ret = -1;
+}
+  }
+
+  free(Dir);
+  return Ret;
+}
+
+int main(int argc, const char *argv[]) {
+  if (test(S_IFDIR | 0777, "/foo/bar/baz/") ||
+  test(S_IFDIR | 0666, "/foo/bar/qux/"))
+return -1;
+  return 0;
+}
Index: test/profile/instrprof-set-dir-mode.c
===
--- /dev/null
+++ test/profile/instrprof-set-dir-mode.c
@@ -0,0 +1,17 @@
+// RUN: %clang_pgogen -o %t %s
+// RUN: %run %t
+
+void __llvm_profile_set_dir_mode(unsigned Mode);
+unsigned __llvm_profile_get_dir_mode(void);
+
+int main(void) {
+  __llvm_profile_set_dir_mode(0777);
+  if (__llvm_profile_get_dir_mode() != 0777)
+return -1;
+
+  __llvm_profile_set_dir_mode(0666);
+  if (__llvm_profile_get_dir_mode() != 0666)
+return -1;
+
+  return 0;
+}
Index: lib/profile/InstrProfilingUtil.h
===
--- lib/profile/InstrProfilingUtil.h
+++ lib/profile/InstrProfilingUtil.h
@@ -16,6 +16,12 @@
 /*! \brief Create a directory tree. */
 void __llvm_profile_recursive_mkdir(char *Pathname);
 
+/*! Set the mode used when creating profile directories. */
+void __llvm_profile_set_dir_mode(unsigned Mode);
+
+/*! Return the directory creation mode. */
+unsigned __llvm_profile_get_dir_mode(void);
+
 int lprofLockFd(int fd);
 int lprofUnlockFd(int fd);
 
Index: lib/profile/InstrProfilingUtil.c
===
--- lib/profile/InstrProfilingUtil.c
+++ lib/profile/InstrProfilingUtil.c
@@ -35,6 +35,8 @@
 #include "InstrProfiling.h"
 #include "InstrProfilingUtil.h"
 
+COMPILER_RT_WEAK unsigned lprofDirMode = 0755;
+
 COMPILER_RT_VISIBILITY
 void __llvm_profile_recursive_mkdir(char *path) {
   int i;
@@ -47,12 +49,19 @@
 #ifdef _WIN32
 _mkdir(path);
 #else
-mkdir(path, 0755); /* Some of these will fail, ignore it. */
+/* Some of these will fail, ignore it. */
+mkdir(path, __llvm_profile_get_dir_mode());
 #endif
 path[i] = save;
   }
 }
 
+COMPILER_RT_VISIBILITY
+void __llvm_profile_set_dir_mode(unsigned Mode) { lprofDirMode = Mode; }
+
+COMPILER_RT_VISIBILITY
+unsigned __llvm_profile_get_dir_mode(void) { return lprofDirMode; }
+
 #if COMPILER_RT_HAS_ATOMICS != 1
 COMPILER_RT_VISIBILITY
 uint32_t lprofBoolCmpXchg(void **Ptr, void *OldV, void *NewV) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49953: [compiler-rt] Add a routine to specify the mode used when creating profile dirs.

[Matt Davis via Phabricator via cfe-commits]

mattd updated this revision to Diff 158144.
mattd added a comment.

- Added a test that creates a directory via __llvm_profile_recursive_mkdir and 
verifies the mode.
- I intend to only commit one of the two tests, but wanted to provide a more 
specific/detailed test.
  - I don't know if we really want such a specific test, but it's a potential 
start at one.
- We can definitely ignore windows here (since there is no mode passed to its 
equivalent to mkdir).
- I don't know if there are other systems we need to ignore, e.g., android?
- I'm still hesitant to adding the more specific test, it feels very os 
specific.
- I didn't see an alternative to FileCheck's (deprecated) %T, so I wrote a 
simple 'dirname' routine that is not dependent on libgen.
  - I hope that is sufficient for this test case.


https://reviews.llvm.org/D49953

Files:
  lib/profile/InstrProfilingUtil.c
  lib/profile/InstrProfilingUtil.h
  test/profile/instrprof-set-dir-mode.c
  test/profile/instrprof-set-dir-mode2.c

Index: test/profile/instrprof-set-dir-mode2.c
===
--- /dev/null
+++ test/profile/instrprof-set-dir-mode2.c
@@ -0,0 +1,73 @@
+// UNSUPPORTED: windows
+// RUN: %clang_pgogen -o %t %s -DTESTPATH=\"%t\"
+// RUN: %run %t
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+void __llvm_profile_set_dir_mode(uint32_t Mode);
+unsigned __llvm_profile_get_dir_mode(void);
+void __llvm_profile_recursive_mkdir(char *Path);
+
+/* Drop characters starting from the last '/'. */
+static char *dirname(const char *Filename) {
+  const char *End;
+  if (!(End = strrchr(Filename, '/')))
+return NULL;
+  char *D = strdup(Filename);
+  if ((intptr_t)End - (intptr_t)Filename - 1 < 0)
+return NULL;
+  D[End - Filename - 1] = '\0';
+  return D;
+}
+
+static char *join(const char *Dir, const char *Subdir) {
+  char *Joined = malloc(strlen(Dir) + strlen(Subdir) + 2);
+  if (!Joined)
+return NULL;
+  sprintf(Joined, "%s/%s", Dir, Subdir);
+  return Joined;
+}
+
+static int test(uint32_t Mode, const char *Subdir) {
+  int Ret = 0;
+  char *Base, *Dir;
+
+  /* Truncate the path removing the filename and adding Subdir. */
+  if (!(Base = dirname(TESTPATH)))
+return -1;
+  if (!(Dir = join(Base, Subdir)))
+return -1;
+
+  /* Create a dir and set the mode accordingly. */
+  __llvm_profile_set_dir_mode(Mode);
+  __llvm_profile_recursive_mkdir(Dir);
+
+  if (Mode != __llvm_profile_get_dir_mode())
+Ret = -1;
+  else {
+const uint32_t Expected = ~umask(0) & Mode;
+struct stat DirSt;
+if (stat(Dir, ) == -1)
+  Ret = -1;
+else if (DirSt.st_mode != Expected) {
+  printf("Modes do not match: Expected %o but found %o (%s)\n", Expected,
+ DirSt.st_mode, Dir);
+  Ret = -1;
+}
+  }
+
+  free(Base);
+  free(Dir);
+  return Ret;
+}
+
+int main(int argc, const char *argv[]) {
+  if (test(S_IFDIR | 0777, "/foo/bar/baz/") ||
+  test(S_IFDIR | 0666, "/foo/bar/qux/"))
+return -1;
+  return 0;
+}
Index: test/profile/instrprof-set-dir-mode.c
===
--- /dev/null
+++ test/profile/instrprof-set-dir-mode.c
@@ -0,0 +1,17 @@
+// RUN: %clang_pgogen -o %t %s
+// RUN: %run %t
+
+void __llvm_profile_set_dir_mode(unsigned Mode);
+unsigned __llvm_profile_get_dir_mode(void);
+
+int main(void) {
+  __llvm_profile_set_dir_mode(0777);
+  if (__llvm_profile_get_dir_mode() != 0777)
+return -1;
+
+  __llvm_profile_set_dir_mode(0666);
+  if (__llvm_profile_get_dir_mode() != 0666)
+return -1;
+
+  return 0;
+}
Index: lib/profile/InstrProfilingUtil.h
===
--- lib/profile/InstrProfilingUtil.h
+++ lib/profile/InstrProfilingUtil.h
@@ -16,6 +16,12 @@
 /*! \brief Create a directory tree. */
 void __llvm_profile_recursive_mkdir(char *Pathname);
 
+/*! Set the mode used when creating profile directories. */
+void __llvm_profile_set_dir_mode(unsigned Mode);
+
+/*! Return the directory creation mode. */
+unsigned __llvm_profile_get_dir_mode(void);
+
 int lprofLockFd(int fd);
 int lprofUnlockFd(int fd);
 
Index: lib/profile/InstrProfilingUtil.c
===
--- lib/profile/InstrProfilingUtil.c
+++ lib/profile/InstrProfilingUtil.c
@@ -35,6 +35,8 @@
 #include "InstrProfiling.h"
 #include "InstrProfilingUtil.h"
 
+COMPILER_RT_WEAK unsigned lprofDirMode = 0755;
+
 COMPILER_RT_VISIBILITY
 void __llvm_profile_recursive_mkdir(char *path) {
   int i;
@@ -47,12 +49,19 @@
 #ifdef _WIN32
 _mkdir(path);
 #else
-mkdir(path, 0755); /* Some of these will fail, ignore it. */
+/* Some of these will fail, ignore it. */
+mkdir(path, __llvm_profile_get_dir_mode());
 #endif
 path[i] = save;
   }
 }
 
+COMPILER_RT_VISIBILITY
+void __llvm_profile_set_dir_mode(unsigned Mode) { lprofDirMode = Mode; }
+
+COMPILER_RT_VISIBILITY
+unsigned __llvm_profile_get_dir_mode(void) { return 

[PATCH] D49953: [compiler-rt] Add a routine to specify the mode used when creating profile dirs.

[Matt Davis via Phabricator via cfe-commits]

mattd added a comment.

In https://reviews.llvm.org/D49953#1180466, @probinson wrote:

> Is it possible/reasonable for the test to call __llvm_profile_recursive_mkdir 
> and then verify the mode of the created directory?
>  The test would have to be flagged as unsupported on Windows but that seems 
> fine.


That't totally possible, as long as it's fine to call fstat()/stat() to gather 
the mode value on the resulting directory.   We'd also
need to get the umask() so that we can ensure that the correct value is set 
with respect to the user's umask.


https://reviews.llvm.org/D49953



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

[PATCH] D49953: [compiler-rt] Add a routine to specify the mode used when creating profile dirs.

[Matt Davis via Phabricator via cfe-commits]

mattd created this revision.
mattd added a reviewer: void.
Herald added a subscriber: dberris.

This patch introduces `llvm_profile_set_dir_mode` and 
`llvm_profile_get_dir_mode` to
the compiler-rt profile API.

Originally, profile data was placed into a directory that was created with a 
hard-coded
mode value of 0755 (for non-win32 builds).  In certain cases, it can be helpful 
to create
directories with a different mode other than 0755.  This patch introduces 
set/get
routines to allow users to specify a desired mode.  The default remains at 0755.


https://reviews.llvm.org/D49953

Files:
  lib/profile/InstrProfilingUtil.c
  lib/profile/InstrProfilingUtil.h
  test/profile/instrprof-set-dir-mode.c


Index: test/profile/instrprof-set-dir-mode.c
===
--- /dev/null
+++ test/profile/instrprof-set-dir-mode.c
@@ -0,0 +1,17 @@
+// RUN: %clang_pgogen -o %t %s
+// RUN: %run %t
+
+void __llvm_profile_set_dir_mode(unsigned Mode);
+unsigned __llvm_profile_get_dir_mode(void);
+
+int main(void) {
+  __llvm_profile_set_dir_mode(0777);
+  if (__llvm_profile_get_dir_mode() != 0777)
+return -1;
+
+  __llvm_profile_set_dir_mode(0666);
+  if (__llvm_profile_get_dir_mode() != 0666)
+return -1;
+
+  return 0;
+}
Index: lib/profile/InstrProfilingUtil.h
===
--- lib/profile/InstrProfilingUtil.h
+++ lib/profile/InstrProfilingUtil.h
@@ -16,6 +16,12 @@
 /*! \brief Create a directory tree. */
 void __llvm_profile_recursive_mkdir(char *Pathname);
 
+/*! Set the mode used when creating profile directories. */
+void __llvm_profile_set_dir_mode(unsigned Mode);
+
+/*! Return the directory creation mode. */
+unsigned __llvm_profile_get_dir_mode(void);
+
 int lprofLockFd(int fd);
 int lprofUnlockFd(int fd);
 
Index: lib/profile/InstrProfilingUtil.c
===
--- lib/profile/InstrProfilingUtil.c
+++ lib/profile/InstrProfilingUtil.c
@@ -35,6 +35,8 @@
 #include "InstrProfiling.h"
 #include "InstrProfilingUtil.h"
 
+COMPILER_RT_WEAK unsigned lprofDirMode = 0755;
+
 COMPILER_RT_VISIBILITY
 void __llvm_profile_recursive_mkdir(char *path) {
   int i;
@@ -47,12 +49,19 @@
 #ifdef _WIN32
 _mkdir(path);
 #else
-mkdir(path, 0755); /* Some of these will fail, ignore it. */
+/* Some of these will fail, ignore it. */
+mkdir(path, __llvm_profile_get_dir_mode());
 #endif
 path[i] = save;
   }
 }
 
+COMPILER_RT_VISIBILITY
+void __llvm_profile_set_dir_mode(unsigned Mode) { lprofDirMode = Mode; }
+
+COMPILER_RT_VISIBILITY
+unsigned __llvm_profile_get_dir_mode(void) { return lprofDirMode; }
+
 #if COMPILER_RT_HAS_ATOMICS != 1
 COMPILER_RT_VISIBILITY
 uint32_t lprofBoolCmpXchg(void **Ptr, void *OldV, void *NewV) {


Index: test/profile/instrprof-set-dir-mode.c
===
--- /dev/null
+++ test/profile/instrprof-set-dir-mode.c
@@ -0,0 +1,17 @@
+// RUN: %clang_pgogen -o %t %s
+// RUN: %run %t
+
+void __llvm_profile_set_dir_mode(unsigned Mode);
+unsigned __llvm_profile_get_dir_mode(void);
+
+int main(void) {
+  __llvm_profile_set_dir_mode(0777);
+  if (__llvm_profile_get_dir_mode() != 0777)
+return -1;
+
+  __llvm_profile_set_dir_mode(0666);
+  if (__llvm_profile_get_dir_mode() != 0666)
+return -1;
+
+  return 0;
+}
Index: lib/profile/InstrProfilingUtil.h
===
--- lib/profile/InstrProfilingUtil.h
+++ lib/profile/InstrProfilingUtil.h
@@ -16,6 +16,12 @@
 /*! \brief Create a directory tree. */
 void __llvm_profile_recursive_mkdir(char *Pathname);
 
+/*! Set the mode used when creating profile directories. */
+void __llvm_profile_set_dir_mode(unsigned Mode);
+
+/*! Return the directory creation mode. */
+unsigned __llvm_profile_get_dir_mode(void);
+
 int lprofLockFd(int fd);
 int lprofUnlockFd(int fd);
 
Index: lib/profile/InstrProfilingUtil.c
===
--- lib/profile/InstrProfilingUtil.c
+++ lib/profile/InstrProfilingUtil.c
@@ -35,6 +35,8 @@
 #include "InstrProfiling.h"
 #include "InstrProfilingUtil.h"
 
+COMPILER_RT_WEAK unsigned lprofDirMode = 0755;
+
 COMPILER_RT_VISIBILITY
 void __llvm_profile_recursive_mkdir(char *path) {
   int i;
@@ -47,12 +49,19 @@
 #ifdef _WIN32
 _mkdir(path);
 #else
-mkdir(path, 0755); /* Some of these will fail, ignore it. */
+/* Some of these will fail, ignore it. */
+mkdir(path, __llvm_profile_get_dir_mode());
 #endif
 path[i] = save;
   }
 }
 
+COMPILER_RT_VISIBILITY
+void __llvm_profile_set_dir_mode(unsigned Mode) { lprofDirMode = Mode; }
+
+COMPILER_RT_VISIBILITY
+unsigned __llvm_profile_get_dir_mode(void) { return lprofDirMode; }
+
 #if COMPILER_RT_HAS_ATOMICS != 1
 COMPILER_RT_VISIBILITY
 uint32_t lprofBoolCmpXchg(void **Ptr, void *OldV, void *NewV) {
___
cfe-commits mailing 

[PATCH] D44901: [Diag] Avoid emitting a redefinition note if no location is available.

[Matt Davis via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC328712: [Diag] Avoid emitting a redefinition note if no 
location is available. (authored by mattd, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D44901

Files:
  lib/Sema/SemaDecl.cpp
  test/Sema/redefine_extname.c


Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -4057,7 +4057,8 @@
   }
 
   // Redefinition coming from different files or couldn't do better above.
-  Diag(Old->getLocation(), diag::note_previous_definition);
+  if (Old->getLocation().isValid())
+Diag(Old->getLocation(), diag::note_previous_definition);
 }
 
 /// We've just determined that \p Old and \p New both appear to be definitions
Index: test/Sema/redefine_extname.c
===
--- test/Sema/redefine_extname.c
+++ test/Sema/redefine_extname.c
@@ -4,3 +4,4 @@
 #pragma redefine_extname foo_static bar_static
 static int foo_static() { return 1; } // expected-warning {{#pragma 
redefine_extname is applicable to external C declarations only; not applied to 
function 'foo_static'}}
 
+unsigned __int128_t; // expected-error {{redefinition of '__int128_t' as 
different kind of symbol}}


Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -4057,7 +4057,8 @@
   }
 
   // Redefinition coming from different files or couldn't do better above.
-  Diag(Old->getLocation(), diag::note_previous_definition);
+  if (Old->getLocation().isValid())
+Diag(Old->getLocation(), diag::note_previous_definition);
 }
 
 /// We've just determined that \p Old and \p New both appear to be definitions
Index: test/Sema/redefine_extname.c
===
--- test/Sema/redefine_extname.c
+++ test/Sema/redefine_extname.c
@@ -4,3 +4,4 @@
 #pragma redefine_extname foo_static bar_static
 static int foo_static() { return 1; } // expected-warning {{#pragma redefine_extname is applicable to external C declarations only; not applied to function 'foo_static'}}
 
+unsigned __int128_t; // expected-error {{redefinition of '__int128_t' as different kind of symbol}}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44901: [Diag] Avoid emitting a redefinition note if no location is available.

[Matt Davis via Phabricator via cfe-commits]

mattd created this revision.
mattd added reviewers: bruno, rsmith.

The "previous definition is here" note is not helpful if there is no location 
information. The note will reference nothing in such a case. This patch first 
checks to see if there is location data, and if so the note diagnostic is 
emitted.

This fixes PR15409.  The issue in the first comment seems to already be 
resolved. This patch addresses the second example.


https://reviews.llvm.org/D44901

Files:
  lib/Sema/SemaDecl.cpp
  test/Sema/redefine_extname.c


Index: test/Sema/redefine_extname.c
===
--- test/Sema/redefine_extname.c
+++ test/Sema/redefine_extname.c
@@ -4,3 +4,4 @@
 #pragma redefine_extname foo_static bar_static
 static int foo_static() { return 1; } // expected-warning {{#pragma 
redefine_extname is applicable to external C declarations only; not applied to 
function 'foo_static'}}
 
+unsigned __int128_t; // expected-error {{redefinition of '__int128_t' as 
different kind of symbol}}
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -4057,7 +4057,8 @@
   }
 
   // Redefinition coming from different files or couldn't do better above.
-  Diag(Old->getLocation(), diag::note_previous_definition);
+  if (Old->getLocation().isValid())
+Diag(Old->getLocation(), diag::note_previous_definition);
 }
 
 /// We've just determined that \p Old and \p New both appear to be definitions


Index: test/Sema/redefine_extname.c
===
--- test/Sema/redefine_extname.c
+++ test/Sema/redefine_extname.c
@@ -4,3 +4,4 @@
 #pragma redefine_extname foo_static bar_static
 static int foo_static() { return 1; } // expected-warning {{#pragma redefine_extname is applicable to external C declarations only; not applied to function 'foo_static'}}
 
+unsigned __int128_t; // expected-error {{redefinition of '__int128_t' as different kind of symbol}}
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -4057,7 +4057,8 @@
   }
 
   // Redefinition coming from different files or couldn't do better above.
-  Diag(Old->getLocation(), diag::note_previous_definition);
+  if (Old->getLocation().isValid())
+Diag(Old->getLocation(), diag::note_previous_definition);
 }
 
 /// We've just determined that \p Old and \p New both appear to be definitions
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42813: [Debug] Annotate compiler generated range-for loop variables.

[Matt Davis via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC325175: [Debug] Annotate compiler generated range-for loop 
variables. (authored by mattd, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D42813

Files:
  include/clang/Sema/Scope.h
  lib/Sema/SemaStmt.cpp
  test/CodeGenCXX/debug-info-range-for-var-names.cpp
  test/CodeGenCXX/debug-info-scope.cpp
  test/CodeGenCXX/vla.cpp

Index: include/clang/Sema/Scope.h
===
--- include/clang/Sema/Scope.h
+++ include/clang/Sema/Scope.h
@@ -259,6 +259,9 @@
   Scope *getTemplateParamParent() { return TemplateParamParent; }
   const Scope *getTemplateParamParent() const { return TemplateParamParent; }
 
+  /// Returns the depth of this scope. The translation-unit has scope depth 0.
+  unsigned getDepth() const { return Depth; }
+
   /// Returns the number of function prototype scopes in this scope
   /// chain.
   unsigned getFunctionPrototypeDepth() const {
Index: test/CodeGenCXX/vla.cpp
===
--- test/CodeGenCXX/vla.cpp
+++ test/CodeGenCXX/vla.cpp
@@ -68,8 +68,8 @@
 void test2(int b) {
   // CHECK-LABEL: define void {{.*}}test2{{.*}}(i32 %b)
   int varr[b];
-  // AMD: %__end = alloca i32*, align 8, addrspace(5)
-  // AMD: [[END:%.*]] = addrspacecast i32* addrspace(5)* %__end to i32**
+  // AMD: %__end1 = alloca i32*, align 8, addrspace(5)
+  // AMD: [[END:%.*]] = addrspacecast i32* addrspace(5)* %__end1 to i32**
   // get the address of %b by checking the first store that stores it 
   //CHECK: store i32 %b, i32* [[PTR_B:%.*]]
 
@@ -86,16 +86,16 @@
   //CHECK: [[VLA_SIZEOF:%.*]] = mul nuw i64 4, [[VLA_NUM_ELEMENTS_PRE]]
   //CHECK-NEXT: [[VLA_NUM_ELEMENTS_POST:%.*]] = udiv i64 [[VLA_SIZEOF]], 4
   //CHECK-NEXT: [[VLA_END_PTR:%.*]] = getelementptr inbounds i32, i32* {{%.*}}, i64 [[VLA_NUM_ELEMENTS_POST]]
-  //X64-NEXT: store i32* [[VLA_END_PTR]], i32** %__end
+  //X64-NEXT: store i32* [[VLA_END_PTR]], i32** %__end1
   //AMD-NEXT: store i32* [[VLA_END_PTR]], i32** [[END]]
   for (int d : varr) 0;
 }
 
 void test3(int b, int c) {
   // CHECK-LABEL: define void {{.*}}test3{{.*}}(i32 %b, i32 %c)
   int varr[b][c];
-  // AMD: %__end = alloca i32*, align 8, addrspace(5)
-  // AMD: [[END:%.*]] = addrspacecast i32* addrspace(5)* %__end to i32**
+  // AMD: %__end1 = alloca i32*, align 8, addrspace(5)
+  // AMD: [[END:%.*]] = addrspacecast i32* addrspace(5)* %__end1 to i32**
   // get the address of %b by checking the first store that stores it 
   //CHECK: store i32 %b, i32* [[PTR_B:%.*]]
   //CHECK-NEXT: store i32 %c, i32* [[PTR_C:%.*]]
Index: test/CodeGenCXX/debug-info-scope.cpp
===
--- test/CodeGenCXX/debug-info-scope.cpp
+++ test/CodeGenCXX/debug-info-scope.cpp
@@ -58,7 +58,7 @@
   }
 
   int x[] = {1, 2};
-  // CHECK: = !DILocalVariable(name: "__range"
+  // CHECK: = !DILocalVariable(name: "__range1"
   // CHECK-SAME:   scope: [[RANGE_FOR:![0-9]*]]
   // CHECK-NOT:line:
   // CHECK-SAME:   ){{$}}
Index: test/CodeGenCXX/debug-info-range-for-var-names.cpp
===
--- test/CodeGenCXX/debug-info-range-for-var-names.cpp
+++ test/CodeGenCXX/debug-info-range-for-var-names.cpp
@@ -0,0 +1,36 @@
+// RUN: %clang_cc1 -emit-llvm -debug-info-kind=limited %s -o - | FileCheck %s
+
+struct vec {
+  using itr = int*;
+  itr begin() { return nullptr; }
+  itr end() { return nullptr; }
+};
+
+void test() {
+  vec as, bs, cs;
+
+  for (auto a : as)
+for (auto b : bs)
+  for (auto c : cs) {
+  }
+}
+
+// CHECK: define void @_Z4testv()
+// CHECK: call void @llvm.dbg.declare(metadata %struct.vec** {{.*}}, metadata ![[RANGE1:[0-9]+]]
+// CHECK: call void @llvm.dbg.declare(metadata i32** {{.*}}, metadata ![[BEGIN1:[0-9]+]]
+// CHECK: call void @llvm.dbg.declare(metadata i32** {{.*}}, metadata ![[END1:[0-9]+]]
+// CHECK: call void @llvm.dbg.declare(metadata %struct.vec** {{.*}}, metadata ![[RANGE2:[0-9]+]]
+// CHECK: call void @llvm.dbg.declare(metadata i32** {{.*}}, metadata ![[BEGIN2:[0-9]+]]
+// CHECK: call void @llvm.dbg.declare(metadata i32** {{.*}}, metadata ![[END2:[0-9]+]]
+// CHECK: call void @llvm.dbg.declare(metadata %struct.vec** {{.*}}, metadata ![[RANGE3:[0-9]+]]
+// CHECK: call void @llvm.dbg.declare(metadata i32** {{.*}}, metadata ![[BEGIN3:[0-9]+]]
+// CHECK: call void @llvm.dbg.declare(metadata i32** {{.*}}, metadata ![[END3:[0-9]+]]
+// CHECK: ![[RANGE1]] = !DILocalVariable(name: "__range1",
+// CHECK: ![[BEGIN1]] = !DILocalVariable(name: "__begin1",
+// CHECK: ![[END1]] = !DILocalVariable(name: "__end1",
+// CHECK: ![[RANGE2]] = !DILocalVariable(name: "__range2",
+// CHECK: ![[BEGIN2]] = !DILocalVariable(name: "__begin2",
+// CHECK: ![[END2]] = !DILocalVariable(name: "__end2",
+// CHECK: ![[RANGE3]] = !DILocalVariable(name: 

[PATCH] D42813: [Debug] Annotate compiler generated range-for loop variables.

[Matt Davis via Phabricator via cfe-commits]

mattd added a comment.

> Great - can you commit this yourself or would you like me to do it for you?

I've got it.  Thanks!


https://reviews.llvm.org/D42813



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

[PATCH] D42813: [Debug] Annotate compiler generated range-for loop variables.

[Matt Davis via Phabricator via cfe-commits]

mattd updated this revision to Diff 134272.
mattd added a comment.

Thanks @dblaikie!  I renamed the test, and cleaned up per your suggestion.  I 
originally regex'd the debug-info lines so that the test would verify that the 
names were artificial; however, being that we already match them as metadata 
earlier, it's not really that necessary; we are only testing name strings 
anyways.  Thanks again for the suggestion.


https://reviews.llvm.org/D42813

Files:
  include/clang/Sema/Scope.h
  lib/Sema/SemaStmt.cpp
  test/CodeGenCXX/debug-info-range-for-var-names.cpp
  test/CodeGenCXX/debug-info-scope.cpp
  test/CodeGenCXX/vla.cpp

Index: test/CodeGenCXX/vla.cpp
===
--- test/CodeGenCXX/vla.cpp
+++ test/CodeGenCXX/vla.cpp
@@ -68,8 +68,8 @@
 void test2(int b) {
   // CHECK-LABEL: define void {{.*}}test2{{.*}}(i32 %b)
   int varr[b];
-  // AMD: %__end = alloca i32*, align 8, addrspace(5)
-  // AMD: [[END:%.*]] = addrspacecast i32* addrspace(5)* %__end to i32**
+  // AMD: %__end1 = alloca i32*, align 8, addrspace(5)
+  // AMD: [[END:%.*]] = addrspacecast i32* addrspace(5)* %__end1 to i32**
   // get the address of %b by checking the first store that stores it 
   //CHECK: store i32 %b, i32* [[PTR_B:%.*]]
 
@@ -86,7 +86,7 @@
   //CHECK: [[VLA_SIZEOF:%.*]] = mul nuw i64 4, [[VLA_NUM_ELEMENTS_PRE]]
   //CHECK-NEXT: [[VLA_NUM_ELEMENTS_POST:%.*]] = udiv i64 [[VLA_SIZEOF]], 4
   //CHECK-NEXT: [[VLA_END_PTR:%.*]] = getelementptr inbounds i32, i32* {{%.*}}, i64 [[VLA_NUM_ELEMENTS_POST]]
-  //X64-NEXT: store i32* [[VLA_END_PTR]], i32** %__end
+  //X64-NEXT: store i32* [[VLA_END_PTR]], i32** %__end1
   //AMD-NEXT: store i32* [[VLA_END_PTR]], i32** [[END]]
   for (int d : varr) 0;
 }
@@ -94,8 +94,8 @@
 void test3(int b, int c) {
   // CHECK-LABEL: define void {{.*}}test3{{.*}}(i32 %b, i32 %c)
   int varr[b][c];
-  // AMD: %__end = alloca i32*, align 8, addrspace(5)
-  // AMD: [[END:%.*]] = addrspacecast i32* addrspace(5)* %__end to i32**
+  // AMD: %__end1 = alloca i32*, align 8, addrspace(5)
+  // AMD: [[END:%.*]] = addrspacecast i32* addrspace(5)* %__end1 to i32**
   // get the address of %b by checking the first store that stores it 
   //CHECK: store i32 %b, i32* [[PTR_B:%.*]]
   //CHECK-NEXT: store i32 %c, i32* [[PTR_C:%.*]]
Index: test/CodeGenCXX/debug-info-scope.cpp
===
--- test/CodeGenCXX/debug-info-scope.cpp
+++ test/CodeGenCXX/debug-info-scope.cpp
@@ -58,7 +58,7 @@
   }
 
   int x[] = {1, 2};
-  // CHECK: = !DILocalVariable(name: "__range"
+  // CHECK: = !DILocalVariable(name: "__range1"
   // CHECK-SAME:   scope: [[RANGE_FOR:![0-9]*]]
   // CHECK-NOT:line:
   // CHECK-SAME:   ){{$}}
Index: test/CodeGenCXX/debug-info-range-for-var-names.cpp
===
--- test/CodeGenCXX/debug-info-range-for-var-names.cpp
+++ test/CodeGenCXX/debug-info-range-for-var-names.cpp
@@ -0,0 +1,36 @@
+// RUN: %clang_cc1 -emit-llvm -debug-info-kind=limited %s -o - | FileCheck %s
+
+struct vec {
+  using itr = int*;
+  itr begin() { return nullptr; }
+  itr end() { return nullptr; }
+};
+
+void test() {
+  vec as, bs, cs;
+
+  for (auto a : as)
+for (auto b : bs)
+  for (auto c : cs) {
+  }
+}
+
+// CHECK: define void @_Z4testv()
+// CHECK: call void @llvm.dbg.declare(metadata %struct.vec** {{.*}}, metadata ![[RANGE1:[0-9]+]]
+// CHECK: call void @llvm.dbg.declare(metadata i32** {{.*}}, metadata ![[BEGIN1:[0-9]+]]
+// CHECK: call void @llvm.dbg.declare(metadata i32** {{.*}}, metadata ![[END1:[0-9]+]]
+// CHECK: call void @llvm.dbg.declare(metadata %struct.vec** {{.*}}, metadata ![[RANGE2:[0-9]+]]
+// CHECK: call void @llvm.dbg.declare(metadata i32** {{.*}}, metadata ![[BEGIN2:[0-9]+]]
+// CHECK: call void @llvm.dbg.declare(metadata i32** {{.*}}, metadata ![[END2:[0-9]+]]
+// CHECK: call void @llvm.dbg.declare(metadata %struct.vec** {{.*}}, metadata ![[RANGE3:[0-9]+]]
+// CHECK: call void @llvm.dbg.declare(metadata i32** {{.*}}, metadata ![[BEGIN3:[0-9]+]]
+// CHECK: call void @llvm.dbg.declare(metadata i32** {{.*}}, metadata ![[END3:[0-9]+]]
+// CHECK: ![[RANGE1]] = !DILocalVariable(name: "__range1",
+// CHECK: ![[BEGIN1]] = !DILocalVariable(name: "__begin1",
+// CHECK: ![[END1]] = !DILocalVariable(name: "__end1",
+// CHECK: ![[RANGE2]] = !DILocalVariable(name: "__range2",
+// CHECK: ![[BEGIN2]] = !DILocalVariable(name: "__begin2",
+// CHECK: ![[END2]] = !DILocalVariable(name: "__end2",
+// CHECK: ![[RANGE3]] = !DILocalVariable(name: "__range3",
+// CHECK: ![[BEGIN3]] = !DILocalVariable(name: "__begin3",
+// CHECK: ![[END3]] = !DILocalVariable(name: "__end3",
Index: lib/Sema/SemaStmt.cpp
===
--- lib/Sema/SemaStmt.cpp
+++ lib/Sema/SemaStmt.cpp
@@ -2025,7 +2025,7 @@
 
 /// Build a variable declaration for a for-range statement.
 VarDecl 

[PATCH] D42813: [Debug] Annotate compiler generated range-for loop variables.

[Matt Davis via Phabricator via cfe-commits]

mattd updated this revision to Diff 134263.
mattd added a comment.

- Added a division by 2 instead of the shift, it reads clearer.
- Updated the associated comment, reflecting that we divide by two because the 
variables we are annotating are within the inner scope of the ranged-based for 
loop.


https://reviews.llvm.org/D42813

Files:
  include/clang/Sema/Scope.h
  lib/Sema/SemaStmt.cpp
  test/CodeGenCXX/debug-for-range-scope-hints.cpp
  test/CodeGenCXX/debug-info-scope.cpp
  test/CodeGenCXX/vla.cpp

Index: test/CodeGenCXX/vla.cpp
===
--- test/CodeGenCXX/vla.cpp
+++ test/CodeGenCXX/vla.cpp
@@ -68,8 +68,8 @@
 void test2(int b) {
   // CHECK-LABEL: define void {{.*}}test2{{.*}}(i32 %b)
   int varr[b];
-  // AMD: %__end = alloca i32*, align 8, addrspace(5)
-  // AMD: [[END:%.*]] = addrspacecast i32* addrspace(5)* %__end to i32**
+  // AMD: %__end1 = alloca i32*, align 8, addrspace(5)
+  // AMD: [[END:%.*]] = addrspacecast i32* addrspace(5)* %__end1 to i32**
   // get the address of %b by checking the first store that stores it 
   //CHECK: store i32 %b, i32* [[PTR_B:%.*]]
 
@@ -86,7 +86,7 @@
   //CHECK: [[VLA_SIZEOF:%.*]] = mul nuw i64 4, [[VLA_NUM_ELEMENTS_PRE]]
   //CHECK-NEXT: [[VLA_NUM_ELEMENTS_POST:%.*]] = udiv i64 [[VLA_SIZEOF]], 4
   //CHECK-NEXT: [[VLA_END_PTR:%.*]] = getelementptr inbounds i32, i32* {{%.*}}, i64 [[VLA_NUM_ELEMENTS_POST]]
-  //X64-NEXT: store i32* [[VLA_END_PTR]], i32** %__end
+  //X64-NEXT: store i32* [[VLA_END_PTR]], i32** %__end1
   //AMD-NEXT: store i32* [[VLA_END_PTR]], i32** [[END]]
   for (int d : varr) 0;
 }
@@ -94,8 +94,8 @@
 void test3(int b, int c) {
   // CHECK-LABEL: define void {{.*}}test3{{.*}}(i32 %b, i32 %c)
   int varr[b][c];
-  // AMD: %__end = alloca i32*, align 8, addrspace(5)
-  // AMD: [[END:%.*]] = addrspacecast i32* addrspace(5)* %__end to i32**
+  // AMD: %__end1 = alloca i32*, align 8, addrspace(5)
+  // AMD: [[END:%.*]] = addrspacecast i32* addrspace(5)* %__end1 to i32**
   // get the address of %b by checking the first store that stores it 
   //CHECK: store i32 %b, i32* [[PTR_B:%.*]]
   //CHECK-NEXT: store i32 %c, i32* [[PTR_C:%.*]]
Index: test/CodeGenCXX/debug-info-scope.cpp
===
--- test/CodeGenCXX/debug-info-scope.cpp
+++ test/CodeGenCXX/debug-info-scope.cpp
@@ -58,7 +58,7 @@
   }
 
   int x[] = {1, 2};
-  // CHECK: = !DILocalVariable(name: "__range"
+  // CHECK: = !DILocalVariable(name: "__range1"
   // CHECK-SAME:   scope: [[RANGE_FOR:![0-9]*]]
   // CHECK-NOT:line:
   // CHECK-SAME:   ){{$}}
Index: test/CodeGenCXX/debug-for-range-scope-hints.cpp
===
--- test/CodeGenCXX/debug-for-range-scope-hints.cpp
+++ test/CodeGenCXX/debug-for-range-scope-hints.cpp
@@ -0,0 +1,36 @@
+// RUN: %clang_cc1 -emit-llvm -debug-info-kind=limited %s -o - | FileCheck %s
+
+struct vec {
+  using itr = int*;
+  itr begin() { return nullptr; }
+  itr end() { return nullptr; }
+};
+
+void test() {
+  vec as, bs, cs;
+
+  for (auto a : as)
+for (auto b : bs)
+  for (auto c : cs) {
+  }
+}
+
+// CHECK: define void @_Z4testv()
+// CHECK: call void @llvm.dbg.declare(metadata %struct.vec** {{.*}}, metadata ![[RANGE1:[0-9]+]]
+// CHECK: call void @llvm.dbg.declare(metadata i32** {{.*}}, metadata ![[BEGIN1:[0-9]+]]
+// CHECK: call void @llvm.dbg.declare(metadata i32** {{.*}}, metadata ![[END1:[0-9]+]]
+// CHECK: call void @llvm.dbg.declare(metadata %struct.vec** {{.*}}, metadata ![[RANGE2:[0-9]+]]
+// CHECK: call void @llvm.dbg.declare(metadata i32** {{.*}}, metadata ![[BEGIN2:[0-9]+]]
+// CHECK: call void @llvm.dbg.declare(metadata i32** {{.*}}, metadata ![[END2:[0-9]+]]
+// CHECK: call void @llvm.dbg.declare(metadata %struct.vec** {{.*}}, metadata ![[RANGE3:[0-9]+]]
+// CHECK: call void @llvm.dbg.declare(metadata i32** {{.*}}, metadata ![[BEGIN3:[0-9]+]]
+// CHECK: call void @llvm.dbg.declare(metadata i32** {{.*}}, metadata ![[END3:[0-9]+]]
+// CHECK: ![[RANGE1]] = !DILocalVariable(name: "__range1", {{.*}}, flags: DIFlagArtificial)
+// CHECK: ![[BEGIN1]] = !DILocalVariable(name: "__begin1", {{.*}}, flags: DIFlagArtificial)
+// CHECK: ![[END1]] = !DILocalVariable(name: "__end1",  {{.*}}, flags: DIFlagArtificial)
+// CHECK: ![[RANGE2]] = !DILocalVariable(name: "__range2",  {{.*}}, flags: DIFlagArtificial)
+// CHECK: ![[BEGIN2]] = !DILocalVariable(name: "__begin2", {{.*}}, flags: DIFlagArtificial)
+// CHECK: ![[END2]] = !DILocalVariable(name: "__end2",  {{.*}}, flags: DIFlagArtificial)
+// CHECK: ![[RANGE3]] = !DILocalVariable(name: "__range3",  {{.*}}, flags: DIFlagArtificial)
+// CHECK: ![[BEGIN3]] = !DILocalVariable(name: "__begin3", {{.*}}, flags: DIFlagArtificial)
+// CHECK: ![[END3]] = !DILocalVariable(name: "__end3", {{.*}}, flags: DIFlagArtificial)
Index: lib/Sema/SemaStmt.cpp

[PATCH] D42549: [CodeGen] Use the zero initializer instead of storing an all zero representation.

[Matt Davis via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC324776: [CodeGen] Use the zero initializer instead of 
storing an all zero… (authored by mattd, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D42549

Files:
  lib/CodeGen/CGExprConstant.cpp
  test/CodeGen/array-init.c


Index: test/CodeGen/array-init.c
===
--- test/CodeGen/array-init.c
+++ test/CodeGen/array-init.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 %s -O0 -triple x86_64-unknown-linux-gnu -emit-llvm -o - | 
FileCheck %s
+
+// CHECK: @{{.*}}.a1 = internal constant [5 x i32] [i32 0, i32 1, i32 2, i32 
0, i32 0]
+// CHECK: @{{.*}}.a2 = internal constant [5 x i32] zeroinitializer
+// CHECK: @{{.*}}.a3 = internal constant [5 x i32] zeroinitializer
+
+void testConstArrayInits(void)
+{
+  const int a1[5] = {0,1,2};
+  const int a2[5] = {0,0,0};
+  const int a3[5] = {0};
+}
Index: lib/CodeGen/CGExprConstant.cpp
===
--- lib/CodeGen/CGExprConstant.cpp
+++ lib/CodeGen/CGExprConstant.cpp
@@ -859,25 +859,33 @@
 
 // Copy initializer elements.
 SmallVector Elts;
-Elts.reserve(NumInitableElts + NumElements);
+Elts.reserve(std::max(NumInitableElts, NumElements));
 
 bool RewriteType = false;
+bool AllNullValues = true;
 for (unsigned i = 0; i < NumInitableElts; ++i) {
   Expr *Init = ILE->getInit(i);
   llvm::Constant *C = Emitter.tryEmitPrivateForMemory(Init, EltType);
   if (!C)
 return nullptr;
   RewriteType |= (C->getType() != ElemTy);
   Elts.push_back(C);
+  if (AllNullValues && !C->isNullValue())
+AllNullValues = false;
 }
 
+// If all initializer elements are "zero," then avoid storing NumElements
+// instances of the zero representation.
+if (AllNullValues)
+  return llvm::ConstantAggregateZero::get(AType);
+
 RewriteType |= (fillC->getType() != ElemTy);
 Elts.resize(NumElements, fillC);
 
 if (RewriteType) {
   // FIXME: Try to avoid packing the array
   std::vector Types;
-  Types.reserve(NumInitableElts + NumElements);
+  Types.reserve(Elts.size());
   for (unsigned i = 0, e = Elts.size(); i < e; ++i)
 Types.push_back(Elts[i]->getType());
   llvm::StructType *SType = llvm::StructType::get(AType->getContext(),


Index: test/CodeGen/array-init.c
===
--- test/CodeGen/array-init.c
+++ test/CodeGen/array-init.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 %s -O0 -triple x86_64-unknown-linux-gnu -emit-llvm -o - | FileCheck %s
+
+// CHECK: @{{.*}}.a1 = internal constant [5 x i32] [i32 0, i32 1, i32 2, i32 0, i32 0]
+// CHECK: @{{.*}}.a2 = internal constant [5 x i32] zeroinitializer
+// CHECK: @{{.*}}.a3 = internal constant [5 x i32] zeroinitializer
+
+void testConstArrayInits(void)
+{
+  const int a1[5] = {0,1,2};
+  const int a2[5] = {0,0,0};
+  const int a3[5] = {0};
+}
Index: lib/CodeGen/CGExprConstant.cpp
===
--- lib/CodeGen/CGExprConstant.cpp
+++ lib/CodeGen/CGExprConstant.cpp
@@ -859,25 +859,33 @@
 
 // Copy initializer elements.
 SmallVector Elts;
-Elts.reserve(NumInitableElts + NumElements);
+Elts.reserve(std::max(NumInitableElts, NumElements));
 
 bool RewriteType = false;
+bool AllNullValues = true;
 for (unsigned i = 0; i < NumInitableElts; ++i) {
   Expr *Init = ILE->getInit(i);
   llvm::Constant *C = Emitter.tryEmitPrivateForMemory(Init, EltType);
   if (!C)
 return nullptr;
   RewriteType |= (C->getType() != ElemTy);
   Elts.push_back(C);
+  if (AllNullValues && !C->isNullValue())
+AllNullValues = false;
 }
 
+// If all initializer elements are "zero," then avoid storing NumElements
+// instances of the zero representation.
+if (AllNullValues)
+  return llvm::ConstantAggregateZero::get(AType);
+
 RewriteType |= (fillC->getType() != ElemTy);
 Elts.resize(NumElements, fillC);
 
 if (RewriteType) {
   // FIXME: Try to avoid packing the array
   std::vector Types;
-  Types.reserve(NumInitableElts + NumElements);
+  Types.reserve(Elts.size());
   for (unsigned i = 0, e = Elts.size(); i < e; ++i)
 Types.push_back(Elts[i]->getType());
   llvm::StructType *SType = llvm::StructType::get(AType->getContext(),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42549: [CodeGen] Use the zero initializer instead of storing an all zero representation.

[Matt Davis via Phabricator via cfe-commits]

mattd updated this revision to Diff 133540.
mattd added a comment.

Thanks for the test tips.  I realize the original was a bit carried away,  my 
apologies for that.  This updated test visits the same code path that we're 
trying to test, and is much more concise.


https://reviews.llvm.org/D42549

Files:
  lib/CodeGen/CGExprConstant.cpp
  test/CodeGen/array-init.c


Index: test/CodeGen/array-init.c
===
--- test/CodeGen/array-init.c
+++ test/CodeGen/array-init.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 %s -O0 -triple x86_64-unknown-linux-gnu -emit-llvm -o - | 
FileCheck %s
+
+// CHECK: @{{.*}}.a1 = internal constant [5 x i32] [i32 0, i32 1, i32 2, i32 
0, i32 0]
+// CHECK: @{{.*}}.a2 = internal constant [5 x i32] zeroinitializer
+// CHECK: @{{.*}}.a3 = internal constant [5 x i32] zeroinitializer
+
+void testConstArrayInits(void)
+{
+  const int a1[5] = {0,1,2};
+  const int a2[5] = {0,0,0};
+  const int a3[5] = {0};
+}
Index: lib/CodeGen/CGExprConstant.cpp
===
--- lib/CodeGen/CGExprConstant.cpp
+++ lib/CodeGen/CGExprConstant.cpp
@@ -859,9 +859,10 @@
 
 // Copy initializer elements.
 SmallVector Elts;
-Elts.reserve(NumInitableElts + NumElements);
+Elts.reserve(std::max(NumInitableElts, NumElements));
 
 bool RewriteType = false;
+bool AllNullValues = true;
 for (unsigned i = 0; i < NumInitableElts; ++i) {
   Expr *Init = ILE->getInit(i);
   llvm::Constant *C = Emitter.tryEmitPrivateForMemory(Init, EltType);
@@ -869,8 +870,15 @@
 return nullptr;
   RewriteType |= (C->getType() != ElemTy);
   Elts.push_back(C);
+  if (AllNullValues && !C->isNullValue())
+AllNullValues = false;
 }
 
+// If all initializer elements are "zero," then avoid storing NumElements
+// instances of the zero representation.
+if (AllNullValues)
+  return llvm::ConstantAggregateZero::get(AType);
+
 RewriteType |= (fillC->getType() != ElemTy);
 Elts.resize(NumElements, fillC);
 
@@ -877,7 +885,7 @@
 if (RewriteType) {
   // FIXME: Try to avoid packing the array
   std::vector Types;
-  Types.reserve(NumInitableElts + NumElements);
+  Types.reserve(Elts.size());
   for (unsigned i = 0, e = Elts.size(); i < e; ++i)
 Types.push_back(Elts[i]->getType());
   llvm::StructType *SType = llvm::StructType::get(AType->getContext(),


Index: test/CodeGen/array-init.c
===
--- test/CodeGen/array-init.c
+++ test/CodeGen/array-init.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 %s -O0 -triple x86_64-unknown-linux-gnu -emit-llvm -o - | FileCheck %s
+
+// CHECK: @{{.*}}.a1 = internal constant [5 x i32] [i32 0, i32 1, i32 2, i32 0, i32 0]
+// CHECK: @{{.*}}.a2 = internal constant [5 x i32] zeroinitializer
+// CHECK: @{{.*}}.a3 = internal constant [5 x i32] zeroinitializer
+
+void testConstArrayInits(void)
+{
+  const int a1[5] = {0,1,2};
+  const int a2[5] = {0,0,0};
+  const int a3[5] = {0};
+}
Index: lib/CodeGen/CGExprConstant.cpp
===
--- lib/CodeGen/CGExprConstant.cpp
+++ lib/CodeGen/CGExprConstant.cpp
@@ -859,9 +859,10 @@
 
 // Copy initializer elements.
 SmallVector Elts;
-Elts.reserve(NumInitableElts + NumElements);
+Elts.reserve(std::max(NumInitableElts, NumElements));
 
 bool RewriteType = false;
+bool AllNullValues = true;
 for (unsigned i = 0; i < NumInitableElts; ++i) {
   Expr *Init = ILE->getInit(i);
   llvm::Constant *C = Emitter.tryEmitPrivateForMemory(Init, EltType);
@@ -869,8 +870,15 @@
 return nullptr;
   RewriteType |= (C->getType() != ElemTy);
   Elts.push_back(C);
+  if (AllNullValues && !C->isNullValue())
+AllNullValues = false;
 }
 
+// If all initializer elements are "zero," then avoid storing NumElements
+// instances of the zero representation.
+if (AllNullValues)
+  return llvm::ConstantAggregateZero::get(AType);
+
 RewriteType |= (fillC->getType() != ElemTy);
 Elts.resize(NumElements, fillC);
 
@@ -877,7 +885,7 @@
 if (RewriteType) {
   // FIXME: Try to avoid packing the array
   std::vector Types;
-  Types.reserve(NumInitableElts + NumElements);
+  Types.reserve(Elts.size());
   for (unsigned i = 0, e = Elts.size(); i < e; ++i)
 Types.push_back(Elts[i]->getType());
   llvm::StructType *SType = llvm::StructType::get(AType->getContext(),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42549: [CodeGen] Use the zero initializer instead of storing an all zero representation.

[Matt Davis via Phabricator via cfe-commits]

mattd updated this revision to Diff 133497.
mattd added a comment.

Thanks for the look @rjmccall .  I tossed in the shortcut as you suggested.  I 
also had to fix the instruction numbering in the test case.


https://reviews.llvm.org/D42549

Files:
  lib/CodeGen/CGExprConstant.cpp
  test/CodeGen/array-init.c

Index: test/CodeGen/array-init.c
===
--- test/CodeGen/array-init.c
+++ test/CodeGen/array-init.c
@@ -0,0 +1,66 @@
+// RUN: %clang_cc1 %s -O0 -triple x86_64-unknown-linux-gnu -emit-llvm -o - | FileCheck %s
+
+// CHECK: @test.a1 = internal global [10 x i32] [i32 0, i32 1, i32 2, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0], align 16
+// CHECK: @test.a2 = internal global [10 x i32] zeroinitializer, align 16
+// CHECK: @test.a3 = internal global [10 x i32] zeroinitializer, align 16
+// CHECK: @test.a4 = internal global [10 x i32] zeroinitializer, align 16
+// CHECK: @test.b1 = internal constant [10 x i32] [i32 0, i32 1, i32 2, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0], align 16
+// CHECK: @test.b2 = internal constant [10 x i32] zeroinitializer, align 16
+// CHECK: @test.b3 = internal constant [10 x i32] zeroinitializer, align 16
+
+// CHECK: define void @test()
+// CHECK: entry:
+// CHECK:   %c1 = alloca [10 x i32], align 16
+// CHECK:   %c2 = alloca [10 x i32], align 16
+// CHECK:   %c3 = alloca [10 x i32], align 16
+// CHECK:   %c4 = alloca [10 x i32], align 16
+// CHECK:   %d1 = alloca [10 x i32], align 16
+// CHECK:   %d2 = alloca [10 x i32], align 16
+// CHECK:   %d3 = alloca [10 x i32], align 16
+// CHECK:   %d4 = alloca [10 x i32], align 16
+// CHECK:   %0 = bitcast [10 x i32]* %c1 to i8*
+// CHECK:   call void @llvm.memset.p0i8.i64(i8* align 16 %0, i8 0, i64 40, i1 false)
+// CHECK:   %1 = bitcast i8* %0 to [10 x i32]*
+// CHECK:   %2 = getelementptr [10 x i32], [10 x i32]* %1, i32 0, i32 1
+// CHECK:   store i32 1, i32* %2
+// CHECK:   %3 = getelementptr [10 x i32], [10 x i32]* %1, i32 0, i32 2
+// CHECK:   store i32 2, i32* %3
+// CHECK:   %4 = bitcast [10 x i32]* %c2 to i8*
+// CHECK:   call void @llvm.memset.p0i8.i64(i8* align 16 %4, i8 0, i64 40, i1 false)
+// CHECK:   %5 = bitcast [10 x i32]* %c3 to i8*
+// CHECK:   call void @llvm.memset.p0i8.i64(i8* align 16 %5, i8 0, i64 40, i1 false)
+// CHECK:   %6 = bitcast [10 x i32]* %d1 to i8*
+// CHECK:   call void @llvm.memset.p0i8.i64(i8* align 16 %6, i8 0, i64 40, i1 true)
+// CHECK:   %7 = bitcast i8* %6 to [10 x i32]*
+// CHECK:   %8 = getelementptr [10 x i32], [10 x i32]* %7, i32 0, i32 1
+// CHECK:   store volatile i32 1, i32* %8
+// CHECK:   %9 = getelementptr [10 x i32], [10 x i32]* %7, i32 0, i32 2
+// CHECK:   store volatile i32 2, i32* %9
+// CHECK:   %10 = bitcast [10 x i32]* %d2 to i8*
+// CHECK:   call void @llvm.memset.p0i8.i64(i8* align 16 %10, i8 0, i64 40, i1 true)
+// CHECK:   %11 = bitcast [10 x i32]* %d3 to i8*
+// CHECK:   call void @llvm.memset.p0i8.i64(i8* align 16 %11, i8 0, i64 40, i1 true)
+// CHECK:   ret void
+// CHECK: }
+
+void test()
+{
+  static int a1[10] = {0,1,2};
+  static int a2[10] = {0,0,0};
+  static int a3[10] = {0};
+  static int a4[10];
+
+  const int b1[10] = {0,1,2};
+  const int b2[10] = {0,0,0};
+  const int b3[10] = {0};
+  
+  int c1[10] = {0,1,2};
+  int c2[10] = {0,0,0};
+  int c3[10] = {0};
+  int c4[10];
+  
+  volatile int d1[10] = {0,1,2};
+  volatile int d2[10] = {0,0,0};
+  volatile int d3[10] = {0};
+  volatile int d4[10];
+}
Index: lib/CodeGen/CGExprConstant.cpp
===
--- lib/CodeGen/CGExprConstant.cpp
+++ lib/CodeGen/CGExprConstant.cpp
@@ -859,9 +859,10 @@
 
 // Copy initializer elements.
 SmallVector Elts;
-Elts.reserve(NumInitableElts + NumElements);
+Elts.reserve(std::max(NumInitableElts, NumElements));
 
 bool RewriteType = false;
+bool AllNullValues = true;
 for (unsigned i = 0; i < NumInitableElts; ++i) {
   Expr *Init = ILE->getInit(i);
   llvm::Constant *C = Emitter.tryEmitPrivateForMemory(Init, EltType);
@@ -869,8 +870,15 @@
 return nullptr;
   RewriteType |= (C->getType() != ElemTy);
   Elts.push_back(C);
+  if (AllNullValues && !C->isNullValue())
+AllNullValues = false;
 }
 
+// If all initializer elements are "zero," then avoid storing NumElements
+// instances of the zero representation.
+if (AllNullValues)
+  return llvm::ConstantAggregateZero::get(AType);
+
 RewriteType |= (fillC->getType() != ElemTy);
 Elts.resize(NumElements, fillC);
 
@@ -877,7 +885,7 @@
 if (RewriteType) {
   // FIXME: Try to avoid packing the array
   std::vector Types;
-  Types.reserve(NumInitableElts + NumElements);
+  Types.reserve(Elts.size());
   for (unsigned i = 0, e = Elts.size(); i < e; ++i)
 Types.push_back(Elts[i]->getType());
   llvm::StructType *SType = 

[PATCH] D42813: [Debug] Annotate compiler generated range-for loop variables.

[Matt Davis via Phabricator via cfe-commits]

mattd added inline comments.



Comment at: lib/Sema/SemaStmt.cpp:2346
+// Assume the variables are nested in the inner scope (loop body).
+const auto DepthStr = std::to_string(S->getDepth() >> 1);
 VarDecl *BeginVar = BuildForRangeVarDecl(*this, ColonLoc, AutoType,

dblaikie wrote:
> Why shifted right/divided by two? (I'd probably write this as "/ 2" rather 
> thane ">> 1" if this is what's)
> 
> I guess this is because a range-for introduces two (actually 3, I think) 
> scopes?
> 
> But what about if there are other random scopes that could be present? Does 
> that adversely affect anything here?
Thanks for the comment.  Honestly, I discovered the pattern after crafting up a 
tightly nested loop example, and realized that my Depths were 2,4,6... for each 
range loop respectively.   I think it's two scopes and the range variables 
(__begin, __end, __range) are nestled inside each inner scope.  Thus each range 
loop seems to be created of 2 scopes.

If there are other scopes between two range-loops, such as the 'if' in the 
example below, then the outer-most "for" is range1/begin1/end1, followed by the 
middle loop: range3/begin3/end3, and the innermost: range4/begin4/end4.

```
for (...) // range1
  if (...) 
for(...) // range 3
  for (...) //range4
```
   


https://reviews.llvm.org/D42813



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

[PATCH] D42813: [Debug] Annotate compiler generated range-for loop variables.

[Matt Davis via Phabricator via cfe-commits]

mattd updated this revision to Diff 132464.
mattd added a comment.

Updating the diff, missed a few deltas that should have been in the original 
patch.


https://reviews.llvm.org/D42813

Files:
  include/clang/Sema/Scope.h
  lib/Sema/SemaStmt.cpp
  test/CodeGenCXX/debug-for-range-scope-hints.cpp
  test/CodeGenCXX/debug-info-scope.cpp
  test/CodeGenCXX/vla.cpp

Index: test/CodeGenCXX/vla.cpp
===
--- test/CodeGenCXX/vla.cpp
+++ test/CodeGenCXX/vla.cpp
@@ -68,8 +68,8 @@
 void test2(int b) {
   // CHECK-LABEL: define void {{.*}}test2{{.*}}(i32 %b)
   int varr[b];
-  // AMD: %__end = alloca i32*, align 8, addrspace(5)
-  // AMD: [[END:%.*]] = addrspacecast i32* addrspace(5)* %__end to i32**
+  // AMD: %__end1 = alloca i32*, align 8, addrspace(5)
+  // AMD: [[END:%.*]] = addrspacecast i32* addrspace(5)* %__end1 to i32**
   // get the address of %b by checking the first store that stores it 
   //CHECK: store i32 %b, i32* [[PTR_B:%.*]]
 
@@ -86,7 +86,7 @@
   //CHECK: [[VLA_SIZEOF:%.*]] = mul nuw i64 4, [[VLA_NUM_ELEMENTS_PRE]]
   //CHECK-NEXT: [[VLA_NUM_ELEMENTS_POST:%.*]] = udiv i64 [[VLA_SIZEOF]], 4
   //CHECK-NEXT: [[VLA_END_PTR:%.*]] = getelementptr inbounds i32, i32* {{%.*}}, i64 [[VLA_NUM_ELEMENTS_POST]]
-  //X64-NEXT: store i32* [[VLA_END_PTR]], i32** %__end
+  //X64-NEXT: store i32* [[VLA_END_PTR]], i32** %__end1
   //AMD-NEXT: store i32* [[VLA_END_PTR]], i32** [[END]]
   for (int d : varr) 0;
 }
@@ -94,8 +94,8 @@
 void test3(int b, int c) {
   // CHECK-LABEL: define void {{.*}}test3{{.*}}(i32 %b, i32 %c)
   int varr[b][c];
-  // AMD: %__end = alloca i32*, align 8, addrspace(5)
-  // AMD: [[END:%.*]] = addrspacecast i32* addrspace(5)* %__end to i32**
+  // AMD: %__end1 = alloca i32*, align 8, addrspace(5)
+  // AMD: [[END:%.*]] = addrspacecast i32* addrspace(5)* %__end1 to i32**
   // get the address of %b by checking the first store that stores it 
   //CHECK: store i32 %b, i32* [[PTR_B:%.*]]
   //CHECK-NEXT: store i32 %c, i32* [[PTR_C:%.*]]
Index: test/CodeGenCXX/debug-info-scope.cpp
===
--- test/CodeGenCXX/debug-info-scope.cpp
+++ test/CodeGenCXX/debug-info-scope.cpp
@@ -58,7 +58,7 @@
   }
 
   int x[] = {1, 2};
-  // CHECK: = !DILocalVariable(name: "__range"
+  // CHECK: = !DILocalVariable(name: "__range1"
   // CHECK-SAME:   scope: [[RANGE_FOR:![0-9]*]]
   // CHECK-NOT:line:
   // CHECK-SAME:   ){{$}}
Index: test/CodeGenCXX/debug-for-range-scope-hints.cpp
===
--- test/CodeGenCXX/debug-for-range-scope-hints.cpp
+++ test/CodeGenCXX/debug-for-range-scope-hints.cpp
@@ -0,0 +1,36 @@
+// RUN: %clang_cc1 -emit-llvm -debug-info-kind=limited %s -o - | FileCheck %s
+
+struct vec {
+  using itr = int*;
+  itr begin() { return nullptr; }
+  itr end() { return nullptr; }
+};
+
+void test() {
+  vec as, bs, cs;
+
+  for (auto a : as)
+for (auto b : bs)
+  for (auto c : cs) {
+  }
+}
+
+// CHECK: define void @_Z4testv()
+// CHECK: call void @llvm.dbg.declare(metadata %struct.vec** {{.*}}, metadata ![[RANGE1:[0-9]+]]
+// CHECK: call void @llvm.dbg.declare(metadata i32** {{.*}}, metadata ![[BEGIN1:[0-9]+]]
+// CHECK: call void @llvm.dbg.declare(metadata i32** {{.*}}, metadata ![[END1:[0-9]+]]
+// CHECK: call void @llvm.dbg.declare(metadata %struct.vec** {{.*}}, metadata ![[RANGE2:[0-9]+]]
+// CHECK: call void @llvm.dbg.declare(metadata i32** {{.*}}, metadata ![[BEGIN2:[0-9]+]]
+// CHECK: call void @llvm.dbg.declare(metadata i32** {{.*}}, metadata ![[END2:[0-9]+]]
+// CHECK: call void @llvm.dbg.declare(metadata %struct.vec** {{.*}}, metadata ![[RANGE3:[0-9]+]]
+// CHECK: call void @llvm.dbg.declare(metadata i32** {{.*}}, metadata ![[BEGIN3:[0-9]+]]
+// CHECK: call void @llvm.dbg.declare(metadata i32** {{.*}}, metadata ![[END3:[0-9]+]]
+// CHECK: ![[RANGE1]] = !DILocalVariable(name: "__range1", {{.*}}, flags: DIFlagArtificial)
+// CHECK: ![[BEGIN1]] = !DILocalVariable(name: "__begin1", {{.*}}, flags: DIFlagArtificial)
+// CHECK: ![[END1]] = !DILocalVariable(name: "__end1",  {{.*}}, flags: DIFlagArtificial)
+// CHECK: ![[RANGE2]] = !DILocalVariable(name: "__range2",  {{.*}}, flags: DIFlagArtificial)
+// CHECK: ![[BEGIN2]] = !DILocalVariable(name: "__begin2", {{.*}}, flags: DIFlagArtificial)
+// CHECK: ![[END2]] = !DILocalVariable(name: "__end2",  {{.*}}, flags: DIFlagArtificial)
+// CHECK: ![[RANGE3]] = !DILocalVariable(name: "__range3",  {{.*}}, flags: DIFlagArtificial)
+// CHECK: ![[BEGIN3]] = !DILocalVariable(name: "__begin3", {{.*}}, flags: DIFlagArtificial)
+// CHECK: ![[END3]] = !DILocalVariable(name: "__end3", {{.*}}, flags: DIFlagArtificial)
Index: lib/Sema/SemaStmt.cpp
===
--- lib/Sema/SemaStmt.cpp
+++ lib/Sema/SemaStmt.cpp
@@ -2025,7 +2025,7 @@
 
 /// Build a variable declaration for a for-range statement.

[PATCH] D42813: [Debug] Annotate compiler generated range-for loop variables.

[Matt Davis via Phabricator via cfe-commits]

mattd created this revision.
mattd added a reviewer: rsmith.
mattd edited the summary of this revision.

This change aims to simplify debugging by annotating the range-for loop 
artificial variables (range, begin, end) with the scope depth.


https://reviews.llvm.org/D42813

Files:
  SemaStmt.cpp


Index: SemaStmt.cpp
===
--- SemaStmt.cpp
+++ SemaStmt.cpp
@@ -2025,7 +2025,7 @@
 
 /// Build a variable declaration for a for-range statement.
 VarDecl *BuildForRangeVarDecl(Sema , SourceLocation Loc,
-  QualType Type, const char *Name) {
+  QualType Type, StringRef Name) {
   DeclContext *DC = SemaRef.CurContext;
   IdentifierInfo *II = ().get(Name);
   TypeSourceInfo *TInfo = SemaRef.Context.getTrivialTypeSourceInfo(Type, Loc);
@@ -2093,11 +2093,13 @@
   return StmtError();
   }
 
-  // Build  auto && __range = range-init
+  // Build  auto && __range = range-init.
+  // Assume the variables are nested in the inner scope (loop body).
+  const auto DepthStr = std::to_string(S->getDepth() >> 1);
   SourceLocation RangeLoc = Range->getLocStart();
   VarDecl *RangeVar = BuildForRangeVarDecl(*this, RangeLoc,
Context.getAutoRRefDeductType(),
-   "__range");
+   std::string("__range") + DepthStr);
   if (FinishForRangeVarDecl(*this, RangeVar, Range, RangeLoc,
 diag::err_for_range_deduction_failure)) {
 LoopVar->setInvalidDecl();
@@ -2340,10 +2342,12 @@
   return StmtError();
 
 // Build auto __begin = begin-expr, __end = end-expr.
+// Assume the variables are nested in the inner scope (loop body).
+const auto DepthStr = std::to_string(S->getDepth() >> 1);
 VarDecl *BeginVar = BuildForRangeVarDecl(*this, ColonLoc, AutoType,
- "__begin");
+ std::string("__begin") + 
DepthStr);
 VarDecl *EndVar = BuildForRangeVarDecl(*this, ColonLoc, AutoType,
-   "__end");
+   std::string("__end") + DepthStr);
 
 // Build begin-expr and end-expr and attach to __begin and __end variables.
 ExprResult BeginExpr, EndExpr;


Index: SemaStmt.cpp
===
--- SemaStmt.cpp
+++ SemaStmt.cpp
@@ -2025,7 +2025,7 @@
 
 /// Build a variable declaration for a for-range statement.
 VarDecl *BuildForRangeVarDecl(Sema , SourceLocation Loc,
-  QualType Type, const char *Name) {
+  QualType Type, StringRef Name) {
   DeclContext *DC = SemaRef.CurContext;
   IdentifierInfo *II = ().get(Name);
   TypeSourceInfo *TInfo = SemaRef.Context.getTrivialTypeSourceInfo(Type, Loc);
@@ -2093,11 +2093,13 @@
   return StmtError();
   }
 
-  // Build  auto && __range = range-init
+  // Build  auto && __range = range-init.
+  // Assume the variables are nested in the inner scope (loop body).
+  const auto DepthStr = std::to_string(S->getDepth() >> 1);
   SourceLocation RangeLoc = Range->getLocStart();
   VarDecl *RangeVar = BuildForRangeVarDecl(*this, RangeLoc,
Context.getAutoRRefDeductType(),
-   "__range");
+   std::string("__range") + DepthStr);
   if (FinishForRangeVarDecl(*this, RangeVar, Range, RangeLoc,
 diag::err_for_range_deduction_failure)) {
 LoopVar->setInvalidDecl();
@@ -2340,10 +2342,12 @@
   return StmtError();
 
 // Build auto __begin = begin-expr, __end = end-expr.
+// Assume the variables are nested in the inner scope (loop body).
+const auto DepthStr = std::to_string(S->getDepth() >> 1);
 VarDecl *BeginVar = BuildForRangeVarDecl(*this, ColonLoc, AutoType,
- "__begin");
+ std::string("__begin") + DepthStr);
 VarDecl *EndVar = BuildForRangeVarDecl(*this, ColonLoc, AutoType,
-   "__end");
+   std::string("__end") + DepthStr);
 
 // Build begin-expr and end-expr and attach to __begin and __end variables.
 ExprResult BeginExpr, EndExpr;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42549: [CodeGen] Use the zero initializer instead of storing an all zero representation.

[Matt Davis via Phabricator via cfe-commits]

mattd added a comment.

Ping :)


https://reviews.llvm.org/D42549



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

[PATCH] D42248: Always allow "#pragma region".

[Matt Davis via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC323577: Always allow #pragma region. (authored 
by mattd, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D42248

Files:
  lib/Lex/Pragma.cpp
  test/Frontend/region-pragmas.c


Index: test/Frontend/region-pragmas.c
===
--- test/Frontend/region-pragmas.c
+++ test/Frontend/region-pragmas.c
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -Wall -verify %s
+// expected-no-diagnostics
+
+#pragma region foo
+#pragma endregion foo
Index: lib/Lex/Pragma.cpp
===
--- lib/Lex/Pragma.cpp
+++ lib/Lex/Pragma.cpp
@@ -1776,13 +1776,15 @@
   ModuleHandler->AddPragma(new PragmaModuleEndHandler());
   ModuleHandler->AddPragma(new PragmaModuleBuildHandler());
   ModuleHandler->AddPragma(new PragmaModuleLoadHandler());
+
+  // Add region pragmas.
+  AddPragmaHandler(new PragmaRegionHandler("region"));
+  AddPragmaHandler(new PragmaRegionHandler("endregion"));
 
   // MS extensions.
   if (LangOpts.MicrosoftExt) {
 AddPragmaHandler(new PragmaWarningHandler());
 AddPragmaHandler(new PragmaIncludeAliasHandler());
-AddPragmaHandler(new PragmaRegionHandler("region"));
-AddPragmaHandler(new PragmaRegionHandler("endregion"));
   }
 
   // Pragmas added by plugins


Index: test/Frontend/region-pragmas.c
===
--- test/Frontend/region-pragmas.c
+++ test/Frontend/region-pragmas.c
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -Wall -verify %s
+// expected-no-diagnostics
+
+#pragma region foo
+#pragma endregion foo
Index: lib/Lex/Pragma.cpp
===
--- lib/Lex/Pragma.cpp
+++ lib/Lex/Pragma.cpp
@@ -1776,13 +1776,15 @@
   ModuleHandler->AddPragma(new PragmaModuleEndHandler());
   ModuleHandler->AddPragma(new PragmaModuleBuildHandler());
   ModuleHandler->AddPragma(new PragmaModuleLoadHandler());
+
+  // Add region pragmas.
+  AddPragmaHandler(new PragmaRegionHandler("region"));
+  AddPragmaHandler(new PragmaRegionHandler("endregion"));
 
   // MS extensions.
   if (LangOpts.MicrosoftExt) {
 AddPragmaHandler(new PragmaWarningHandler());
 AddPragmaHandler(new PragmaIncludeAliasHandler());
-AddPragmaHandler(new PragmaRegionHandler("region"));
-AddPragmaHandler(new PragmaRegionHandler("endregion"));
   }
 
   // Pragmas added by plugins
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42549: [CodeGen] Use the zero initializer instead of storing an all zero representation.

[Matt Davis via Phabricator via cfe-commits]

mattd created this revision.
mattd added a reviewer: majnemer.

This change avoids the overhead of storing, and later crawling,
an initializer list of all zeros for arrays. When LLVM
visits this (llvm/IR/Constants.cpp) ConstantArray::getImpl()
it will scan the list looking for an array of all zero.

We can avoid the store, and short-cut the scan, by detecting
all zeros when clang builds-up the initialization representation.

This was brought to my attention when investigating PR36030


https://reviews.llvm.org/D42549

Files:
  lib/CodeGen/CGExprConstant.cpp
  test/CodeGen/array-init.c

Index: test/CodeGen/array-init.c
===
--- test/CodeGen/array-init.c
+++ test/CodeGen/array-init.c
@@ -0,0 +1,65 @@
+// RUN: %clang_cc1 %s -O0 -triple x86_64-unknown-linux-gnu -emit-llvm -o - | FileCheck %s
+
+// CHECK: @test.a1 = internal global [10 x i32] [i32 0, i32 1, i32 2, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0], align 16
+// CHECK: @test.a2 = internal global [10 x i32] zeroinitializer, align 16
+// CHECK: @test.a3 = internal global [10 x i32] zeroinitializer, align 16
+// CHECK: @test.a4 = internal global [10 x i32] zeroinitializer, align 16
+// CHECK: @test.b1 = internal constant [10 x i32] [i32 0, i32 1, i32 2, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0], align 16
+// CHECK: @test.b2 = internal constant [10 x i32] zeroinitializer, align 16
+// CHECK: @test.b3 = internal constant [10 x i32] zeroinitializer, align 16
+
+// CHECK: define void @test() #0 {
+// CHECK:   %c1 = alloca [10 x i32], align 16
+// CHECK:   %c2 = alloca [10 x i32], align 16
+// CHECK:   %c3 = alloca [10 x i32], align 16
+// CHECK:   %c4 = alloca [10 x i32], align 16
+// CHECK:   %d1 = alloca [10 x i32], align 16
+// CHECK:   %d2 = alloca [10 x i32], align 16
+// CHECK:   %d3 = alloca [10 x i32], align 16
+// CHECK:   %d4 = alloca [10 x i32], align 16
+// CHECK:   %1 = bitcast [10 x i32]* %c1 to i8*
+// CHECK:   call void @llvm.memset.p0i8.i64(i8* align 16 %1, i8 0, i64 40, i1 false)
+// CHECK:   %2 = bitcast i8* %1 to [10 x i32]*
+// CHECK:   %3 = getelementptr [10 x i32], [10 x i32]* %2, i32 0, i32 1
+// CHECK:   store i32 1, i32* %3
+// CHECK:   %4 = getelementptr [10 x i32], [10 x i32]* %2, i32 0, i32 2
+// CHECK:   store i32 2, i32* %4
+// CHECK:   %5 = bitcast [10 x i32]* %c2 to i8*
+// CHECK:   call void @llvm.memset.p0i8.i64(i8* align 16 %5, i8 0, i64 40, i1 false)
+// CHECK:   %6 = bitcast [10 x i32]* %c3 to i8*
+// CHECK:   call void @llvm.memset.p0i8.i64(i8* align 16 %6, i8 0, i64 40, i1 false)
+// CHECK:   %7 = bitcast [10 x i32]* %d1 to i8*
+// CHECK:   call void @llvm.memset.p0i8.i64(i8* align 16 %7, i8 0, i64 40, i1 true)
+// CHECK:   %8 = bitcast i8* %7 to [10 x i32]*
+// CHECK:   %9 = getelementptr [10 x i32], [10 x i32]* %8, i32 0, i32 1
+// CHECK:   store volatile i32 1, i32* %9
+// CHECK:   %10 = getelementptr [10 x i32], [10 x i32]* %8, i32 0, i32 2
+// CHECK:   store volatile i32 2, i32* %10
+// CHECK:   %11 = bitcast [10 x i32]* %d2 to i8*
+// CHECK:   call void @llvm.memset.p0i8.i64(i8* align 16 %11, i8 0, i64 40, i1 true)
+// CHECK:   %12 = bitcast [10 x i32]* %d3 to i8*
+// CHECK:   call void @llvm.memset.p0i8.i64(i8* align 16 %12, i8 0, i64 40, i1 true)
+// CHECK:   ret void
+// CHECK: }
+
+void test()
+{
+  static int a1[10] = {0,1,2};
+  static int a2[10] = {0,0,0};
+  static int a3[10] = {0};
+  static int a4[10];
+
+  const int b1[10] = {0,1,2};
+  const int b2[10] = {0,0,0};
+  const int b3[10] = {0};
+  
+  int c1[10] = {0,1,2};
+  int c2[10] = {0,0,0};
+  int c3[10] = {0};
+  int c4[10];
+  
+  volatile int d1[10] = {0,1,2};
+  volatile int d2[10] = {0,0,0};
+  volatile int d3[10] = {0};
+  volatile int d4[10];
+}
Index: lib/CodeGen/CGExprConstant.cpp
===
--- lib/CodeGen/CGExprConstant.cpp
+++ lib/CodeGen/CGExprConstant.cpp
@@ -859,9 +859,10 @@
 
 // Copy initializer elements.
 SmallVector Elts;
-Elts.reserve(NumInitableElts + NumElements);
+Elts.reserve(std::max(NumInitableElts, NumElements));
 
 bool RewriteType = false;
+bool AllNullValues = true;
 for (unsigned i = 0; i < NumInitableElts; ++i) {
   Expr *Init = ILE->getInit(i);
   llvm::Constant *C = Emitter.tryEmitPrivateForMemory(Init, EltType);
@@ -869,8 +870,15 @@
 return nullptr;
   RewriteType |= (C->getType() != ElemTy);
   Elts.push_back(C);
+  if (!C->isNullValue())
+AllNullValues = false;
 }
 
+// If all initializer elements are "zero," then avoid storing NumElements
+// instances of the zero representation.
+if (AllNullValues)
+  return llvm::ConstantAggregateZero::get(AType);
+
 RewriteType |= (fillC->getType() != ElemTy);
 Elts.resize(NumElements, fillC);
 
@@ -877,7 +885,7 @@
 if (RewriteType) {
   // FIXME: Try to avoid packing the array
   std::vector Types;
-  

[PATCH] D42248: Always allow "#pragma region".

[Matt Davis via Phabricator via cfe-commits]

mattd added a comment.

Thanks @majnemer!  Would you mind committing this on my behalf? I do not have 
commit access, thanks.


https://reviews.llvm.org/D42248



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

[PATCH] D42248: Always allow "#pragma region".

[Matt Davis via Phabricator via cfe-commits]

mattd added a comment.

Ping.


https://reviews.llvm.org/D42248



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

[PATCH] D42248: Always allow "#pragma region".

[Matt Davis via Phabricator via cfe-commits]

mattd updated this revision to Diff 130523.
mattd retitled this revision from "[LangOpts] Add a LangOpt to represent 
"#pragma region" support." to "Always allow "#pragma region".".
mattd edited the summary of this revision.
mattd added a comment.

I'm certainly fine with always allowing this pragma.   I can see it useful for 
other editors that might wish to utilize the hint.


https://reviews.llvm.org/D42248

Files:
  lib/Lex/Pragma.cpp
  test/Frontend/region-pragmas.c


Index: test/Frontend/region-pragmas.c
===
--- test/Frontend/region-pragmas.c
+++ test/Frontend/region-pragmas.c
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -Wall -verify %s
+// expected-no-diagnostics
+
+#pragma region foo
+#pragma endregion foo
Index: lib/Lex/Pragma.cpp
===
--- lib/Lex/Pragma.cpp
+++ lib/Lex/Pragma.cpp
@@ -1776,13 +1776,15 @@
   ModuleHandler->AddPragma(new PragmaModuleEndHandler());
   ModuleHandler->AddPragma(new PragmaModuleBuildHandler());
   ModuleHandler->AddPragma(new PragmaModuleLoadHandler());
+
+  // Add region pragmas.
+  AddPragmaHandler(new PragmaRegionHandler("region"));
+  AddPragmaHandler(new PragmaRegionHandler("endregion"));
 
   // MS extensions.
   if (LangOpts.MicrosoftExt) {
 AddPragmaHandler(new PragmaWarningHandler());
 AddPragmaHandler(new PragmaIncludeAliasHandler());
-AddPragmaHandler(new PragmaRegionHandler("region"));
-AddPragmaHandler(new PragmaRegionHandler("endregion"));
   }
 
   // Pragmas added by plugins


Index: test/Frontend/region-pragmas.c
===
--- test/Frontend/region-pragmas.c
+++ test/Frontend/region-pragmas.c
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -Wall -verify %s
+// expected-no-diagnostics
+
+#pragma region foo
+#pragma endregion foo
Index: lib/Lex/Pragma.cpp
===
--- lib/Lex/Pragma.cpp
+++ lib/Lex/Pragma.cpp
@@ -1776,13 +1776,15 @@
   ModuleHandler->AddPragma(new PragmaModuleEndHandler());
   ModuleHandler->AddPragma(new PragmaModuleBuildHandler());
   ModuleHandler->AddPragma(new PragmaModuleLoadHandler());
+
+  // Add region pragmas.
+  AddPragmaHandler(new PragmaRegionHandler("region"));
+  AddPragmaHandler(new PragmaRegionHandler("endregion"));
 
   // MS extensions.
   if (LangOpts.MicrosoftExt) {
 AddPragmaHandler(new PragmaWarningHandler());
 AddPragmaHandler(new PragmaIncludeAliasHandler());
-AddPragmaHandler(new PragmaRegionHandler("region"));
-AddPragmaHandler(new PragmaRegionHandler("endregion"));
   }
 
   // Pragmas added by plugins
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42248: [LangOpts] Add a LangOpt to represent "#pragma region" support.

[Matt Davis via Phabricator via cfe-commits]

mattd added a comment.

In https://reviews.llvm.org/D42248#980541, @majnemer wrote:

> Why not always support the pragma regardless of the compiler mode? Our 
> "support" for it just ignores it anyway...


Thanks for the reply @majnemer.

I am not opposed to that idea.  My change just operates similar to the existing 
behavior.  The only reservation I had against not always accepting the pragma 
is that it might mislead devs who are not using PS4/VS based environments.


https://reviews.llvm.org/D42248



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

[PATCH] D42248: [LangOpts] Add a LangOpt to represent "#pragma region" support.

[Matt Davis via Phabricator via cfe-commits]

mattd created this revision.
mattd added reviewers: rnk, rsmith.

Both MS and PS4 targets are capable of recognizing the
existence of:  #pragma region, #pragma endregion.

This patch adds a LangOpt and sets the value based on target 
information or MS compatibility. In the case of PS4 or MS we
should avoid emitting "unknown pragma" warnings for regions.
This change prevents that situation.


https://reviews.llvm.org/D42248

Files:
  include/clang/Basic/LangOptions.def
  lib/Frontend/CompilerInvocation.cpp
  lib/Lex/Pragma.cpp
  test/Frontend/region-pragmas.c


Index: test/Frontend/region-pragmas.c
===
--- test/Frontend/region-pragmas.c
+++ test/Frontend/region-pragmas.c
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -triple x86_64-scei-ps4 -Wall -verify %s
+// RUN: %clang_cc1 -fms-compatibility -Wall -verify %s
+// RUN: %clang_cc1 -Wall -Wno-unknown-pragmas -verify %s
+// expected-no-diagnostics
+
+#pragma region foo
+#pragma endregion foo
Index: lib/Lex/Pragma.cpp
===
--- lib/Lex/Pragma.cpp
+++ lib/Lex/Pragma.cpp
@@ -1781,6 +1781,10 @@
   if (LangOpts.MicrosoftExt) {
 AddPragmaHandler(new PragmaWarningHandler());
 AddPragmaHandler(new PragmaIncludeAliasHandler());
+  }
+
+  // Region support.
+  if (LangOpts.MicrosoftPragmaRegion) {
 AddPragmaHandler(new PragmaRegionHandler("region"));
 AddPragmaHandler(new PragmaRegionHandler("endregion"));
   }
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -2160,6 +2160,7 @@
 
   Opts.MSVCCompat = Args.hasArg(OPT_fms_compatibility);
   Opts.MicrosoftExt = Opts.MSVCCompat || Args.hasArg(OPT_fms_extensions);
+  Opts.MicrosoftPragmaRegion = Opts.MicrosoftExt || T.isPS4();
   Opts.AsmBlocks = Args.hasArg(OPT_fasm_blocks) || Opts.MicrosoftExt;
   Opts.MSCompatibilityVersion = 0;
   if (const Arg *A = Args.getLastArg(OPT_fms_compatibility_version)) {
Index: include/clang/Basic/LangOptions.def
===
--- include/clang/Basic/LangOptions.def
+++ include/clang/Basic/LangOptions.def
@@ -85,6 +85,7 @@
 LANGOPT(C17   , 1, 0, "C17")
 LANGOPT(MSVCCompat, 1, 0, "Microsoft Visual C++ full compatibility 
mode")
 LANGOPT(MicrosoftExt  , 1, 0, "Microsoft C++ extensions")
+BENIGN_LANGOPT(MicrosoftPragmaRegion, 1, 0, "region pragma support")
 LANGOPT(AsmBlocks , 1, 0, "Microsoft inline asm blocks")
 LANGOPT(Borland   , 1, 0, "Borland extensions")
 LANGOPT(CPlusPlus , 1, 0, "C++")


Index: test/Frontend/region-pragmas.c
===
--- test/Frontend/region-pragmas.c
+++ test/Frontend/region-pragmas.c
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -triple x86_64-scei-ps4 -Wall -verify %s
+// RUN: %clang_cc1 -fms-compatibility -Wall -verify %s
+// RUN: %clang_cc1 -Wall -Wno-unknown-pragmas -verify %s
+// expected-no-diagnostics
+
+#pragma region foo
+#pragma endregion foo
Index: lib/Lex/Pragma.cpp
===
--- lib/Lex/Pragma.cpp
+++ lib/Lex/Pragma.cpp
@@ -1781,6 +1781,10 @@
   if (LangOpts.MicrosoftExt) {
 AddPragmaHandler(new PragmaWarningHandler());
 AddPragmaHandler(new PragmaIncludeAliasHandler());
+  }
+
+  // Region support.
+  if (LangOpts.MicrosoftPragmaRegion) {
 AddPragmaHandler(new PragmaRegionHandler("region"));
 AddPragmaHandler(new PragmaRegionHandler("endregion"));
   }
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -2160,6 +2160,7 @@
 
   Opts.MSVCCompat = Args.hasArg(OPT_fms_compatibility);
   Opts.MicrosoftExt = Opts.MSVCCompat || Args.hasArg(OPT_fms_extensions);
+  Opts.MicrosoftPragmaRegion = Opts.MicrosoftExt || T.isPS4();
   Opts.AsmBlocks = Args.hasArg(OPT_fasm_blocks) || Opts.MicrosoftExt;
   Opts.MSCompatibilityVersion = 0;
   if (const Arg *A = Args.getLastArg(OPT_fms_compatibility_version)) {
Index: include/clang/Basic/LangOptions.def
===
--- include/clang/Basic/LangOptions.def
+++ include/clang/Basic/LangOptions.def
@@ -85,6 +85,7 @@
 LANGOPT(C17   , 1, 0, "C17")
 LANGOPT(MSVCCompat, 1, 0, "Microsoft Visual C++ full compatibility mode")
 LANGOPT(MicrosoftExt  , 1, 0, "Microsoft C++ extensions")
+BENIGN_LANGOPT(MicrosoftPragmaRegion, 1, 0, "region pragma support")
 LANGOPT(AsmBlocks , 1, 0, "Microsoft inline asm blocks")
 LANGOPT(Borland   , 1, 0, "Borland extensions")
 LANGOPT(CPlusPlus , 1, 0, "C++")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D40398: Remove ValueDependent assertions on ICE checks.

[Matt Davis via Phabricator via cfe-commits]

mattd abandoned this revision.
mattd added a comment.

Abandoning as this is no longer a problem with release builds.


https://reviews.llvm.org/D40398



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

[PATCH] D41421: Eliminate a magic number. NFC.

[Matt Davis via Phabricator via cfe-commits]

mattd added a comment.

Ping :) I would like someone to commit this on my behalf, thanks!


https://reviews.llvm.org/D41421



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

[PATCH] D41421: Eliminate a magic number. NFC.

[Matt Davis via Phabricator via cfe-commits]

mattd added a comment.

Thanks Eli!  I do not have commit permissions, so someone else will have to 
commit this on my behalf.


https://reviews.llvm.org/D41421



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

[PATCH] D41421: Eliminate a magic number. NFC.

[Matt Davis via Phabricator via cfe-commits]

mattd updated this revision to Diff 127641.
mattd added a comment.

Thanks for the suggestion Eli.  Took your initial suggestion.


https://reviews.llvm.org/D41421

Files:
  lib/Frontend/PrintPreprocessedOutput.cpp


Index: lib/Frontend/PrintPreprocessedOutput.cpp
===
--- lib/Frontend/PrintPreprocessedOutput.cpp
+++ lib/Frontend/PrintPreprocessedOutput.cpp
@@ -752,7 +752,7 @@
 } else if (Tok.isLiteral() && !Tok.needsCleaning() &&
Tok.getLiteralData()) {
   OS.write(Tok.getLiteralData(), Tok.getLength());
-} else if (Tok.getLength() < 256) {
+} else if (Tok.getLength() < llvm::array_lengthof(Buffer)) {
   const char *TokPtr = Buffer;
   unsigned Len = PP.getSpelling(Tok, TokPtr);
   OS.write(TokPtr, Len);


Index: lib/Frontend/PrintPreprocessedOutput.cpp
===
--- lib/Frontend/PrintPreprocessedOutput.cpp
+++ lib/Frontend/PrintPreprocessedOutput.cpp
@@ -752,7 +752,7 @@
 } else if (Tok.isLiteral() && !Tok.needsCleaning() &&
Tok.getLiteralData()) {
   OS.write(Tok.getLiteralData(), Tok.getLength());
-} else if (Tok.getLength() < 256) {
+} else if (Tok.getLength() < llvm::array_lengthof(Buffer)) {
   const char *TokPtr = Buffer;
   unsigned Len = PP.getSpelling(Tok, TokPtr);
   OS.write(TokPtr, Len);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41421: Eliminate a magic number. NFC.

[Matt Davis via Phabricator via cfe-commits]

mattd created this revision.
mattd added a reviewer: bkramer.

Calculate sizeof Buffer instead of using a magic value.


https://reviews.llvm.org/D41421

Files:
  PrintPreprocessedOutput.cpp


Index: PrintPreprocessedOutput.cpp
===
--- PrintPreprocessedOutput.cpp
+++ PrintPreprocessedOutput.cpp
@@ -752,7 +752,7 @@
 } else if (Tok.isLiteral() && !Tok.needsCleaning() &&
Tok.getLiteralData()) {
   OS.write(Tok.getLiteralData(), Tok.getLength());
-} else if (Tok.getLength() < 256) {
+} else if (Tok.getLength() < sizeof(Buffer)) {
   const char *TokPtr = Buffer;
   unsigned Len = PP.getSpelling(Tok, TokPtr);
   OS.write(TokPtr, Len);


Index: PrintPreprocessedOutput.cpp
===
--- PrintPreprocessedOutput.cpp
+++ PrintPreprocessedOutput.cpp
@@ -752,7 +752,7 @@
 } else if (Tok.isLiteral() && !Tok.needsCleaning() &&
Tok.getLiteralData()) {
   OS.write(Tok.getLiteralData(), Tok.getLength());
-} else if (Tok.getLength() < 256) {
+} else if (Tok.getLength() < sizeof(Buffer)) {
   const char *TokPtr = Buffer;
   unsigned Len = PP.getSpelling(Tok, TokPtr);
   OS.write(TokPtr, Len);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40398: Remove ValueDependent assertions on ICE checks.

[Matt Davis via Phabricator via cfe-commits]

mattd added a comment.

ping


https://reviews.llvm.org/D40398



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

[PATCH] D40566: Check if MemberExpr arguments are type-dependent.

[Matt Davis via Phabricator via cfe-commits]

mattd added a comment.

Thanks for the comments Eli!  I agree, it makes sense to flag certain 
expressions to be type dependent at the earliest time of compilation.  Clang 
already does this, as the Value/Type dependence is established upon Expr 
construction from MemberExpr construction.

As we have discussed,  in the case of MemberExpr, Clang bases dependence on the 
'this' pointer, which can be seen in the MemberExpr constructor. And from the 
attached test case, this is decision is not correct.  From what we noted, 
member access expressions can base their dependence on the type of the member.

However, after trying to propagate this change sooner rather than later (in the 
MemberExpr ctor), I realized that I might have been changing language 
semantics.  One thing, in particular, is how we handle address-of expressions.  
 Clang predicates them, internally, on type-dependence.  From that perspective, 
we will never try to bind a address-of expression within a template definition, 
if the address-of is for a member variable.  The bind will only occur during 
template instantiation.  Another case we have to be aware of are MemberExpr 
where the member is a CXXMethodExpr, which would be dependent given the 
implicit 'this' parameter; that's correct, but I don't want to break that 
either.


https://reviews.llvm.org/D40566



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

[PATCH] D40574: Bounds check argument_with_type_tag attribute.

[Matt Davis via Phabricator via cfe-commits]

mattd marked an inline comment as done.
mattd added a comment.

Thanks for the approval Aaron.  I do not have commit access, would you mind 
submitting this on my behalf?


https://reviews.llvm.org/D40574



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

[PATCH] D40574: Bounds check argument_with_type_tag attribute.

[Matt Davis via Phabricator via cfe-commits]

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



Comment at: test/Sema/error-type-safety.cpp:3-4
+
+static const int test_void
+  __attribute__((type_tag_for_datatype(test, void))) = 0;
+

aaron.ballman wrote:
> Is this declaration necessary?
Hi Aaron, thanks for the input.  Yes, defining the tag is necessary in order to 
test the argument index.  If the type tag is not satisfied, the code path 
checking argument tag index will never be reached.  My update will test the 
bounds and is a little more comprehensive.


https://reviews.llvm.org/D40574



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

[PATCH] D40574: Bounds check argument_with_type_tag attribute.

[Matt Davis via Phabricator via cfe-commits]

mattd updated this revision to Diff 124783.
mattd added a comment.

- Removed the new lines.
- Added white space between the data type and pointer character.
- Updated the test to check the exact bounds, and over-bounds cases
- Combined the diagnostic messages into one via 'select'




https://reviews.llvm.org/D40574

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  lib/Sema/SemaChecking.cpp
  test/Sema/error-type-safety.cpp

Index: test/Sema/error-type-safety.cpp
===
--- test/Sema/error-type-safety.cpp
+++ test/Sema/error-type-safety.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+#define INT_TAG 42
+
+static const int test_in
+  __attribute__((type_tag_for_datatype(test, int))) = INT_TAG;
+
+// Argument index: 1, Type tag index: 2
+void test_bounds_index(...)
+  __attribute__((argument_with_type_tag(test, 1, 2)));
+
+// Argument index: 3, Type tag index: 1
+void test_bounds_arg_index(...)
+  __attribute__((argument_with_type_tag(test, 3, 1)));
+
+void test_bounds()
+{
+  // Test the boundary edges (ensure no off-by-one) with argument indexing.
+  test_bounds_index(1, INT_TAG);
+
+  test_bounds_index(1); // expected-error {{type tag index 2 is greater than the number of arguments specified}}
+  test_bounds_arg_index(INT_TAG, 1); // expected-error {{argument index 3 is greater than the number of arguments specified}}
+}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -2754,7 +2754,7 @@
 // Type safety checking.
 if (FDecl) {
   for (const auto *I : FDecl->specific_attrs())
-CheckArgumentWithTypeTag(I, Args.data());
+CheckArgumentWithTypeTag(I, Args, Loc);
 }
   }
 
@@ -12329,10 +12329,18 @@
 }
 
 void Sema::CheckArgumentWithTypeTag(const ArgumentWithTypeTagAttr *Attr,
-const Expr * const *ExprArgs) {
+const ArrayRef ExprArgs,
+SourceLocation CallSiteLoc) {
   const IdentifierInfo *ArgumentKind = Attr->getArgumentKind();
   bool IsPointerAttr = Attr->getIsPointer();
 
+  // Retrieve the argument representing the 'type_tag'.
+  if (Attr->getTypeTagIdx() >= ExprArgs.size()) {
+// Add 1 to display the user's specified value.
+Diag(CallSiteLoc, diag::err_tag_index_out_of_range)
+<< 0 << Attr->getTypeTagIdx() + 1;
+return;
+  }
   const Expr *TypeTagExpr = ExprArgs[Attr->getTypeTagIdx()];
   bool FoundWrongKind;
   TypeTagData TypeInfo;
@@ -12346,6 +12354,13 @@
 return;
   }
 
+  // Retrieve the argument representing the 'arg_idx'.
+  if (Attr->getArgumentIdx() >= ExprArgs.size()) {
+// Add 1 to display the user's specified value.
+Diag(CallSiteLoc, diag::err_tag_index_out_of_range)
+<< 1 << Attr->getArgumentIdx() + 1;
+return;
+  }
   const Expr *ArgumentExpr = ExprArgs[Attr->getArgumentIdx()];
   if (IsPointerAttr) {
 // Skip implicit cast of pointer to `void *' (as a function argument).
Index: include/clang/Sema/Sema.h
===
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -10455,7 +10455,8 @@
   /// \brief Peform checks on a call of a function with argument_with_type_tag
   /// or pointer_with_type_tag attributes.
   void CheckArgumentWithTypeTag(const ArgumentWithTypeTagAttr *Attr,
-const Expr * const *ExprArgs);
+const ArrayRef ExprArgs,
+SourceLocation CallSiteLoc);
 
   /// \brief Check if we are taking the address of a packed field
   /// as this may be a problem if the pointer value is dereferenced.
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -7919,6 +7919,8 @@
   "'type_tag_for_datatype' attribute requires the initializer to be "
   "an %select{integer|integral}0 constant expression "
   "that can be represented by a 64 bit integer">;
+def err_tag_index_out_of_range : Error<
+  "%select{type tag|argument}0 index %1 is greater than the number of arguments specified">;
 def warn_type_tag_for_datatype_wrong_kind : Warning<
   "this type tag was not designed to be used with this function">,
   InGroup;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40574: Bounds check argument_with_type_tag attribute.

[Matt Davis via Phabricator via cfe-commits]

mattd created this revision.

Fixes: PR28520

Perform a bounds check on a function's argument list, before accessing any 
index value specified by an 'argument_with_type_tag' attribute.


https://reviews.llvm.org/D40574

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  lib/Sema/SemaChecking.cpp
  test/Sema/error-type-safety.cpp

Index: test/Sema/error-type-safety.cpp
===
--- test/Sema/error-type-safety.cpp
+++ test/Sema/error-type-safety.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+static const int test_void
+  __attribute__((type_tag_for_datatype(test, void))) = 0;
+
+void test_invalid_type_tag_index(int datatype, ...)
+  __attribute__((argument_with_type_tag(test,1,99)));
+
+void test_invalid_arg_index(int datatype, ...)
+  __attribute__((argument_with_type_tag(test,99,1)));
+
+void test_bounds()
+{
+  test_invalid_type_tag_index(0); // expected-error {{type tag index 99 is greater than the number of arguments specified}}
+  test_invalid_arg_index(0); // expected-error {{argument tag index 99 is greater than the number of arguments specified}}
+}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -2754,7 +2754,7 @@
 // Type safety checking.
 if (FDecl) {
   for (const auto *I : FDecl->specific_attrs())
-CheckArgumentWithTypeTag(I, Args.data());
+CheckArgumentWithTypeTag(I, Args, Loc);
 }
   }
 
@@ -12329,11 +12329,20 @@
 }
 
 void Sema::CheckArgumentWithTypeTag(const ArgumentWithTypeTagAttr *Attr,
-const Expr * const *ExprArgs) {
+const ArrayRef ExprArgs,
+SourceLocation CallSiteLoc) {
   const IdentifierInfo *ArgumentKind = Attr->getArgumentKind();
   bool IsPointerAttr = Attr->getIsPointer();
 
+  // Retrieve the argument representing the 'type_tag'.
+  if (Attr->getTypeTagIdx() >= ExprArgs.size()) {
+// Add 1 to display the user's specified value.
+Diag(CallSiteLoc, diag::err_type_tag_out_of_range)
+<< Attr->getTypeTagIdx() + 1;
+return;
+  }
   const Expr *TypeTagExpr = ExprArgs[Attr->getTypeTagIdx()];
+
   bool FoundWrongKind;
   TypeTagData TypeInfo;
   if (!GetMatchingCType(ArgumentKind, TypeTagExpr, Context,
@@ -12346,7 +12355,15 @@
 return;
   }
 
+  // Retrieve the argument representing the 'arg_idx'.
+  if (Attr->getArgumentIdx() >= ExprArgs.size()) {
+// Add 1 to display the user's specified value.
+Diag(CallSiteLoc, diag::err_arg_tag_out_of_range)
+<< Attr->getArgumentIdx() + 1;
+return;
+  }
   const Expr *ArgumentExpr = ExprArgs[Attr->getArgumentIdx()];
+
   if (IsPointerAttr) {
 // Skip implicit cast of pointer to `void *' (as a function argument).
 if (const ImplicitCastExpr *ICE = dyn_cast(ArgumentExpr))
Index: include/clang/Sema/Sema.h
===
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -10455,7 +10455,8 @@
   /// \brief Peform checks on a call of a function with argument_with_type_tag
   /// or pointer_with_type_tag attributes.
   void CheckArgumentWithTypeTag(const ArgumentWithTypeTagAttr *Attr,
-const Expr * const *ExprArgs);
+const ArrayRef ExprArgs,
+SourceLocation CallSiteLoc);
 
   /// \brief Check if we are taking the address of a packed field
   /// as this may be a problem if the pointer value is dereferenced.
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -7919,6 +7919,10 @@
   "'type_tag_for_datatype' attribute requires the initializer to be "
   "an %select{integer|integral}0 constant expression "
   "that can be represented by a 64 bit integer">;
+def err_type_tag_out_of_range : Error<
+  "type tag index %0 is greater than the number of arguments specified">;
+def err_arg_tag_out_of_range : Error<
+  "argument tag index %0 is greater than the number of arguments specified">;
 def warn_type_tag_for_datatype_wrong_kind : Warning<
   "this type tag was not designed to be used with this function">,
   InGroup;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40566: Check if MemberExpr arguments are type-dependent.

[Matt Davis via Phabricator via cfe-commits]

mattd created this revision.

Fixes: PR28290

When checking an argument list, arguments from the templated class instance, 
were
returning 'is dependent' based on the 'this' pointer. In that case, arguments 
were
being marked dependent, and name lookup was delayed until template 
instantiation. This
had the side-effect of -fdelayed-template-parsing in certain cases
(attached test case), when that should not have been the case, especially
since that flag was never passed.

According to the standard the referenced member's type needs
to be checked:

[temp.dep.expr]p5:
A class member access expression (5.2.5) is type-dependent if the expression
refers to a member of the current instantiation and the type of the referenced
member is dependent, or the class member access expression refers to a member
of an unknown specialization.

This change decides if the argument belongs to a MemberExpr. If so, then the
type of the expression is checked if it is dependent or not.


https://reviews.llvm.org/D40566

Files:
  lib/AST/Expr.cpp
  test/CXX/class.access/class.access.dcl/p1.cpp
  test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p4.cpp
  test/SemaTemplate/template-with-invalid-decl.cpp


Index: test/SemaTemplate/template-with-invalid-decl.cpp
===
--- test/SemaTemplate/template-with-invalid-decl.cpp
+++ test/SemaTemplate/template-with-invalid-decl.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -verify %s
+
+class UBS;
+
+template  struct FIBH 
+{
+  FIBH() { GDN(BS); } // expected-error {{use of undeclared identifier 'GDN'}}
+  class UBS* BS;
+};
+
+extern void foo()
+{
+  FIBH IBH;
+}
+
+extern void GDN(UBS* BS);
Index: test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p4.cpp
===
--- test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p4.cpp
+++ test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p4.cpp
@@ -29,7 +29,7 @@
   };
 }
 
-struct Opaque0 {};
+struct Opaque0 {}; // expected-note-re 1-2{{candidate constructor {{.*
 
 namespace test1 {
   struct A {
@@ -112,7 +112,7 @@
 }
 
 void test5() {
-  Opaque0 _ = hiding;
+  Opaque0 _ = hiding; // expected-error {{no viable conversion from 'int' 
to 'Opaque0'}}
 }
   };
 }
Index: test/CXX/class.access/class.access.dcl/p1.cpp
===
--- test/CXX/class.access/class.access.dcl/p1.cpp
+++ test/CXX/class.access/class.access.dcl/p1.cpp
@@ -56,7 +56,7 @@
   };
 }
 
-struct Opaque0 {};
+struct Opaque0 {}; // expected-note-re 1-2 {{candidate constructor {{.*
 
 namespace test1 {
   struct A {
@@ -196,7 +196,7 @@
 }
 
 void test5() {
-  Opaque0 _ = hiding;
+  Opaque0 _ = hiding; // expected-error {{no viable conversion from 'int' 
to 'Opaque0'}}
 }
   };
 }
Index: lib/AST/Expr.cpp
===
--- lib/AST/Expr.cpp
+++ lib/AST/Expr.cpp
@@ -2738,12 +2738,27 @@
   return false;
 }
 
+/// \brief Returns true if the expression is a MemberExpr
+/// with a dependent type.
+static bool isDependentMember(const Expr *E)
+{
+  if (const auto ME = dyn_cast_or_null(E))
+if (const auto Member = ME->getMemberDecl())
+  return Member->getType()->isDependentType();
+
+  return false;
+}
+
 /// hasAnyTypeDependentArguments - Determines if any of the expressions
 /// in Exprs is type-dependent.
 bool Expr::hasAnyTypeDependentArguments(ArrayRef Exprs) {
-  for (unsigned I = 0; I < Exprs.size(); ++I)
-if (Exprs[I]->isTypeDependent())
+  for (const auto E : Exprs) {
+if (isa(E)) {
+  if (isDependentMember(E))
+return true;
+} else if (E->isTypeDependent())
   return true;
+  }
 
   return false;
 }


Index: test/SemaTemplate/template-with-invalid-decl.cpp
===
--- test/SemaTemplate/template-with-invalid-decl.cpp
+++ test/SemaTemplate/template-with-invalid-decl.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -verify %s
+
+class UBS;
+
+template  struct FIBH 
+{
+  FIBH() { GDN(BS); } // expected-error {{use of undeclared identifier 'GDN'}}
+  class UBS* BS;
+};
+
+extern void foo()
+{
+  FIBH IBH;
+}
+
+extern void GDN(UBS* BS);
Index: test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p4.cpp
===
--- test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p4.cpp
+++ test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p4.cpp
@@ -29,7 +29,7 @@
   };
 }
 
-struct Opaque0 {};
+struct Opaque0 {}; // expected-note-re 1-2{{candidate constructor {{.*
 
 namespace test1 {
   struct A {
@@ -112,7 +112,7 @@
 }
 
 void test5() {
-  Opaque0 _ = hiding;
+  Opaque0 _ = hiding; // expected-error {{no viable conversion from 'int' to 'Opaque0'}}
 }
   };
 }
Index: test/CXX/class.access/class.access.dcl/p1.cpp
===
--- 

[PATCH] D40398: Remove ValueDependent assertions on ICE checks.

[Matt Davis via Phabricator via cfe-commits]

mattd created this revision.

In the case of ill-formed templates, a value dependent name can reach the 
integral constant expression (ICE) check. After an error occurs following an 
ill-formed template, an ICE check can be performed on a template definition. 
That definition might contain an expression, such as "const a = sizeof(T);" 
where T is a template type argument. In this case, the latter expression is 
ValueDependent, T is type dependent, and the sizeof expression is therefore 
considered Value Dependent.

The assertions will trigger, if the implementation of a templated member 
function occurs in a separate namespace from its definition (bad code). If such 
a case occurs, an ODR marking of a variable decl is performed, which happens to 
check that a particular variable is constant.

Additionally, ValueDependent items can be ICEs, but we never check that case, 
and the assertions assume we never see them in the first place.  In general, I 
don't like removing assertions, for obvious reasons.  But in this case, I am 
not sure they are necessary, and can be problematic as seen by the included 
test case.


https://reviews.llvm.org/D40398

Files:
  lib/AST/Decl.cpp
  lib/AST/ExprConstant.cpp
  test/SemaTemplate/illformed-template-ice.cpp


Index: test/SemaTemplate/illformed-template-ice.cpp
===
--- test/SemaTemplate/illformed-template-ice.cpp
+++ test/SemaTemplate/illformed-template-ice.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+class A {
+public:
+  template  void B();
+};
+
+namespace {
+  template  void A::B() { // expected-error {{cannot define or 
redeclare 'B' here because namespace '' does not enclose namespace 'A'}}
+const int a = sizeof(T);
+int  b = a;
+  }
+}
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -10250,7 +10250,6 @@
 }
 
 static ICEDiag CheckICE(const Expr* E, const ASTContext ) {
-  assert(!E->isValueDependent() && "Should not see value dependent exprs!");
   if (!E->getType()->isIntegralOrEnumerationType())
 return ICEDiag(IK_NotICE, E->getLocStart());
 
Index: lib/AST/Decl.cpp
===
--- lib/AST/Decl.cpp
+++ lib/AST/Decl.cpp
@@ -2216,7 +2216,6 @@
 return Eval->Evaluated.isUninit() ? nullptr : >Evaluated;
 
   const auto *Init = cast(Eval->Value);
-  assert(!Init->isValueDependent());
 
   if (Eval->IsEvaluating) {
 // FIXME: Produce a diagnostic for self-initialization.
@@ -2284,7 +2283,6 @@
 return Eval->IsICE;
 
   const auto *Init = cast(Eval->Value);
-  assert(!Init->isValueDependent());
 
   // In C++11, evaluate the initializer to check whether it's a constant
   // expression.


Index: test/SemaTemplate/illformed-template-ice.cpp
===
--- test/SemaTemplate/illformed-template-ice.cpp
+++ test/SemaTemplate/illformed-template-ice.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+class A {
+public:
+  template  void B();
+};
+
+namespace {
+  template  void A::B() { // expected-error {{cannot define or redeclare 'B' here because namespace '' does not enclose namespace 'A'}}
+const int a = sizeof(T);
+int  b = a;
+  }
+}
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -10250,7 +10250,6 @@
 }
 
 static ICEDiag CheckICE(const Expr* E, const ASTContext ) {
-  assert(!E->isValueDependent() && "Should not see value dependent exprs!");
   if (!E->getType()->isIntegralOrEnumerationType())
 return ICEDiag(IK_NotICE, E->getLocStart());
 
Index: lib/AST/Decl.cpp
===
--- lib/AST/Decl.cpp
+++ lib/AST/Decl.cpp
@@ -2216,7 +2216,6 @@
 return Eval->Evaluated.isUninit() ? nullptr : >Evaluated;
 
   const auto *Init = cast(Eval->Value);
-  assert(!Init->isValueDependent());
 
   if (Eval->IsEvaluating) {
 // FIXME: Produce a diagnostic for self-initialization.
@@ -2284,7 +2283,6 @@
 return Eval->IsICE;
 
   const auto *Init = cast(Eval->Value);
-  assert(!Init->isValueDependent());
 
   // In C++11, evaluate the initializer to check whether it's a constant
   // expression.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits