[libcxx] r337235 - Address "always inline function is not always inlinable" warning with GCC.

[Eric Fiselier via cfe-commits]

Author: ericwf
Date: Mon Jul 16 22:48:48 2018
New Revision: 337235

URL: http://llvm.org/viewvc/llvm-project?rev=337235=rev
Log:
Address "always inline function is not always inlinable" warning with GCC.

When an always_inline function is used prior to the functions definition,
the compiler may not be able to inline it as requested by the attribute.
GCC flags the `basic_string(CharT const*)` function as one such example.

This patch supresses the warning, and the problem, by moving the
definition of the string constructor to the inline declaration.
This ensures the body is available when it is first ODR used.

Modified:
libcxx/trunk/include/string

Modified: libcxx/trunk/include/string
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/string?rev=337235=337234=337235=diff
==
--- libcxx/trunk/include/string (original)
+++ libcxx/trunk/include/string Mon Jul 16 22:48:48 2018
@@ -807,8 +807,14 @@ public:
 #endif  // _LIBCPP_CXX03_LANG
 
 template ::value, 
nullptr_t>::type>
-_LIBCPP_INLINE_VISIBILITY
-basic_string(const _CharT* __s);
+_LIBCPP_INLINE_VISIBILITY
+basic_string(const _CharT* __s) {
+  _LIBCPP_ASSERT(__s != nullptr, "basic_string(const char*) detected 
nullptr");
+  __init(__s, traits_type::length(__s));
+#   if _LIBCPP_DEBUG_LEVEL >= 2
+  __get_db()->__insert_c(this);
+#   endif
+}
 
 template ::value, 
nullptr_t>::type>
 _LIBCPP_INLINE_VISIBILITY
@@ -1774,17 +1780,6 @@ basic_string<_CharT, _Traits, _Allocator
 }
 
 template 
-template 
-basic_string<_CharT, _Traits, _Allocator>::basic_string(const _CharT* __s)
-{
-_LIBCPP_ASSERT(__s != nullptr, "basic_string(const char*) detected 
nullptr");
-__init(__s, traits_type::length(__s));
-#if _LIBCPP_DEBUG_LEVEL >= 2
-__get_db()->__insert_c(this);
-#endif
-}
-
-template 
 template 
 basic_string<_CharT, _Traits, _Allocator>::basic_string(const _CharT* __s, 
const _Allocator& __a)
 : __r_(__second_tag(), __a)


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

[PATCH] D48266: [Driver] Add -fno-digraphs

[Jacob Bandes-Storch via Phabricator via cfe-commits]

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

Ping again 


Repository:
  rL LLVM

https://reviews.llvm.org/D48266



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

[PATCH] D48266: [Driver] Add -fno-digraphs

[Jacob Bandes-Storch via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL337232: [Driver] Add -fno-digraphs (authored by jtbandes, 
committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D48266?vs=155116=155809#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D48266

Files:
  cfe/trunk/include/clang/Driver/Options.td
  cfe/trunk/lib/Driver/ToolChains/Clang.cpp
  cfe/trunk/lib/Frontend/CompilerInvocation.cpp
  cfe/trunk/test/Lexer/digraph.c


Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -1335,6 +1335,10 @@
 def fno_diagnostics_show_option : Flag<["-"], "fno-diagnostics-show-option">, 
Group;
 def fno_diagnostics_show_note_include_stack : Flag<["-"], 
"fno-diagnostics-show-note-include-stack">,
 Flags<[CC1Option]>, Group;
+def fdigraphs : Flag<["-"], "fdigraphs">, Group, Flags<[CC1Option]>,
+  HelpText<"Enable alternative token representations '<:', ':>', '<%', '%>', 
'%:', '%:%:' (default)">;
+def fno_digraphs : Flag<["-"], "fno-digraphs">, Group, 
Flags<[CC1Option]>,
+  HelpText<"Disallow alternative token representations '<:', ':>', '<%', '%>', 
'%:', '%:%:'">;
 def fno_declspec : Flag<["-"], "fno-declspec">, Group,
   HelpText<"Disallow __declspec as a keyword">, Flags<[CC1Option]>;
 def fno_dollars_in_identifiers : Flag<["-"], "fno-dollars-in-identifiers">, 
Group,
Index: cfe/trunk/test/Lexer/digraph.c
===
--- cfe/trunk/test/Lexer/digraph.c
+++ cfe/trunk/test/Lexer/digraph.c
@@ -1,6 +1,13 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -ffreestanding %s
-// expected-no-diagnostics
+// RUN: %clang_cc1 -DDIGRAPHS=1 -fsyntax-only -verify -ffreestanding %s
+// RUN: %clang_cc1 -DDIGRAPHS=1 -fno-digraphs -fdigraphs -fsyntax-only -verify 
-ffreestanding %s
+// RUN: %clang_cc1 -fno-digraphs -fsyntax-only -verify -ffreestanding %s
+// RUN: %clang_cc1 -fdigraphs -fno-digraphs -fsyntax-only -verify 
-ffreestanding %s
+// RUN: %clang_cc1 -std=c89 -DDIGRAPHS=1 -fdigraphs -fsyntax-only -verify 
-ffreestanding %s
+// RUN: %clang_cc1 -std=c89 -fno-digraphs -fsyntax-only -verify -ffreestanding 
%s
+
+#if DIGRAPHS
 
+// expected-no-diagnostics
 %:include 
 
 %:ifndef BUFSIZE
@@ -14,3 +21,15 @@
 d<:len:> = s<:len:>;
 %>
 %>
+#else
+
+// expected-error@+1 {{expected identifier or '('}}
+%:include 
+;
+// expected-error@+1 {{expected ')'}} expected-note@+1{{to match this '('}}
+void copy(char d<::>);
+
+// expected-error@+1 {{expected function body}}
+void copy() <% %>
+
+#endif
Index: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp
@@ -3925,6 +3925,7 @@
   // Forward -f (flag) options which we can pass directly.
   Args.AddLastArg(CmdArgs, options::OPT_femit_all_decls);
   Args.AddLastArg(CmdArgs, options::OPT_fheinous_gnu_extensions);
+  Args.AddLastArg(CmdArgs, options::OPT_fdigraphs, options::OPT_fno_digraphs);
   Args.AddLastArg(CmdArgs, options::OPT_fno_operator_names);
   Args.AddLastArg(CmdArgs, options::OPT_femulated_tls,
   options::OPT_fno_emulated_tls);
Index: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
===
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp
@@ -2173,6 +2173,8 @@
   Opts.GNUKeywords = Args.hasFlag(OPT_fgnu_keywords, OPT_fno_gnu_keywords,
   Opts.GNUKeywords);
 
+  Opts.Digraphs = Args.hasFlag(OPT_fdigraphs, OPT_fno_digraphs, Opts.Digraphs);
+
   if (Args.hasArg(OPT_fno_operator_names))
 Opts.CXXOperatorNames = 0;
 


Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -1335,6 +1335,10 @@
 def fno_diagnostics_show_option : Flag<["-"], "fno-diagnostics-show-option">, Group;
 def fno_diagnostics_show_note_include_stack : Flag<["-"], "fno-diagnostics-show-note-include-stack">,
 Flags<[CC1Option]>, Group;
+def fdigraphs : Flag<["-"], "fdigraphs">, Group, Flags<[CC1Option]>,
+  HelpText<"Enable alternative token representations '<:', ':>', '<%', '%>', '%:', '%:%:' (default)">;
+def fno_digraphs : Flag<["-"], "fno-digraphs">, Group, Flags<[CC1Option]>,
+  HelpText<"Disallow alternative token representations '<:', ':>', '<%', '%>', '%:', '%:%:'">;
 def fno_declspec : Flag<["-"], "fno-declspec">, Group,
   HelpText<"Disallow __declspec as a keyword">, Flags<[CC1Option]>;
 def fno_dollars_in_identifiers : Flag<["-"], "fno-dollars-in-identifiers">, Group,
Index: cfe/trunk/test/Lexer/digraph.c

[PATCH] D49348: Harden/relax clang/test/CodeGen/opt-record-MIR.c test

[Adam Nemet via Phabricator via cfe-commits]

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

LGTM, thanks!


Repository:
  rC Clang

https://reviews.llvm.org/D49348



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

r337231 - [analyzer] Fix size_t in tests.

[Artem Dergachev via cfe-commits]

Author: dergachev
Date: Mon Jul 16 18:39:25 2018
New Revision: 337231

URL: http://llvm.org/viewvc/llvm-project?rev=337231=rev
Log:
[analyzer] Fix size_t in tests.

Should fix a buildbot. No functional change intended.



Modified:
cfe/trunk/test/Analysis/pr37802.cpp

Modified: cfe/trunk/test/Analysis/pr37802.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/pr37802.cpp?rev=337231=337230=337231=diff
==
--- cfe/trunk/test/Analysis/pr37802.cpp (original)
+++ cfe/trunk/test/Analysis/pr37802.cpp Mon Jul 16 18:39:25 2018
@@ -2,7 +2,8 @@
 
 // expected-no-diagnostics
 
-void *operator new(unsigned long, void *h) { return h; }
+typedef __typeof(sizeof(int)) size_t;
+void *operator new(size_t, void *h) { return h; }
 
 // I've no idea what this code does, but it used to crash, so let's keep it.
 namespace pr37802_v1 {


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

[PATCH] D47757: [Sema] Produce diagnostics when unavailable aligned allocation/deallocation functions are called

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

ping


Repository:
  rC Clang

https://reviews.llvm.org/D47757



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

[PATCH] D49360: [analyzer] Add support for more basic_string API in DanglingInternalBufferChecker

[Devin Coughlin via Phabricator via cfe-commits]

dcoughlin added a comment.

It is really nice to see this checker take shape! Some drive by diagnostic 
comments in line.




Comment at: test/Analysis/dangling-internal-buffer.cpp:175
   std::string s;
-  {
-c = s.c_str();
-  }
-  consume(c); // no-warning
+  c = s.c_str(); // expected-note {{Pointer to dangling buffer was obtained 
here}}
+  s.clear(); // expected-note {{Method call is allowed to invalidate the 
internal buffer}}

In other parts of clang we use the term "inner pointer" to mean a pointer that 
will be invalidated if its containing object is destroyed 
https://clang.llvm.org/docs/AutomaticReferenceCounting.html#interior-pointers. 
There are existing attributes that use this term to specify that a method 
returns an inner pointer.

I think it would be good to use the same terminology here. So the diagnostic 
could be something like "Dangling inner pointer obtained here".



Comment at: test/Analysis/dangling-internal-buffer.cpp:176
+  c = s.c_str(); // expected-note {{Pointer to dangling buffer was obtained 
here}}
+  s.clear(); // expected-note {{Method call is allowed to invalidate the 
internal buffer}}
+  consume(c);// expected-warning {{Use of memory after it is freed}}

What do you think about explicitly mentioning the name of the method here when 
we have it? This will make it more clear when there are multiple methods on the 
same line.

I also think that instead of saying "is allowed to" (which raises the question 
"by whom?") you could make it more direct.

How about:
"Inner pointer invalidated by call to 'clear'"

or, for the destructor "Inner pointer invalidated by call to destructor"?

What do you think?

If you're worried about this wording being to strong, you could weaken it with 
a "may be" to:

"Inner pointer may be invalidated by call to 'clear'"




https://reviews.llvm.org/D49360



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

[PATCH] D49215: [analyzer] Admit that some copy/move constructors have more than one argument.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL337229: [CFG] [analyzer] Allow elidable copies to have more 
than one arguments. (authored by dergachev, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D49215?vs=155105=155800#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D49215

Files:
  cfe/trunk/lib/Analysis/CFG.cpp
  cfe/trunk/test/Analysis/cfg-rich-constructors.cpp


Index: cfe/trunk/lib/Analysis/CFG.cpp
===
--- cfe/trunk/lib/Analysis/CFG.cpp
+++ cfe/trunk/lib/Analysis/CFG.cpp
@@ -1263,7 +1263,6 @@
 // Support pre-C++17 copy elision AST.
 auto *CE = cast(Child);
 if (BuildOpts.MarkElidedCXXConstructors && CE->isElidable()) {
-  assert(CE->getNumArgs() == 1);
   findConstructionContexts(withExtraLayer(CE), CE->getArg(0));
 }
 
Index: cfe/trunk/test/Analysis/cfg-rich-constructors.cpp
===
--- cfe/trunk/test/Analysis/cfg-rich-constructors.cpp
+++ cfe/trunk/test/Analysis/cfg-rich-constructors.cpp
@@ -878,3 +878,26 @@
   useDByReference(D());
 }
 } // end namespace argument_constructors
+
+namespace copy_elision_with_extra_arguments {
+class C {
+public:
+  C();
+  C(const C , int x = 0);
+};
+
+// CHECK: void testCopyElisionWhenCopyConstructorHasExtraArguments()
+// CHECK:[B1]
+// CXX11-ELIDE-NEXT: 1: copy_elision_with_extra_arguments::C() 
(CXXConstructExpr, [B1.3], [B1.5], class copy_elision_with_extra_arguments::C)
+// CXX11-NOELIDE-NEXT: 1: copy_elision_with_extra_arguments::C() 
(CXXConstructExpr, [B1.3], class copy_elision_with_extra_arguments::C)
+// CXX11-NEXT: 2: [B1.1] (ImplicitCastExpr, NoOp, const class 
copy_elision_with_extra_arguments::C)
+// CXX11-NEXT: 3: [B1.2]
+// CXX11-NEXT: 4:
+// CXX11-NEXT: 5: [B1.3] (CXXConstructExpr, [B1.6], class 
copy_elision_with_extra_arguments::C)
+// CXX11-NEXT: 6: copy_elision_with_extra_arguments::C c = 
copy_elision_with_extra_arguments::C();
+// CXX17-NEXT: 1: copy_elision_with_extra_arguments::C() 
(CXXConstructExpr, [B1.2], class copy_elision_with_extra_arguments::C)
+// CXX17-NEXT: 2: copy_elision_with_extra_arguments::C c = 
copy_elision_with_extra_arguments::C();
+void testCopyElisionWhenCopyConstructorHasExtraArguments() {
+  C c = C();
+}
+} // namespace copy_elision_with_extra_arguments


Index: cfe/trunk/lib/Analysis/CFG.cpp
===
--- cfe/trunk/lib/Analysis/CFG.cpp
+++ cfe/trunk/lib/Analysis/CFG.cpp
@@ -1263,7 +1263,6 @@
 // Support pre-C++17 copy elision AST.
 auto *CE = cast(Child);
 if (BuildOpts.MarkElidedCXXConstructors && CE->isElidable()) {
-  assert(CE->getNumArgs() == 1);
   findConstructionContexts(withExtraLayer(CE), CE->getArg(0));
 }
 
Index: cfe/trunk/test/Analysis/cfg-rich-constructors.cpp
===
--- cfe/trunk/test/Analysis/cfg-rich-constructors.cpp
+++ cfe/trunk/test/Analysis/cfg-rich-constructors.cpp
@@ -878,3 +878,26 @@
   useDByReference(D());
 }
 } // end namespace argument_constructors
+
+namespace copy_elision_with_extra_arguments {
+class C {
+public:
+  C();
+  C(const C , int x = 0);
+};
+
+// CHECK: void testCopyElisionWhenCopyConstructorHasExtraArguments()
+// CHECK:[B1]
+// CXX11-ELIDE-NEXT: 1: copy_elision_with_extra_arguments::C() (CXXConstructExpr, [B1.3], [B1.5], class copy_elision_with_extra_arguments::C)
+// CXX11-NOELIDE-NEXT: 1: copy_elision_with_extra_arguments::C() (CXXConstructExpr, [B1.3], class copy_elision_with_extra_arguments::C)
+// CXX11-NEXT: 2: [B1.1] (ImplicitCastExpr, NoOp, const class copy_elision_with_extra_arguments::C)
+// CXX11-NEXT: 3: [B1.2]
+// CXX11-NEXT: 4:
+// CXX11-NEXT: 5: [B1.3] (CXXConstructExpr, [B1.6], class copy_elision_with_extra_arguments::C)
+// CXX11-NEXT: 6: copy_elision_with_extra_arguments::C c = copy_elision_with_extra_arguments::C();
+// CXX17-NEXT: 1: copy_elision_with_extra_arguments::C() (CXXConstructExpr, [B1.2], class copy_elision_with_extra_arguments::C)
+// CXX17-NEXT: 2: copy_elision_with_extra_arguments::C c = copy_elision_with_extra_arguments::C();
+void testCopyElisionWhenCopyConstructorHasExtraArguments() {
+  C c = C();
+}
+} // namespace copy_elision_with_extra_arguments
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r337229 - [CFG] [analyzer] Allow elidable copies to have more than one arguments.

[Artem Dergachev via cfe-commits]

Author: dergachev
Date: Mon Jul 16 17:57:57 2018
New Revision: 337229

URL: http://llvm.org/viewvc/llvm-project?rev=337229=rev
Log:
[CFG] [analyzer] Allow elidable copies to have more than one arguments.

Copy-constructors and move-constructors may have default arguments. It is
incorrect to assert that they only have one argument, i.e. the reference to the
object being copied or moved. Remove the assertion.

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

Modified:
cfe/trunk/lib/Analysis/CFG.cpp
cfe/trunk/test/Analysis/cfg-rich-constructors.cpp

Modified: cfe/trunk/lib/Analysis/CFG.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CFG.cpp?rev=337229=337228=337229=diff
==
--- cfe/trunk/lib/Analysis/CFG.cpp (original)
+++ cfe/trunk/lib/Analysis/CFG.cpp Mon Jul 16 17:57:57 2018
@@ -1263,7 +1263,6 @@ void CFGBuilder::findConstructionContext
 // Support pre-C++17 copy elision AST.
 auto *CE = cast(Child);
 if (BuildOpts.MarkElidedCXXConstructors && CE->isElidable()) {
-  assert(CE->getNumArgs() == 1);
   findConstructionContexts(withExtraLayer(CE), CE->getArg(0));
 }
 

Modified: cfe/trunk/test/Analysis/cfg-rich-constructors.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/cfg-rich-constructors.cpp?rev=337229=337228=337229=diff
==
--- cfe/trunk/test/Analysis/cfg-rich-constructors.cpp (original)
+++ cfe/trunk/test/Analysis/cfg-rich-constructors.cpp Mon Jul 16 17:57:57 2018
@@ -878,3 +878,26 @@ void passArgumentWithDestructorByReferen
   useDByReference(D());
 }
 } // end namespace argument_constructors
+
+namespace copy_elision_with_extra_arguments {
+class C {
+public:
+  C();
+  C(const C , int x = 0);
+};
+
+// CHECK: void testCopyElisionWhenCopyConstructorHasExtraArguments()
+// CHECK:[B1]
+// CXX11-ELIDE-NEXT: 1: copy_elision_with_extra_arguments::C() 
(CXXConstructExpr, [B1.3], [B1.5], class copy_elision_with_extra_arguments::C)
+// CXX11-NOELIDE-NEXT: 1: copy_elision_with_extra_arguments::C() 
(CXXConstructExpr, [B1.3], class copy_elision_with_extra_arguments::C)
+// CXX11-NEXT: 2: [B1.1] (ImplicitCastExpr, NoOp, const class 
copy_elision_with_extra_arguments::C)
+// CXX11-NEXT: 3: [B1.2]
+// CXX11-NEXT: 4:
+// CXX11-NEXT: 5: [B1.3] (CXXConstructExpr, [B1.6], class 
copy_elision_with_extra_arguments::C)
+// CXX11-NEXT: 6: copy_elision_with_extra_arguments::C c = 
copy_elision_with_extra_arguments::C();
+// CXX17-NEXT: 1: copy_elision_with_extra_arguments::C() 
(CXXConstructExpr, [B1.2], class copy_elision_with_extra_arguments::C)
+// CXX17-NEXT: 2: copy_elision_with_extra_arguments::C c = 
copy_elision_with_extra_arguments::C();
+void testCopyElisionWhenCopyConstructorHasExtraArguments() {
+  C c = C();
+}
+} // namespace copy_elision_with_extra_arguments


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

[PATCH] D49317: Move __construct_forward (etc.) out of std::allocator_traits.

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added inline comments.



Comment at: include/vector:298
+__copy_construct_forward(_Alloc& __a, _Iter __begin1, _Iter __end1,
+ _Ptr& __begin2, _CopyViaMemcpy)
+{

vsapsai wrote:
> Why does this function use `_CopyViaMemcpy` and not `false_type` like other 
> functions?
Oops, that's totally cruft left over from an earlier revision. Fixed!



Comment at: include/vector:300
+{
+using _Alloc_traits = allocator_traits<_Alloc>;
+for (; __begin1 != __end1; ++__begin1, (void)++__begin2)

vsapsai wrote:
> Have you checked why `using` is accepted in C++03 mode? The tests are passing 
> but I expected a compiler warning and didn't investigate further.
I talked with Glen Fernandes about this on Slack the other day. I think the 
deal is that `make check-cxx` runs only the `-std=c++2a` tests, and if you want 
`-std=c++03` you have to run them manually with `llvm-lit --param=-std=c++03 
-sv path/to/tests`. Which of course I didn't do. :)
If there's a more foolproof way of automatically testing libc++ in *all* 
compiler modes, I'd like to know about it.

Fixed!



Comment at: include/vector:542
+template
+struct __vector_copy_via_memcpy : integral_constant >::value || 
!__has_construct<_Allocator, _Tp*, _Tp>::value) &&

vsapsai wrote:
> I think the name `__vector_constructable_via_memcpy` better reflects the 
> meaning. It detects cases when individual element construction can be safely 
> replaced with memcpy, so it feels more about construct than about copy. And 
> `copy_via_memcpy` is too imperative as for me, not really conveying it has 
> boolean semantic.
> `copy_via_memcpy` is too imperative for me

I see your point. However, for background... in my other branch, this trait is 
joined by two companions:
```
struct __vector_relocate_via_memcpy
struct __vector_destroy_via_noop
```
So I'd like a naming scheme that fits all three use-cases comfortably.

How about just adding the word "should"?  
`__vector_should_construct_via_memcpy`, `__vector_should_destroy_via_noop`, 
etc?  Would that sufficiently address the "too imperative" issue?



Comment at: include/vector:1015
 {
+typedef typename __vector_copy_via_memcpy<_Tp, _Allocator>::type 
__copy_via_memcpy;
 __annotate_delete();

vsapsai wrote:
> It's not immediately obvious why there is no check like 
> `is_same<_ForwardIterator, _Tp*>` here. My guess is that we are using 
> variables like `this->__end_`, `v.__begin_` that we know are pointers. Don't 
> think it's really a problem and not suggesting any changes, decided to 
> mention it's a little bit tricky to understand.
Your guess is 100% correct, AFAIK. All we're doing here is copying from one 
`__split_buffer` to another, so both sides are always a contiguous range.


Repository:
  rCXX libc++

https://reviews.llvm.org/D49317



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

[PATCH] D49317: Move __construct_forward (etc.) out of std::allocator_traits.

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone updated this revision to Diff 155796.
Quuxplusone marked 4 inline comments as done.
Quuxplusone added a comment.

Address @vsapsai's review comments.


Repository:
  rCXX libc++

https://reviews.llvm.org/D49317

Files:
  include/memory
  include/vector
  test/libcxx/containers/sequences/vector/specialized_allocator_traits.pass.cpp

Index: test/libcxx/containers/sequences/vector/specialized_allocator_traits.pass.cpp
===
--- /dev/null
+++ test/libcxx/containers/sequences/vector/specialized_allocator_traits.pass.cpp
@@ -0,0 +1,100 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// UNSUPPORTED: c++98, c++03
+
+// 
+
+// Test that vector does not use non-standard members of std::allocator_traits.
+// Specializing std::allocator_traits is arguably non-conforming, but libc++'s
+// support for specialized std::allocator_traits is a feature, not a bug.
+// Breaking (and subsequently deleting) this unit test should be done as a
+// conscious decision.
+
+#include 
+
+template 
+class A1
+{
+public:
+using value_type = T;
+
+A1() = default;
+
+template 
+A1(const A1&) {}
+
+T *allocate(std::size_t n)
+{
+return (T *)std::malloc(n * sizeof (T));
+}
+
+void deallocate(T* p, std::size_t)
+{
+std::free(p);
+}
+};
+
+template
+struct std::allocator_traits> {
+using allocator_type = A1;
+using value_type = T;
+using pointer = T*;
+using const_pointer = const T*;
+using void_pointer = void*;
+using const_void_pointer = const void*;
+using difference_type = std::ptrdiff_t;
+using size_type = std::size_t;
+using propagate_on_container_copy_assignment = std::true_type;
+using propagate_on_container_move_assignment = std::true_type;
+using propagate_on_container_swap = std::true_type;
+using is_always_equal = std::true_type;
+
+template using rebind_alloc = A1;
+template using rebind_traits = std::allocator_traits>;
+
+static T *allocate(A1& a, size_t n) {
+return a.allocate(n);
+}
+
+static void deallocate(A1& a, T *p, size_t n) {
+return a.deallocate(p, n);
+}
+
+template
+static void construct(A1&, U *p, Args&&... args) {
+::new ((void*)p) U(std::forward(args)...);
+}
+
+template
+static void destroy(A1&, U *p) {
+p->~U();
+}
+
+static A1 select_on_container_copy_construction(const A1& a) {
+return a.select_on_container_copy_construction();
+}
+
+static size_type max_size(const A1&) {
+return size_t(-1);
+}
+};
+
+int main()
+{
+std::vector> v = {1, 2, 3};
+v.resize(10);
+v.insert(v.begin() + 4, 4);
+assert(v[0] == 1);
+assert(v[1] == 2);
+assert(v[2] == 3);
+assert(v[3] == 0);
+assert(v[4] == 4);
+assert(v[5] == 0);
+}
Index: include/vector
===
--- include/vector
+++ include/vector
@@ -291,6 +291,84 @@
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
+template 
+_LIBCPP_INLINE_VISIBILITY
+inline void
+__copy_construct_forward(_Alloc& __a, _Iter __begin1, _Iter __end1,
+ _Ptr& __begin2, false_type)
+{
+typedef allocator_traits<_Alloc> _Alloc_traits;
+for (; __begin1 != __end1; ++__begin1, (void)++__begin2)
+_Alloc_traits::construct(__a, _VSTD::__to_raw_pointer(__begin2), *__begin1);
+}
+
+template 
+_LIBCPP_INLINE_VISIBILITY
+inline void
+__copy_construct_forward(_Alloc&, _Iter __begin1, _Iter __end1,
+ _Ptr& __begin2, true_type)
+{
+typedef typename iterator_traits<_Iter>::value_type _Tp;
+typedef typename remove_const<_Tp>::type _Vp;
+ptrdiff_t _Np = __end1 - __begin1;
+if (_Np > 0) {
+_VSTD::memcpy(const_cast<_Vp*>(_VSTD::__to_raw_pointer(__begin2)), _VSTD::__to_raw_pointer(__begin1), _Np * sizeof(_Tp));
+__begin2 += _Np;
+}
+}
+
+template 
+_LIBCPP_INLINE_VISIBILITY
+inline void
+__move_construct_forward(_Alloc& __a, _Ptr __begin1, _Ptr __end1,
+ _Ptr& __begin2, false_type)
+{
+typedef allocator_traits<_Alloc> _Alloc_traits;
+for (; __begin1 != __end1; ++__begin1, ++__begin2)
+_Alloc_traits::construct(__a, _VSTD::__to_raw_pointer(__begin2), _VSTD::move_if_noexcept(*__begin1));
+}
+
+template 
+_LIBCPP_INLINE_VISIBILITY
+inline void
+__move_construct_forward(_Alloc&, _Ptr __begin1, _Ptr __end1,
+ _Ptr& __begin2, true_type)
+{
+typedef typename iterator_traits<_Ptr>::value_type _Tp;
+ptrdiff_t _Np = __end1 - __begin1;
+if (_Np > 0) {
+

r337228 - [analyzer] pr37802: Fix symbolic-pointer-to-boolean casts during load.

[Artem Dergachev via cfe-commits]

Author: dergachev
Date: Mon Jul 16 17:42:35 2018
New Revision: 337228

URL: http://llvm.org/viewvc/llvm-project?rev=337228=rev
Log:
[analyzer] pr37802: Fix symbolic-pointer-to-boolean casts during load.

The canonical representation of pointer {$x} casted to boolean is
"$x != 0", not "$x". Assertion added in r337227 catches that.

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

Added:
cfe/trunk/test/Analysis/pr37802.cpp
Modified:
cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
cfe/trunk/test/Analysis/casts.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp?rev=337228=337227=337228=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp Mon Jul 16 17:42:35 
2018
@@ -159,7 +159,8 @@ SVal SimpleSValBuilder::evalCastFromLoc(
   return nonloc::SymbolVal(SymMgr.getExtentSymbol(FTR));
 
 if (const SymbolicRegion *SymR = R->getSymbolicBase())
-  return nonloc::SymbolVal(SymR->getSymbol());
+  return makeNonLoc(SymR->getSymbol(), BO_NE,
+BasicVals.getZeroWithPtrWidth(), castTy);
 
 // FALL-THROUGH
 LLVM_FALLTHROUGH;

Modified: cfe/trunk/test/Analysis/casts.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/casts.cpp?rev=337228=337227=337228=diff
==
--- cfe/trunk/test/Analysis/casts.cpp (original)
+++ cfe/trunk/test/Analysis/casts.cpp Mon Jul 16 17:42:35 2018
@@ -35,3 +35,9 @@ int *&(char *p) {
 bool testCastToIntPtrRValueRef(char *p, int *s) {
   return castToIntPtrRValueRef(p) != s; // no-crash
 }
+
+bool retrievePointerFromBoolean(int *p) {
+  bool q;
+  *reinterpret_cast() = p;
+  return q;
+}

Added: cfe/trunk/test/Analysis/pr37802.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/pr37802.cpp?rev=337228=auto
==
--- cfe/trunk/test/Analysis/pr37802.cpp (added)
+++ cfe/trunk/test/Analysis/pr37802.cpp Mon Jul 16 17:42:35 2018
@@ -0,0 +1,106 @@
+// RUN: %clang_analyze_cc1 -w -analyzer-checker=core -verify %s
+
+// expected-no-diagnostics
+
+void *operator new(unsigned long, void *h) { return h; }
+
+// I've no idea what this code does, but it used to crash, so let's keep it.
+namespace pr37802_v1 {
+struct J {
+  int *p;
+};
+class X {
+  void *ar;
+
+public:
+  X(void *t) : ar(t) {}
+  template 
+  void f(const T ) {
+new (ar) T(t);
+  }
+};
+class Y {
+public:
+  template 
+  void f(T &&);
+  void f(J t) {
+f(*t.p);
+  }
+};
+class Z {
+  int at() const {}
+
+public:
+  Z(const Z ) {
+other.au(X(this));
+  }
+  template 
+  void au(T t) const {
+void *c = const_cast(this);
+if (at()) {
+  t.f(*static_cast(c));
+} else {
+  t.f(*static_cast(c));
+}
+  }
+};
+Z g() {
+  Z az = g();
+  Z e = az;
+  Y d;
+  e.au(d);
+}
+} // namespace pr37802_v1
+
+
+// This slightly modified code crashed differently.
+namespace pr37802_v2 {
+struct J {
+  int *p;
+};
+
+class X {
+  void *ar;
+
+public:
+  X(void *t) : ar(t) {}
+  void f(const J ) { new (ar) J(t); }
+  void f(const bool ) { new (ar) bool(t); }
+};
+
+class Y {
+public:
+  void boolf(bool &&);
+  void f(J &&);
+  void f(J t) { boolf(*t.p); }
+};
+
+class Z {
+  int at() const {}
+
+public:
+  Z(const Z ) { other.au(X(this)); }
+  void au(X t) const {
+void *c = const_cast(this);
+if (at()) {
+  t.f(*static_cast(c));
+} else {
+  t.f(*static_cast(c));
+}
+  }
+  void au(Y t) const {
+void *c = const_cast(this);
+if (at()) {
+  t.f(*static_cast(c));
+} else {
+}
+  }
+};
+
+Z g() {
+  Z az = g();
+  Z e = az;
+  Y d;
+  e.au(d);
+}
+} // namespace pr37802_v2


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

[PATCH] D48232: [analyzer] pr37802: Fix symbolic-pointer-to-boolean casts during load.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL337228: [analyzer] pr37802: Fix symbolic-pointer-to-boolean 
casts during load. (authored by dergachev, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D48232?vs=151542=155797#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D48232

Files:
  cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  cfe/trunk/test/Analysis/casts.cpp
  cfe/trunk/test/Analysis/pr37802.cpp

Index: cfe/trunk/test/Analysis/casts.cpp
===
--- cfe/trunk/test/Analysis/casts.cpp
+++ cfe/trunk/test/Analysis/casts.cpp
@@ -35,3 +35,9 @@
 bool testCastToIntPtrRValueRef(char *p, int *s) {
   return castToIntPtrRValueRef(p) != s; // no-crash
 }
+
+bool retrievePointerFromBoolean(int *p) {
+  bool q;
+  *reinterpret_cast() = p;
+  return q;
+}
Index: cfe/trunk/test/Analysis/pr37802.cpp
===
--- cfe/trunk/test/Analysis/pr37802.cpp
+++ cfe/trunk/test/Analysis/pr37802.cpp
@@ -0,0 +1,106 @@
+// RUN: %clang_analyze_cc1 -w -analyzer-checker=core -verify %s
+
+// expected-no-diagnostics
+
+void *operator new(unsigned long, void *h) { return h; }
+
+// I've no idea what this code does, but it used to crash, so let's keep it.
+namespace pr37802_v1 {
+struct J {
+  int *p;
+};
+class X {
+  void *ar;
+
+public:
+  X(void *t) : ar(t) {}
+  template 
+  void f(const T ) {
+new (ar) T(t);
+  }
+};
+class Y {
+public:
+  template 
+  void f(T &&);
+  void f(J t) {
+f(*t.p);
+  }
+};
+class Z {
+  int at() const {}
+
+public:
+  Z(const Z ) {
+other.au(X(this));
+  }
+  template 
+  void au(T t) const {
+void *c = const_cast(this);
+if (at()) {
+  t.f(*static_cast(c));
+} else {
+  t.f(*static_cast(c));
+}
+  }
+};
+Z g() {
+  Z az = g();
+  Z e = az;
+  Y d;
+  e.au(d);
+}
+} // namespace pr37802_v1
+
+
+// This slightly modified code crashed differently.
+namespace pr37802_v2 {
+struct J {
+  int *p;
+};
+
+class X {
+  void *ar;
+
+public:
+  X(void *t) : ar(t) {}
+  void f(const J ) { new (ar) J(t); }
+  void f(const bool ) { new (ar) bool(t); }
+};
+
+class Y {
+public:
+  void boolf(bool &&);
+  void f(J &&);
+  void f(J t) { boolf(*t.p); }
+};
+
+class Z {
+  int at() const {}
+
+public:
+  Z(const Z ) { other.au(X(this)); }
+  void au(X t) const {
+void *c = const_cast(this);
+if (at()) {
+  t.f(*static_cast(c));
+} else {
+  t.f(*static_cast(c));
+}
+  }
+  void au(Y t) const {
+void *c = const_cast(this);
+if (at()) {
+  t.f(*static_cast(c));
+} else {
+}
+  }
+};
+
+Z g() {
+  Z az = g();
+  Z e = az;
+  Y d;
+  e.au(d);
+}
+} // namespace pr37802_v2
Index: cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -159,7 +159,8 @@
   return nonloc::SymbolVal(SymMgr.getExtentSymbol(FTR));
 
 if (const SymbolicRegion *SymR = R->getSymbolicBase())
-  return nonloc::SymbolVal(SymR->getSymbol());
+  return makeNonLoc(SymR->getSymbol(), BO_NE,
+BasicVals.getZeroWithPtrWidth(), castTy);
 
 // FALL-THROUGH
 LLVM_FALLTHROUGH;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49265: [Tooling] Add operator== to CompileCommand

[David Blaikie via Phabricator via cfe-commits]

dblaikie accepted this revision.
dblaikie added a comment.

Looks good, Thanks!


Repository:
  rC Clang

https://reviews.llvm.org/D49265



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

[PATCH] D48205: [analyzer] Assert that nonloc::SymbolVal always wraps a non-Loc-type symbol.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC337227: [analyzer] Assert that nonloc::SymbolVal always 
wraps a non-Loc-type symbol. (authored by dergachev, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D48205

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp


Index: include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
@@ -343,11 +343,14 @@
 
 namespace nonloc {
 
-/// Represents symbolic expression.
+/// Represents symbolic expression that isn't a location.
 class SymbolVal : public NonLoc {
 public:
   SymbolVal() = delete;
-  SymbolVal(SymbolRef sym) : NonLoc(SymbolValKind, sym) { assert(sym); }
+  SymbolVal(SymbolRef sym) : NonLoc(SymbolValKind, sym) {
+assert(sym);
+assert(!Loc::isLocType(sym->getType()));
+  }
 
   SymbolRef getSymbol() const {
 return (const SymExpr *) Data;
Index: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===
--- lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -1238,7 +1238,7 @@
 
 SVal VisitSymbolData(const SymbolData *S) {
   if (const llvm::APSInt *I =
-  SVB.getKnownValue(State, nonloc::SymbolVal(S)))
+  SVB.getKnownValue(State, SVB.makeSymbolVal(S)))
 return Loc::isLocType(S->getType()) ? (SVal)SVB.makeIntLocVal(*I)
 : (SVal)SVB.makeIntVal(*I);
   return SVB.makeSymbolVal(S);


Index: include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
@@ -343,11 +343,14 @@
 
 namespace nonloc {
 
-/// Represents symbolic expression.
+/// Represents symbolic expression that isn't a location.
 class SymbolVal : public NonLoc {
 public:
   SymbolVal() = delete;
-  SymbolVal(SymbolRef sym) : NonLoc(SymbolValKind, sym) { assert(sym); }
+  SymbolVal(SymbolRef sym) : NonLoc(SymbolValKind, sym) {
+assert(sym);
+assert(!Loc::isLocType(sym->getType()));
+  }
 
   SymbolRef getSymbol() const {
 return (const SymExpr *) Data;
Index: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===
--- lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -1238,7 +1238,7 @@
 
 SVal VisitSymbolData(const SymbolData *S) {
   if (const llvm::APSInt *I =
-  SVB.getKnownValue(State, nonloc::SymbolVal(S)))
+  SVB.getKnownValue(State, SVB.makeSymbolVal(S)))
 return Loc::isLocType(S->getType()) ? (SVal)SVB.makeIntLocVal(*I)
 : (SVal)SVB.makeIntVal(*I);
   return SVB.makeSymbolVal(S);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r337227 - [analyzer] Assert that nonloc::SymbolVal always wraps a non-Loc-type symbol.

[Artem Dergachev via cfe-commits]

Author: dergachev
Date: Mon Jul 16 17:22:27 2018
New Revision: 337227

URL: http://llvm.org/viewvc/llvm-project?rev=337227=rev
Log:
[analyzer] Assert that nonloc::SymbolVal always wraps a non-Loc-type symbol.

In the current SVal hierarchy there are multiple ways of representing certain
values but few are actually used and expected to be seen by the code.

In particular, a value of a symbolic pointer is always represented by a
loc::MemRegionVal that wraps a SymbolicRegion that wraps the pointer symbol
and never by a nonloc::SymbolVal that wraps that symbol directly.

Assert the aforementioned fact. Fix one minor violation of it.

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

Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h?rev=337227=337226=337227=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h Mon Jul 
16 17:22:27 2018
@@ -343,11 +343,14 @@ private:
 
 namespace nonloc {
 
-/// Represents symbolic expression.
+/// Represents symbolic expression that isn't a location.
 class SymbolVal : public NonLoc {
 public:
   SymbolVal() = delete;
-  SymbolVal(SymbolRef sym) : NonLoc(SymbolValKind, sym) { assert(sym); }
+  SymbolVal(SymbolRef sym) : NonLoc(SymbolValKind, sym) {
+assert(sym);
+assert(!Loc::isLocType(sym->getType()));
+  }
 
   SymbolRef getSymbol() const {
 return (const SymExpr *) Data;

Modified: cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp?rev=337227=337226=337227=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp Mon Jul 16 17:22:27 
2018
@@ -1238,7 +1238,7 @@ SVal SimpleSValBuilder::simplifySVal(Pro
 
 SVal VisitSymbolData(const SymbolData *S) {
   if (const llvm::APSInt *I =
-  SVB.getKnownValue(State, nonloc::SymbolVal(S)))
+  SVB.getKnownValue(State, SVB.makeSymbolVal(S)))
 return Loc::isLocType(S->getType()) ? (SVal)SVB.makeIntLocVal(*I)
 : (SVal)SVB.makeIntVal(*I);
   return SVB.makeSymbolVal(S);


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

[PATCH] D49317: Move __construct_forward (etc.) out of std::allocator_traits.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

My review is incomplete, especially I cannot say with confidence if the 
proposed change is entirely free from unintended consequences that might break 
code not covered by the test suite. So other reviewers are welcome to chime in.




Comment at: include/vector:298
+__copy_construct_forward(_Alloc& __a, _Iter __begin1, _Iter __end1,
+ _Ptr& __begin2, _CopyViaMemcpy)
+{

Why does this function use `_CopyViaMemcpy` and not `false_type` like other 
functions?



Comment at: include/vector:300
+{
+using _Alloc_traits = allocator_traits<_Alloc>;
+for (; __begin1 != __end1; ++__begin1, (void)++__begin2)

Have you checked why `using` is accepted in C++03 mode? The tests are passing 
but I expected a compiler warning and didn't investigate further.



Comment at: include/vector:366-367
+ptrdiff_t _Np = __end1 - __begin1;
+if (_Np > 0) {
+__end2 -= _Np;
+_VSTD::memcpy(_VSTD::__to_raw_pointer(__end2), 
_VSTD::__to_raw_pointer(__begin1), _Np * sizeof(_Tp));

Good. I think decrementing `__end2` after `_Np` check is better than what we 
had before.



Comment at: include/vector:542
+template
+struct __vector_copy_via_memcpy : integral_constant >::value || 
!__has_construct<_Allocator, _Tp*, _Tp>::value) &&

I think the name `__vector_constructable_via_memcpy` better reflects the 
meaning. It detects cases when individual element construction can be safely 
replaced with memcpy, so it feels more about construct than about copy. And 
`copy_via_memcpy` is too imperative as for me, not really conveying it has 
boolean semantic.



Comment at: include/vector:1015
 {
+typedef typename __vector_copy_via_memcpy<_Tp, _Allocator>::type 
__copy_via_memcpy;
 __annotate_delete();

It's not immediately obvious why there is no check like 
`is_same<_ForwardIterator, _Tp*>` here. My guess is that we are using variables 
like `this->__end_`, `v.__begin_` that we know are pointers. Don't think it's 
really a problem and not suggesting any changes, decided to mention it's a 
little bit tricky to understand.


Repository:
  rCXX libc++

https://reviews.llvm.org/D49317



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

[PATCH] D48266: [Driver] Add -fno-digraphs

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

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

Looks good with one cleanup.




Comment at: lib/Frontend/CompilerInvocation.cpp:2177-2178
 
+  if (const Arg *A = Args.getLastArg(OPT_fdigraphs, OPT_fno_digraphs))
+Opts.Digraphs = A->getOption().matches(OPT_fdigraphs) ? 1 : 0;
+

Simplify this to `Opts.Digraphs = Args.hasFlag(OPT_fdigraphs, 
OPT_fno_diagraphs, Opts.Digraphs);`


Repository:
  rC Clang

https://reviews.llvm.org/D48266



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

[PATCH] D15225: [Driver] Sanitizer support based on runtime library presence

[Kuba (Brecka) Mracek via Phabricator via cfe-commits]

kubamracek added a comment.

This looks great to me, but someone else should review this as well.


https://reviews.llvm.org/D15225



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

[PATCH] D49360: [analyzer] Add support for more basic_string API in DanglingInternalBufferChecker

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Hmm, the destructor-specific message was pretty good, can we keep it? It should 
be possible to print a different message depending on the program point within 
`N`.


https://reviews.llvm.org/D49360



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

r337226 - Restructure checking for, and warning on, lifetime extension.

[Richard Smith via cfe-commits]

Author: rsmith
Date: Mon Jul 16 17:11:41 2018
New Revision: 337226

URL: http://llvm.org/viewvc/llvm-project?rev=337226=rev
Log:
Restructure checking for, and warning on, lifetime extension.

This change implements C++ DR1696, which makes initialization of a
reference member of a class from a temporary object ill-formed. The
standard wording here is imprecise, but we interpret it as meaning that
any time a mem-initializer would result in lifetime extension, the
program is ill-formed.

Modified:
cfe/trunk/include/clang/Basic/DiagnosticGroups.td
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Sema/Initialization.h
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/lib/Sema/SemaExprCXX.cpp
cfe/trunk/lib/Sema/SemaInit.cpp
cfe/trunk/test/Analysis/initializer.cpp
cfe/trunk/test/CXX/drs/dr16xx.cpp
cfe/trunk/test/CXX/drs/dr18xx.cpp
cfe/trunk/test/CXX/special/class.copy/p11.0x.copy.cpp
cfe/trunk/test/CXX/special/class.copy/p11.0x.move.cpp
cfe/trunk/test/CXX/special/class.ctor/p5-0x.cpp
cfe/trunk/test/CXX/temp/temp.param/p5.cpp
cfe/trunk/test/CodeGenCXX/cxx0x-initializer-stdinitializerlist.cpp
cfe/trunk/test/CodeGenCXX/temporaries.cpp
cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp
cfe/trunk/test/SemaCXX/constexpr-default-arg.cpp
cfe/trunk/test/SemaCXX/cxx0x-initializer-stdinitializerlist.cpp
cfe/trunk/test/SemaCXX/eval-crashes.cpp
cfe/trunk/test/SemaCXX/member-init.cpp
cfe/trunk/test/SemaCXX/warn-dangling-field.cpp
cfe/trunk/www/cxx_dr_status.html

Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=337226=337225=337226=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Mon Jul 16 17:11:41 2018
@@ -272,6 +272,7 @@ def ShiftOpParentheses: DiagGroup<"shift
 def OverloadedShiftOpParentheses: DiagGroup<"overloaded-shift-op-parentheses">;
 def DanglingElse: DiagGroup<"dangling-else">;
 def DanglingField : DiagGroup<"dangling-field">;
+def DanglingInitializerList : DiagGroup<"dangling-initializer-list">;
 def DistributedObjectModifiers : DiagGroup<"distributed-object-modifiers">;
 def ExpansionToDefined : DiagGroup<"expansion-to-defined">;
 def FlagEnum : DiagGroup<"flag-enum">;

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=337226=337225=337226=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon Jul 16 17:11:41 
2018
@@ -2049,10 +2049,6 @@ def err_implied_std_initializer_list_not
   "not found; include ">;
 def err_malformed_std_initializer_list : Error<
   "std::initializer_list must be a class template with a single type 
parameter">;
-def warn_dangling_std_initializer_list : Warning<
-  "array backing the initializer list will be destroyed at the end of "
-  "%select{the full-expression|the constructor}0">,
-  InGroup>;
 def err_auto_init_list_from_c : Error<
   "cannot use __auto_type with initializer list in C">;
 def err_auto_bitfield : Error<
@@ -7844,13 +7840,39 @@ def warn_bind_ref_member_to_parameter :
 def warn_init_ptr_member_to_parameter_addr : Warning<
   "initializing pointer member %0 with the stack address of parameter %1">,
   InGroup;
-def warn_bind_ref_member_to_temporary : Warning<
-  "binding reference %select{|subobject of }1member %0 to a temporary value">,
-  InGroup;
 def note_ref_or_ptr_member_declared_here : Note<
   "%select{reference|pointer}0 member declared here">;
-def note_ref_subobject_of_member_declared_here : Note<
-  "member with reference subobject declared here">;
+
+def err_bind_ref_member_to_temporary : Error<
+  "%select{reference|backing array for 'std::initializer_list'}2 "
+  "%select{|subobject of }1member %0 "
+  "%select{binds to|is}2 a temporary object "
+  "whose lifetime would be shorter than the constructed object">;
+def note_lifetime_extending_member_declared_here : Note<
+  "%select{%select{reference|'std::initializer_list'}0 member|"
+  "member with %select{reference|'std::initializer_list'}0 subobject}1 "
+  "declared here">;
+def warn_new_dangling_reference : Warning<
+  "temporary bound to reference member of allocated object "
+  "will be destroyed at the end of the full-expression">,
+  InGroup;
+def warn_new_dangling_initializer_list : Warning<
+  "array backing "
+  "%select{initializer list subobject of the allocated object|"
+  "the allocated initializer list}0 "
+  "will be destroyed at the end of the full-expression">,
+  InGroup;
+def 

[PATCH] D47358: : Implement {un, }synchronized_pool_resource.

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone updated this revision to Diff 155791.
Quuxplusone added a comment.

Implement similar cosmetic cleanup to https://reviews.llvm.org/D47111, but for 
the pool resources this time.
I think the argument for keeping do_allocate and do_deallocate in the .cpp file 
is stronger for these guys than for monotonic_buffer_resource (where Eric 
convinced me to move them into the .h file for the benefit of the inliner). 
However, I'm willing to be persuaded (as I was in that case).


Repository:
  rCXX libc++

https://reviews.llvm.org/D47358

Files:
  include/experimental/memory_resource
  src/experimental/memory_resource.cpp
  
test/libcxx/experimental/memory/memory.resource.pool/memory.resource.pool.mem/unsynchronized_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.ctor/ctor_does_not_allocate.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.ctor/sync_with_default_resource.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.ctor/unsync_with_default_resource.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/equality.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/sync_allocate.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/sync_allocate_overaligned_request.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/sync_allocate_reuse_blocks.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/sync_deallocate_matches_allocate.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/unsync_allocate.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/unsync_allocate_overaligned_request.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/unsync_allocate_reuse_blocks.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/unsync_deallocate_matches_allocate.pass.cpp
  test/std/experimental/memory/memory.resource.pool/pool_options.pass.cpp
  test/support/count_new.hpp

Index: test/support/count_new.hpp
===
--- test/support/count_new.hpp
+++ test/support/count_new.hpp
@@ -211,6 +211,11 @@
 return disable_checking || n != delete_called;
 }
 
+bool checkDeleteCalledGreaterThan(int n) const
+{
+return disable_checking || delete_called > n;
+}
+
 bool checkAlignedNewCalledEq(int n) const
 {
 return disable_checking || n == aligned_new_called;
Index: test/std/experimental/memory/memory.resource.pool/pool_options.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.pool/pool_options.pass.cpp
@@ -0,0 +1,26 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// REQUIRES: c++experimental
+// UNSUPPORTED: c++98, c++03, c++11, c++14
+// UNSUPPORTED: apple-clang-7
+
+// 
+
+// struct pool_options
+
+#include 
+#include 
+
+int main()
+{
+const std::experimental::pmr::pool_options p;
+assert(p.max_blocks_per_chunk == 0);
+assert(p.largest_required_pool_block == 0);
+}
Index: test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/unsync_deallocate_matches_allocate.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/unsync_deallocate_matches_allocate.pass.cpp
@@ -0,0 +1,110 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// REQUIRES: c++experimental
+// UNSUPPORTED: c++98, c++03
+
+// 
+
+// class unsynchronized_pool_resource
+
+#include 
+#include 
+#include 
+
+struct allocation_record {
+size_t bytes;
+size_t align;
+explicit allocation_record(size_t b, size_t a) : bytes(b), align(a) {}
+bool operator==(const allocation_record& rhs) const {
+return (bytes == rhs.bytes) && (align == rhs.align);
+}
+bool operator<(const allocation_record& rhs) const {
+if (bytes != rhs.bytes) return (bytes < rhs.bytes);
+return (align < rhs.align);
+}
+};
+
+class test_resource : public 

[PATCH] D47233: [CodeGen] Emit MSVC RTTI for Obj-C EH types

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added inline comments.



Comment at: lib/CodeGen/CGObjCMac.cpp:7457-7460
 CGObjCNonFragileABIMac::GetEHType(QualType T) {
   // There's a particular fixed type info for 'id'.
   if (T->isObjCIdType() || T->isObjCQualifiedIdType()) {
+if (CGM.getTriple().isWindowsMSVCEnvironment())

smeenai wrote:
> rjmccall wrote:
> > smeenai wrote:
> > > rjmccall wrote:
> > > > rnk wrote:
> > > > > @rjmccall how should this be organized in the long run? At this 
> > > > > point, the naming seems totally wrong. Is the non-fragile ABI sort of 
> > > > > the canonical way forward for Obj-C, i.e. it's what a new platform 
> > > > > would want to use to best stay in sync with the future of obj-c?
> > > > For Darwin, yes, absolutely.
> > > > 
> > > > I think this method should probably just completely delegate to the 
> > > > `CGCXXABI` using a new `getAddrOfObjCRTTIDescriptor` method.
> > > To be clear, you'd want the entirety of the EHType emission logic to be 
> > > shifted to CGCXXABI? I think that would make sense, and I can look into 
> > > it.
> > Right.
> Sorry, getting back to this now.
> 
> What did you have in mind for handling the different Obj-C runtimes? Were you 
> envisioning the new getAddrOfObjCRTTIDescriptor method supporting just the 
> non-fragile Mac ABI or all of them?
Looked into this more. ItaniumRTTIBuilder already has a 
BuildObjCObjectTypeInfo, which confused me for a bit until I realized it's only 
ever used for the fragile ABI, and even then only when using C++ try/catch 
(instead of Obj-C style @try/@catch), which is a bit strange. Everything else 
does its own thing.

From what I've been able to make out, these are the current possibilities for 
generating Obj-C RTTI for the Itanium ABI:
* Fragile macOS runtime using Obj-C @try/@catch: doesn't actually seem to 
generate any RTTI as far as I can tell, and uses `objc_exception_match` instead.
* Fragile macOS runtime using C++ try/catch: goes through 
`ItaniumRTTIBuilder::BuildObjCObjectTypeInfo`, which generates C++ compatible 
RTTI referencing a C++ class type info.
* Non-fragile macOS runtime: generates its own C++ compatible RTTI referencing 
`objc_ehtype_vtable`.
* GNUStep for Objective-C++: generates its own C++ compatible RTTI referencing 
`_ZTVN7gnustep7libobjc22__objc_class_type_infoE`.
* All other GNU runtimes (including GNUStep for Objective-C): just uses the 
identifier name string of the interface as its "RTTI".

I think it makes sense to only have the new `getAddrOfObjCRTTIDescriptor` 
method handle the non-fragile macOS runtime for now, and perhaps 
`ItaniumRTTIBuilder::BuildObjCObjectTypeInfo` should be renamed (or at least 
have a comment added) to indicate that it's only used for the fragile macOS 
runtime when catching an Obj-C type with C++ catch.


Repository:
  rC Clang

https://reviews.llvm.org/D47233



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

[PATCH] D15225: [Driver] Sanitizer support based on runtime library presence

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov updated this revision to Diff 155785.
george.karpenkov edited the summary of this revision.
george.karpenkov edited reviewers, added: delcypher; removed: bob.wilson, 
glider, t.p.northover, samsonov, beanz.
george.karpenkov added a comment.

Attempt #2: reduced version of this patch, without ubsan support.


https://reviews.llvm.org/D15225

Files:
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/lib/Driver/ToolChains/Darwin.h
  
clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.asan_ios_dynamic.dylib
  
clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.asan_iossim_dynamic.dylib
  
clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.asan_osx_dynamic.dylib
  
clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.asan_tvos_dynamic.dylib
  
clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.asan_tvossim_dynamic.dylib
  
clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.asan_watchos_dynamic.dylib
  
clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.asan_watchossim_dynamic.dylib
  clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.fuzzer_osx.a
  
clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.lsan_ios_dynamic.dylib
  
clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.lsan_iossim_dynamic.dylib
  
clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.lsan_osx_dynamic.dylib
  
clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.lsan_tvossim_dynamic.dylib
  
clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.tsan_iossim_dynamic.dylib
  
clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.tsan_osx_dynamic.dylib
  
clang/test/Driver/Inputs/resource_dir/lib/darwin/libclang_rt.tsan_tvossim_dynamic.dylib
  clang/test/Driver/darwin-asan-nofortify.c
  clang/test/Driver/darwin-sanitizer-ld.c
  clang/test/Driver/fsanitize.c
  clang/test/Driver/fuzzer.c
  clang/test/Driver/sanitizer-ld.c

Index: clang/test/Driver/sanitizer-ld.c
===
--- clang/test/Driver/sanitizer-ld.c
+++ clang/test/Driver/sanitizer-ld.c
@@ -530,6 +530,7 @@
 
 // RUN: %clangxx -fsanitize=address %s -### -o %t.o 2>&1 \
 // RUN: -mmacosx-version-min=10.6 \
+// RUN: -resource-dir=%S/Inputs/resource_dir \
 // RUN: -target x86_64-apple-darwin13.4.0 -fuse-ld=ld -stdlib=platform \
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-ASAN-DARWIN106-CXX %s
@@ -539,6 +540,7 @@
 
 // RUN: %clangxx -fsanitize=leak %s -### -o %t.o 2>&1 \
 // RUN: -mmacosx-version-min=10.6 \
+// RUN: -resource-dir=%S/Inputs/resource_dir \
 // RUN: -target x86_64-apple-darwin13.4.0 -fuse-ld=ld -stdlib=platform \
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-LSAN-DARWIN106-CXX %s
@@ -598,6 +600,7 @@
 
 // RUN: %clang -fsanitize=cfi -fsanitize-stats %s -### -o %t.o 2>&1 \
 // RUN: -target x86_64-apple-darwin -fuse-ld=ld \
+// RUN: -resource-dir=%S/Inputs/resource_dir \
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-CFI-STATS-DARWIN %s
 // CHECK-CFI-STATS-DARWIN: "{{.*}}ld{{(.exe)?}}"
Index: clang/test/Driver/fuzzer.c
===
--- clang/test/Driver/fuzzer.c
+++ clang/test/Driver/fuzzer.c
@@ -1,28 +1,28 @@
 // Test flags inserted by -fsanitize=fuzzer.
 
-// RUN: %clang -fsanitize=fuzzer %s -target x86_64-apple-darwin14 -### 2>&1 | FileCheck --check-prefixes=CHECK-FUZZER-LIB,CHECK-COVERAGE-FLAGS %s
+// RUN: %clang -fsanitize=fuzzer %s -target x86_64-apple-darwin14 -resource-dir %S/Inputs/resource_dir -### 2>&1 | FileCheck --check-prefixes=CHECK-FUZZER-LIB,CHECK-COVERAGE-FLAGS %s
 //
 // CHECK-FUZZER-LIB: libclang_rt.fuzzer
 // CHECK-COVERAGE: -fsanitize-coverage-inline-8bit-counters
 // CHECK-COVERAGE-SAME: -fsanitize-coverage-indirect-calls
 // CHECK-COVERAGE-SAME: -fsanitize-coverage-trace-cmp
 // CHECK-COVERAGE-SAME: -fsanitize-coverage-pc-table
 
-// RUN: %clang -fsanitize=fuzzer -target i386-unknown-linux -stdlib=platform %s -### 2>&1 | FileCheck --check-prefixes=CHECK-LIBCXX-LINUX %s
+// RUN: %clang -fsanitize=fuzzer -target i386-unknown-linux -stdlib=platform -resource-dir %S/Inputs/resource_dir %s -### 2>&1 | FileCheck --check-prefixes=CHECK-LIBCXX-LINUX %s
 //
 // CHECK-LIBCXX-LINUX: -lstdc++
 
-// RUN: %clang -target x86_64-apple-darwin14 -fsanitize=fuzzer %s -### 2>&1 | FileCheck --check-prefixes=CHECK-LIBCXX-DARWIN %s
+// RUN: %clang -target x86_64-apple-darwin14 -fsanitize=fuzzer -resource-dir %S/Inputs/resource_dir %s -### 2>&1 | FileCheck --check-prefixes=CHECK-LIBCXX-DARWIN %s
 //
 // CHECK-LIBCXX-DARWIN: -lc++
 
 
 // Check that we don't link in libFuzzer.a when producing a shared object.
-// RUN: %clang -fsanitize=fuzzer %s -shared -o %t.so -### 2>&1 | FileCheck --check-prefixes=CHECK-NOLIB-SO %s
+// RUN: %clang -fsanitize=fuzzer 

[PATCH] D41458: [libc++][C++17] Elementary string conversions for integral types

[Zhihao Yuan via Phabricator via cfe-commits]

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



Comment at: include/charconv:244
+static _LIBCPP_INLINE_VISIBILITY char const*
+read(char const* __p, char const* __ep, type& __a, type& __b)
+{

Quuxplusone wrote:
> mclow.lists wrote:
> > lichray wrote:
> > > mclow.lists wrote:
> > > > Same comment as above about `read` and `inner_product` - they need to 
> > > > be "ugly names"
> > > Unlike `traits` which is a template parameter name in the standard, 
> > > `read` and `inner_product` are function names in the standard, which 
> > > means the users cannot make a macro for them (and there is no guarantee 
> > > about what name you make **not** get by including certain headers), so we 
> > > don't need to use ugly names here, am I right?
> > I understand your reasoning, but I don't agree. 
> > 
> > Just last month, I had to rename a function in `vector` from `allocate` to 
> > `__vallocate` because it confused our "is this an allocator" detection. The 
> > function in question was private, so it shouldn't have mattered, but GCC 
> > has a bug where sometimes it partially ignores access restrictions in 
> > non-deduced contexts, and then throws a hard error when it comes back to a 
> > different context. The easiest workaround was to rename the function in 
> > `vector`.
> > 
> > Since then, I've been leery of public names that match others. This is 
> > pretty obscure, since it's in a private namespace, but I'd feel better if 
> > they were `__read` and `__inner_product`.
> > 
> FWIW, +1 to ugly names. Even if the un-ugly code is "technically not broken 
> yet", and besides the technical reason Marshall gives,
> (1) it's nice that libc++ has style rules and sticks to them, precisely to 
> *avoid* bikeshedding the name of every private member in the world;
> (2) just because users can't `#define read write` doesn't mean they *won't* 
> do it. I would actually be extremely surprised if `read` were *not* defined 
> as a macro somewhere inside ``. :)
> 
> See also: "should this function call be `_VSTD::`-qualified?" Sometimes the 
> answer is technically "no", but stylistically "yes", precisely to indicate 
> that we *don't* intend for it to be an ADL customization point. Consistent 
> style leads to maintainability.
`read` is a function decl in ``, not redefined in other forms in 
``.



Comment at: test/support/charconv_test_helpers.h:40
+constexpr bool
+_fits_in(T, true_type /* non-narrowing*/, ...)
+{

mclow.lists wrote:
> We don't need to use ugly names here in the test suite.
> 
Err, it's not?  Just an impl version of `fits_in` (without the _ prefix).


Repository:
  rCXX libc++

https://reviews.llvm.org/D41458



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

[PATCH] D48266: [Driver] Add -fno-digraphs

[Jacob Bandes-Storch via Phabricator via cfe-commits]

jtbandes added a comment.

Ping again 


Repository:
  rC Clang

https://reviews.llvm.org/D48266



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

[PATCH] D41458: [libc++][C++17] Elementary string conversions for integral types

[Zhihao Yuan via Phabricator via cfe-commits]

lichray updated this revision to Diff 155780.
lichray added a comment.

Uglify all the names


Repository:
  rCXX libc++

https://reviews.llvm.org/D41458

Files:
  include/CMakeLists.txt
  include/charconv
  include/module.modulemap
  src/charconv.cpp
  test/libcxx/double_include.sh.cpp
  test/std/utilities/charconv/
  test/std/utilities/charconv/charconv.from.chars/
  test/std/utilities/charconv/charconv.from.chars/integral.bool.fail.cpp
  test/std/utilities/charconv/charconv.from.chars/integral.pass.cpp
  test/std/utilities/charconv/charconv.to.chars/
  test/std/utilities/charconv/charconv.to.chars/integral.bool.fail.cpp
  test/std/utilities/charconv/charconv.to.chars/integral.pass.cpp
  test/support/charconv_test_helpers.h

Index: test/support/charconv_test_helpers.h
===
--- /dev/null
+++ test/support/charconv_test_helpers.h
@@ -0,0 +1,232 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef SUPPORT_CHARCONV_TEST_HELPERS_H
+#define SUPPORT_CHARCONV_TEST_HELPERS_H
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "test_macros.h"
+
+#if TEST_STD_VER <= 11
+#error This file requires C++14
+#endif
+
+using std::false_type;
+using std::true_type;
+
+template 
+constexpr auto
+is_non_narrowing(From a) -> decltype(To{a}, true_type())
+{
+return {};
+}
+
+template 
+constexpr auto
+is_non_narrowing(...) -> false_type
+{
+return {};
+}
+
+template 
+constexpr bool
+_fits_in(T, true_type /* non-narrowing*/, ...)
+{
+return true;
+}
+
+template >
+constexpr bool
+_fits_in(T v, false_type, true_type /* T signed*/, true_type /* X signed */)
+{
+return xl::lowest() <= v && v <= (xl::max)();
+}
+
+template >
+constexpr bool
+_fits_in(T v, false_type, true_type /* T signed */, false_type /* X unsigned*/)
+{
+return 0 <= v && std::make_unsigned_t(v) <= (xl::max)();
+}
+
+template >
+constexpr bool
+_fits_in(T v, false_type, false_type /* T unsigned */, ...)
+{
+return v <= std::make_unsigned_t((xl::max)());
+}
+
+template 
+constexpr bool
+fits_in(T v)
+{
+return _fits_in(v, is_non_narrowing(v), std::is_signed(),
+   std::is_signed());
+}
+
+template 
+struct to_chars_test_base
+{
+template 
+void test(T v, char const ()[N], Ts... args)
+{
+using std::to_chars;
+std::to_chars_result r;
+
+constexpr size_t len = N - 1;
+static_assert(len > 0, "expected output won't be empty");
+
+if (!fits_in(v))
+return;
+
+r = to_chars(buf, buf + len - 1, X(v), args...);
+assert(r.ptr == buf + len - 1);
+assert(r.ec == std::errc::value_too_large);
+
+r = to_chars(buf, buf + sizeof(buf), X(v), args...);
+assert(r.ptr == buf + len);
+assert(r.ec == std::errc{});
+assert(memcmp(buf, expect, len) == 0);
+}
+
+template 
+void test_value(X v, Ts... args)
+{
+using std::to_chars;
+std::to_chars_result r;
+
+r = to_chars(buf, buf + sizeof(buf), v, args...);
+assert(r.ec == std::errc{});
+*r.ptr = '\0';
+
+auto a = fromchars(buf, r.ptr, args...);
+assert(v == a);
+
+auto ep = r.ptr - 1;
+r = to_chars(buf, ep, v, args...);
+assert(r.ptr == ep);
+assert(r.ec == std::errc::value_too_large);
+}
+
+private:
+static auto fromchars(char const* p, char const* ep, int base, true_type)
+{
+char* last;
+auto r = strtoll(p, , base);
+assert(last == ep);
+
+return r;
+}
+
+static auto fromchars(char const* p, char const* ep, int base, false_type)
+{
+char* last;
+auto r = strtoull(p, , base);
+assert(last == ep);
+
+return r;
+}
+
+static auto fromchars(char const* p, char const* ep, int base = 10)
+{
+return fromchars(p, ep, base, std::is_signed());
+}
+
+char buf[100];
+};
+
+template 
+struct roundtrip_test_base
+{
+template 
+void test(T v, Ts... args)
+{
+using std::from_chars;
+using std::to_chars;
+std::from_chars_result r2;
+std::to_chars_result r;
+X x = 0xc;
+
+if (fits_in(v))
+{
+r = to_chars(buf, buf + sizeof(buf), v, args...);
+assert(r.ec == std::errc{});
+
+r2 = from_chars(buf, r.ptr, x, args...);
+assert(r2.ptr == r.ptr);
+assert(x == X(v));
+}
+else
+{
+r = to_chars(buf, buf + sizeof(buf), v, args...);
+assert(r.ec == std::errc{});
+
+r2 = from_chars(buf, r.ptr, x, args...);

[PATCH] D48786: [Preprocessor] Stop entering included files after hitting a fatal error.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Ping.


https://reviews.llvm.org/D48786



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

[PATCH] D49235: [ASTImporter] Import described template (if any) of function.

[Aleksei Sidorin via Phabricator via cfe-commits]

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

LGTM.


Repository:
  rC Clang

https://reviews.llvm.org/D49235



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

[PATCH] D49293: [ASTImporter] Add support for import of CXXInheritedCtorInitExpr.

[Aleksei Sidorin via Phabricator via cfe-commits]

a_sidorin added inline comments.



Comment at: lib/AST/ASTImporter.cpp:6741
+
+  auto *Ctor = dyn_cast(Importer.Import(
+  E->getConstructor()));

balazske wrote:
> a_sidorin wrote:
> > cast_or_null?
> dyn_cast_or_null: Import may return nullptr, but if not, the cast should 
> succeed (not a CXXConstructorDecl would be error).
> Or (but some lines more code) check separately for null return from Import, 
> and then use `cast`?
Usually, we explicitly check that the imported decl has the same kind as the 
source one by filtering lookup results or by creating a decl of same kind. So, 
it's better to assert with cast_or_null in case of kind mismatch.


Repository:
  rC Clang

https://reviews.llvm.org/D49293



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

[PATCH] D49296: [ASTImporter] Fix import of unnamed structs

[Aleksei Sidorin via Phabricator via cfe-commits]

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

Thank you Gabor!


Repository:
  rC Clang

https://reviews.llvm.org/D49296



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

[PATCH] D49265: [Tooling] Add operator== to CompileCommand

[Simon Marchi via Phabricator via cfe-commits]

simark updated this revision to Diff 155773.
simark added a comment.

Add tests for both == and !=.

I need to rebuild ~800 files (because I pulled llvm/clang), so I did not
actually test it, but I'll do so before pushing tomorrow, of course.


Repository:
  rC Clang

https://reviews.llvm.org/D49265

Files:
  include/clang/Tooling/CompilationDatabase.h
  unittests/Tooling/CompilationDatabaseTest.cpp


Index: unittests/Tooling/CompilationDatabaseTest.cpp
===
--- unittests/Tooling/CompilationDatabaseTest.cpp
+++ unittests/Tooling/CompilationDatabaseTest.cpp
@@ -736,5 +736,33 @@
   EXPECT_EQ(getCommand("foo/bar/baz/shout.C"), "clang -D 
FOO/BAR/BAZ/SHOUT.cc");
 }
 
+TEST(CompileCommandTest, EqualityOperator) {
+  CompileCommand CCRef("/foo/bar", "hello.c", {"a", "b"}, "hello.o");
+  CompileCommand CCTest = CCRef;
+
+  EXPECT_TRUE(CCRef == CCTest);
+  EXPECT_FALSE(CCRef != CCTest);
+
+  CCTest = CCRef;
+  CCTest.Directory = "/foo/baz";
+  EXPECT_FALSE(CCRef == CCTest);
+  EXPECT_TRUE(CCRef != CCTest);
+
+  CCTest = CCRef;
+  CCTest.Filename = "bonjour.c";
+  EXPECT_FALSE(CCRef == CCTest);
+  EXPECT_TRUE(CCRef != CCTest);
+
+  CCTest = CCRef;
+  CCTest.CommandLine.push_back("c");
+  EXPECT_FALSE(CCRef == CCTest);
+  EXPECT_TRUE(CCRef != CCTest);
+
+  CCTest = CCRef;
+  CCTest.Output = "bonjour.o";
+  EXPECT_FALSE(CCRef == CCTest);
+  EXPECT_TRUE(CCRef != CCTest);
+}
+
 } // end namespace tooling
 } // end namespace clang
Index: include/clang/Tooling/CompilationDatabase.h
===
--- include/clang/Tooling/CompilationDatabase.h
+++ include/clang/Tooling/CompilationDatabase.h
@@ -59,6 +59,15 @@
 
   /// The output file associated with the command.
   std::string Output;
+
+  friend bool operator==(const CompileCommand , const CompileCommand ) 
{
+return LHS.Directory == RHS.Directory && LHS.Filename == RHS.Filename &&
+   LHS.CommandLine == RHS.CommandLine && LHS.Output == RHS.Output;
+  }
+
+  friend bool operator!=(const CompileCommand , const CompileCommand ) 
{
+return !(LHS == RHS);
+  }
 };
 
 /// Interface for compilation databases.


Index: unittests/Tooling/CompilationDatabaseTest.cpp
===
--- unittests/Tooling/CompilationDatabaseTest.cpp
+++ unittests/Tooling/CompilationDatabaseTest.cpp
@@ -736,5 +736,33 @@
   EXPECT_EQ(getCommand("foo/bar/baz/shout.C"), "clang -D FOO/BAR/BAZ/SHOUT.cc");
 }
 
+TEST(CompileCommandTest, EqualityOperator) {
+  CompileCommand CCRef("/foo/bar", "hello.c", {"a", "b"}, "hello.o");
+  CompileCommand CCTest = CCRef;
+
+  EXPECT_TRUE(CCRef == CCTest);
+  EXPECT_FALSE(CCRef != CCTest);
+
+  CCTest = CCRef;
+  CCTest.Directory = "/foo/baz";
+  EXPECT_FALSE(CCRef == CCTest);
+  EXPECT_TRUE(CCRef != CCTest);
+
+  CCTest = CCRef;
+  CCTest.Filename = "bonjour.c";
+  EXPECT_FALSE(CCRef == CCTest);
+  EXPECT_TRUE(CCRef != CCTest);
+
+  CCTest = CCRef;
+  CCTest.CommandLine.push_back("c");
+  EXPECT_FALSE(CCRef == CCTest);
+  EXPECT_TRUE(CCRef != CCTest);
+
+  CCTest = CCRef;
+  CCTest.Output = "bonjour.o";
+  EXPECT_FALSE(CCRef == CCTest);
+  EXPECT_TRUE(CCRef != CCTest);
+}
+
 } // end namespace tooling
 } // end namespace clang
Index: include/clang/Tooling/CompilationDatabase.h
===
--- include/clang/Tooling/CompilationDatabase.h
+++ include/clang/Tooling/CompilationDatabase.h
@@ -59,6 +59,15 @@
 
   /// The output file associated with the command.
   std::string Output;
+
+  friend bool operator==(const CompileCommand , const CompileCommand ) {
+return LHS.Directory == RHS.Directory && LHS.Filename == RHS.Filename &&
+   LHS.CommandLine == RHS.CommandLine && LHS.Output == RHS.Output;
+  }
+
+  friend bool operator!=(const CompileCommand , const CompileCommand ) {
+return !(LHS == RHS);
+  }
 };
 
 /// Interface for compilation databases.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49360: [analyzer] Add support for more basic_string API in DanglingInternalBufferChecker

[Reka Kovacs via Phabricator via cfe-commits]

rnkovacs added a comment.

In https://reviews.llvm.org/D49360#1163113, @NoQ wrote:

> Also we rarely commit to adding a test for every single supported API 
> function; bonus points for that, but usually 2-3 functions from a series of 
> similar functions is enough :)


Um, okay, noted for next time :)


https://reviews.llvm.org/D49360



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

[PATCH] D49300: [ASTImporter] Fix poisonous structural equivalence cache

[Aleksei Sidorin via Phabricator via cfe-commits]

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

Hi Gabor,

Thank you for this explanation, it sounds reasonable. Ithink it should be 
placed into the commit message or into the comment somewhere.


Repository:
  rC Clang

https://reviews.llvm.org/D49300



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

[PATCH] D49360: [analyzer] Add support for more basic_string API in DanglingInternalBufferChecker

[Reka Kovacs via Phabricator via cfe-commits]

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

Added standard quote, marking the section about non-member functions that may 
also invalidate the buffer as a TODO.
Also changed the note message to that suggested by @NoQ (thanks!). All tests 
pass now.


https://reviews.llvm.org/D49360

Files:
  lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
  lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  test/Analysis/dangling-internal-buffer.cpp

Index: test/Analysis/dangling-internal-buffer.cpp
===
--- test/Analysis/dangling-internal-buffer.cpp
+++ test/Analysis/dangling-internal-buffer.cpp
@@ -2,13 +2,35 @@
 
 namespace std {
 
-template< typename CharT >
+typedef int size_type;
+
+template 
 class basic_string {
 public:
+  basic_string();
+  basic_string(const CharT *s);
+
   ~basic_string();
+  void clear();
+
+  basic_string =(const basic_string );
+  basic_string +=(const basic_string );
+
   const CharT *c_str() const;
   const CharT *data() const;
   CharT *data();
+
+  basic_string (size_type count, CharT ch);
+  basic_string (size_type count, CharT ch);
+  basic_string (size_type index, size_type count);
+  basic_string (size_type index, size_type count, CharT ch);
+  basic_string (size_type pos, size_type count, const basic_string );
+  void pop_back();
+  void push_back(CharT ch);
+  void reserve(size_type new_cap);
+  void resize(size_type count);
+  void shrink_to_fit();
+  void swap(basic_string );
 };
 
 typedef basic_string string;
@@ -23,73 +45,70 @@
 void consume(const char16_t *) {}
 void consume(const char32_t *) {}
 
-void deref_after_scope_char_cstr() {
-  const char *c;
+void deref_after_scope_char(bool cond) {
+  const char *c, *d;
   {
 std::string s;
 c = s.c_str(); // expected-note {{Pointer to dangling buffer was obtained here}}
-  } // expected-note {{Internal buffer is released because the object was destroyed}}
+d = s.data();  // expected-note {{Pointer to dangling buffer was obtained here}}
+  }// expected-note {{Method call is allowed to invalidate the internal buffer}}
+  // expected-note@-1 {{Method call is allowed to invalidate the internal buffer}}
   std::string s;
   const char *c2 = s.c_str();
-  consume(c); // expected-warning {{Use of memory after it is freed}}
-  // expected-note@-1 {{Use of memory after it is freed}}
-}
-
-void deref_after_scope_char_data() {
-  const char *c;
-  {
-std::string s;
-c = s.data(); // expected-note {{Pointer to dangling buffer was obtained here}}
-  } // expected-note {{Internal buffer is released because the object was destroyed}}
-  std::string s;
-  const char *c2 = s.data();
-  consume(c); // expected-warning {{Use of memory after it is freed}}
-  // expected-note@-1 {{Use of memory after it is freed}}
+  if (cond) {
+// expected-note@-1 {{Assuming 'cond' is not equal to 0}}
+// expected-note@-2 {{Taking true branch}}
+// expected-note@-3 {{Assuming 'cond' is 0}}
+// expected-note@-4 {{Taking false branch}}
+consume(c); // expected-warning {{Use of memory after it is freed}}
+// expected-note@-1 {{Use of memory after it is freed}}
+  } else {
+consume(d); // expected-warning {{Use of memory after it is freed}}
+// expected-note@-1 {{Use of memory after it is freed}}
+  }
 }
 
 void deref_after_scope_char_data_non_const() {
   char *c;
   {
 std::string s;
 c = s.data(); // expected-note {{Pointer to dangling buffer was obtained here}}
-  } // expected-note {{Internal buffer is released because the object was destroyed}}
+  }   // expected-note {{Method call is allowed to invalidate the internal buffer}}
   std::string s;
   char *c2 = s.data();
   consume(c); // expected-warning {{Use of memory after it is freed}}
   // expected-note@-1 {{Use of memory after it is freed}}
 }
 
-
-void deref_after_scope_wchar_t_cstr() {
-  const wchar_t *w;
+void deref_after_scope_wchar_t(bool cond) {
+  const wchar_t *c, *d;
   {
-std::wstring ws;
-w = ws.c_str(); // expected-note {{Pointer to dangling buffer was obtained here}}
-  } // expected-note {{Internal buffer is released because the object was destroyed}}
-  std::wstring ws;
-  const wchar_t *w2 = ws.c_str();
-  consume(w); // expected-warning {{Use of memory after it is freed}}
-  // expected-note@-1 {{Use of memory after it is freed}}
-}
-
-void deref_after_scope_wchar_t_data() {
-  const wchar_t *w;
-  {
-std::wstring ws;
-w = ws.data(); // expected-note {{Pointer to dangling buffer was obtained here}}
-  } // expected-note {{Internal buffer is released because the object was destroyed}}
-  std::wstring ws;
-  const wchar_t *w2 = ws.data();
-  consume(w); // expected-warning {{Use of memory after it is freed}}
-  // expected-note@-1 {{Use of memory after it is freed}}
+std::wstring s;
+c = 

[PATCH] D30691: [analyzer] Support for naive cross translational unit analysis

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.
Herald added a subscriber: mikhail.ramalho.

Just noticed: `getRuntimeDefinition()` has a lot of overrides in `CallEvent` 
sub-classes, and there paths that don't defer to 
`AnyFunctionCall::getRuntimeDefinition()`, eg., ` 
CXXInstanceCall::getRuntimeDefinition()` => `if (MD->isVirtual()) ...`. Which 
means that for some calls we aren't even trying to make a CTU lookup.


Repository:
  rC Clang

https://reviews.llvm.org/D30691



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

[PATCH] D49265: [Tooling] Add operator== to CompileCommand

[Simon Marchi via Phabricator via cfe-commits]

simark added a comment.

In https://reviews.llvm.org/D49265#1164227, @dblaikie wrote:

> In theory you'd need separate tests for op== and op!= returning false 
> (currently all the tests would pass if both implementations returned true in 
> all cases), but not the biggest deal.


Good point, I'll update it, it will take a second.


Repository:
  rC Clang

https://reviews.llvm.org/D49265



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

[PATCH] D45124: [CodeGen] Record if a C++ record is a trivial type when emitting Codeview

[Aaron Smith via Phabricator via cfe-commits]

asmith added a reviewer: aleksandr.urakov.
asmith added a comment.

Adding Aleksandr


Repository:
  rC Clang

https://reviews.llvm.org/D45124



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

[PATCH] D49119: [Sema][ObjC] Issue a warning when a method declared in a protocol is non-escaping but the corresponding method in the implementation is escaping.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Overall looks good to me. Maybe add a test when a protocol is declared for an 
interface, not for a category. Something like

  __attribute__((objc_root_class))
  @interface C4 
  -(void) m0:(int*) p; // expected-warning {{parameter of overriding method 
should be annotated with __attribute__((noescape))}}
  @end
  
  @implementation C4
  -(void) m0:(int*) p {
  }
  @end

Looks like there is no such test already and it seems different enough to be 
worth adding. The idea for this test was triggered by code `for (auto *C : 
IDecl->visible_categories())` and the goal is to cover a case when protocol is 
not for a category.

Also I had a few ideas for tests when the warning isn't required and it is 
absent. But I'm not sure they are actually valuable. If you are interested, we 
can discuss it in more details.

Another feedback is an idea for potential improvement: have a note pointing to 
the place where protocol conformity is declared. Currently the warning looks 
like

  code_review.m:16:19: warning: parameter of overriding method should be 
annotated with __attribute__((noescape))
[-Wmissing-noescape]
  -(void) m0:(int*) p { // expected-warning {{parameter of overriding method 
should be annotated with __attri...
^
  code_review.m:2:44: note: parameter of overridden method is annotated with 
__attribute__((noescape))
  -(void) m0:(int*)__attribute__((noescape)) p; // expected-note {{parameter of 
overridden method is annotate...
 ^

and we can see the method both in implementation and in protocol. But in some 
cases it might be unclear where exactly that protocol was added to your class. 
I'm not sure this change is sufficiently useful, it's more for discussion.


Repository:
  rC Clang

https://reviews.llvm.org/D49119



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

[PATCH] D49330: [compiler-rt] Include -lm when using compiler-rt, due to dependencies in some __div methods.

[Jordan Rupprecht via Phabricator via cfe-commits]

rupprecht added a comment.

Thanks Eli, I'll see how hard it is to remove those calls to logb/logbf (those 
seem to be the only ones causing link errors) and revert this change if that's 
possible.


Repository:
  rC Clang

https://reviews.llvm.org/D49330



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

[PATCH] D48395: Added PublicOnly flag

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added inline comments.



Comment at: clang-tools-extra/test/clang-doc/yaml-module.cpp:14
+// CHECK-A: ---
+// CHECK-A-NEXT: USR: 
'{{[0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z]}}'
+// CHECK-A-NEXT: Name:'moduleFunction'

ioeric wrote:
> This could be `[0-9A-Z]{N}` where N = length(USR), right?
The `{n}` syntax doesn't work with FileCheck. 


https://reviews.llvm.org/D48395



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

[PATCH] D49403: More aggressively complete RecordTypes with Function Pointers

[Erich Keane via Phabricator via cfe-commits]

erichkeane created this revision.
erichkeane added reviewers: majnemer, rjmccall.

I discovered that function pointers inside a RecordType that had
its type-determination done in a function whose signature matched
said function pointer resulted in the function pointer type being
emitted empty.  This resulted in information being lost that is
interesting to certain optimizations.

See: https://godbolt.org/g/EegViY

This patch accomplishes this in 2 different situations:
1- When the function itself is being emitted, first convert all

  the return/parameter types to ensure that they are available when
  completing the function type.  This should not result in any additional
  work besides completing parameter types that previously were not completed.

2- When the function type is being evaluated, defer conversion of the record

  type until it is no longer dependent. 


https://reviews.llvm.org/D49403

Files:
  lib/CodeGen/CGCall.cpp
  lib/CodeGen/CodeGenTypes.cpp
  test/CodeGen/func_ptr_recursive_layout.c
  test/CodeGenCXX/pr18962.cpp


Index: lib/CodeGen/CGCall.cpp
===
--- lib/CodeGen/CGCall.cpp
+++ lib/CodeGen/CGCall.cpp
@@ -756,6 +756,13 @@
 
   unsigned CC = ClangCallConvToLLVMCallConv(info.getCC());
 
+  // Prime the type-cache with the types for this function early.  This enables
+  // parameter types that are dependent on this function's type to be properly
+  // emitted.
+  ConvertType(resultType);
+  for (const CanQualType  : argTypes)
+ConvertType(QT);
+
   // Construct the function info.  We co-allocate the ArgInfos.
   FI = CGFunctionInfo::create(CC, instanceMethod, chainCall, info,
   paramInfos, resultType, argTypes, required);
Index: lib/CodeGen/CodeGenTypes.cpp
===
--- lib/CodeGen/CodeGenTypes.cpp
+++ lib/CodeGen/CodeGenTypes.cpp
@@ -169,6 +169,14 @@
   if (const auto *AT = CGT.getContext().getAsArrayType(T))
 return isSafeToConvert(AT->getElementType(), CGT, AlreadyChecked);
 
+  // If this type has a function pointer that is in the process of being laid
+  // out, delay emitting this RecordDecl until that function type is finished.
+  if (T->isFunctionPointerType())
+return isSafeToConvert(T->getPointeeType(), CGT, AlreadyChecked);
+
+  if (T->isFunctionType())
+return !CGT.isRecordBeingLaidOut(T.getCanonicalType().getTypePtr());
+
   // Otherwise, there is no concern about transforming this.  We only care 
about
   // things that are contained by-value in a structure that can have another 
   // structure as a member.
Index: test/CodeGen/func_ptr_recursive_layout.c
===
--- test/CodeGen/func_ptr_recursive_layout.c
+++ test/CodeGen/func_ptr_recursive_layout.c
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple x86_64-linux-pc -emit-llvm -o - %s | FileCheck %s
+
+struct has_fp;
+typedef void (*fp)(const struct has_fp *f);
+struct has_fp {
+  fp i;
+};
+void func(const struct has_fp *f);
+
+void usage() {
+  (void)func;
+}
+
+struct has_fp2;
+typedef void (*fp2)(const struct has_fp2 *f);
+struct has_fp2 {
+  fp2 i;
+};
+void func2(const struct has_fp2 *f) {}
+
+// CHECK-DAG: %struct.has_fp = type { void (%struct.has_fp*)* }
+// CHECK-DAG: %struct.has_fp2 = type { void (%struct.has_fp2*)* }
+
Index: test/CodeGenCXX/pr18962.cpp
===
--- test/CodeGenCXX/pr18962.cpp
+++ test/CodeGenCXX/pr18962.cpp
@@ -25,7 +25,7 @@
 }
 
 // We end up using an opaque type for 'append' to avoid circular references.
-// CHECK: %class.A = type { {}* }
+// CHECK: %class.A = type { void (%class.A*)* }
 // CHECK: %class.C = type <{ %class.D*, %class.B, [3 x i8] }>
 // CHECK: %class.D = type { %class.C.base, [3 x i8] }
 // CHECK: %class.C.base = type <{ %class.D*, %class.B }>


Index: lib/CodeGen/CGCall.cpp
===
--- lib/CodeGen/CGCall.cpp
+++ lib/CodeGen/CGCall.cpp
@@ -756,6 +756,13 @@
 
   unsigned CC = ClangCallConvToLLVMCallConv(info.getCC());
 
+  // Prime the type-cache with the types for this function early.  This enables
+  // parameter types that are dependent on this function's type to be properly
+  // emitted.
+  ConvertType(resultType);
+  for (const CanQualType  : argTypes)
+ConvertType(QT);
+
   // Construct the function info.  We co-allocate the ArgInfos.
   FI = CGFunctionInfo::create(CC, instanceMethod, chainCall, info,
   paramInfos, resultType, argTypes, required);
Index: lib/CodeGen/CodeGenTypes.cpp
===
--- lib/CodeGen/CodeGenTypes.cpp
+++ lib/CodeGen/CodeGenTypes.cpp
@@ -169,6 +169,14 @@
   if (const auto *AT = CGT.getContext().getAsArrayType(T))
 return isSafeToConvert(AT->getElementType(), CGT, AlreadyChecked);
 
+  // If this type has a function 

[PATCH] D47894: [clang]: Add support for "-fno-delete-null-pointer-checks"

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

In https://reviews.llvm.org/D47894#1158811, @manojgupta wrote:

> @efriedma @jyknight Does the change match your expectations where warnings 
> are still generated but codeGen does not emit nonnull attribute?


Yes, this seems sensible IMO.




Comment at: include/clang/Driver/Options.td:1080
+def fdelete_null_pointer_checks : Flag<["-"],
+  "fdelete-null-pointer-checks">, Group;
+def fno_delete_null_pointer_checks : Flag<["-"],

Do we need help text for this one too?



Comment at: test/CodeGen/nonnull.c:1-2
-// RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm < %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm < %s | FileCheck 
-check-prefix=PREFIX-NONNULL %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm 
-fno-delete-null-pointer-checks < %s | FileCheck -check-prefix=PREFIX-NO-NULL %s
 

These prefixes are very confusingly named. NONNULL and NO-NULL sound like the 
same thing.
I'd suggest using, across all the files here,
CHECK:
NULL-INVALID:
NULL-VALID:

That'd also avoid the need to rename all the "CHECK" lines to something else, 
which is most of the diff in these files.



Comment at: test/Sema/nonnull.c:2
 // RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -fno-delete-null-pointer-checks -verify %s
 // rdar://9584012

manojgupta wrote:
> All warnings are still issued if nullptr is passed to function nonnull 
> attribute.
SGTM, but this should be a comment in the file, not the code review.


Repository:
  rC Clang

https://reviews.llvm.org/D47894



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

[PATCH] D41458: [libc++][C++17] Elementary string conversions for integral types

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added inline comments.



Comment at: include/charconv:244
+static _LIBCPP_INLINE_VISIBILITY char const*
+read(char const* __p, char const* __ep, type& __a, type& __b)
+{

mclow.lists wrote:
> lichray wrote:
> > mclow.lists wrote:
> > > Same comment as above about `read` and `inner_product` - they need to be 
> > > "ugly names"
> > Unlike `traits` which is a template parameter name in the standard, `read` 
> > and `inner_product` are function names in the standard, which means the 
> > users cannot make a macro for them (and there is no guarantee about what 
> > name you make **not** get by including certain headers), so we don't need 
> > to use ugly names here, am I right?
> I understand your reasoning, but I don't agree. 
> 
> Just last month, I had to rename a function in `vector` from `allocate` to 
> `__vallocate` because it confused our "is this an allocator" detection. The 
> function in question was private, so it shouldn't have mattered, but GCC has 
> a bug where sometimes it partially ignores access restrictions in non-deduced 
> contexts, and then throws a hard error when it comes back to a different 
> context. The easiest workaround was to rename the function in `vector`.
> 
> Since then, I've been leery of public names that match others. This is pretty 
> obscure, since it's in a private namespace, but I'd feel better if they were 
> `__read` and `__inner_product`.
> 
FWIW, +1 to ugly names. Even if the un-ugly code is "technically not broken 
yet", and besides the technical reason Marshall gives,
(1) it's nice that libc++ has style rules and sticks to them, precisely to 
*avoid* bikeshedding the name of every private member in the world;
(2) just because users can't `#define read write` doesn't mean they *won't* do 
it. I would actually be extremely surprised if `read` were *not* defined as a 
macro somewhere inside ``. :)

See also: "should this function call be `_VSTD::`-qualified?" Sometimes the 
answer is technically "no", but stylistically "yes", precisely to indicate that 
we *don't* intend for it to be an ADL customization point. Consistent style 
leads to maintainability.


Repository:
  rCXX libc++

https://reviews.llvm.org/D41458



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

[PATCH] D47474: Implement cpu_dispatch/cpu_specific Multiversioning

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Bump!  Would love to have someone take a look!


https://reviews.llvm.org/D47474



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

[PATCH] D49265: [Tooling] Add operator== to CompileCommand

[David Blaikie via Phabricator via cfe-commits]

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

In theory you'd need separate tests for op== and op!= returning false 
(currently all the tests would pass if both implementations returned true in 
all cases), but not the biggest deal.


Repository:
  rC Clang

https://reviews.llvm.org/D49265



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

[PATCH] D49387: [analyzer] Make checkEndFunction() give access to the return statement

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL337215: [analyzer] Make checkEndFunction() give access to 
the return statement. (authored by rkovacs, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D49387?vs=155713=155757#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D49387

Files:
  cfe/trunk/include/clang/StaticAnalyzer/Core/Checker.h
  cfe/trunk/include/clang/StaticAnalyzer/Core/CheckerManager.h
  cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
  cfe/trunk/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp
  cfe/trunk/lib/StaticAnalyzer/Checkers/MisusedMovedObjectChecker.cpp
  cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
  cfe/trunk/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
  cfe/trunk/lib/StaticAnalyzer/Checkers/TestAfterDivZeroChecker.cpp
  cfe/trunk/lib/StaticAnalyzer/Checkers/TraversalChecker.cpp
  cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp
  cfe/trunk/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/CheckerManager.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp

Index: cfe/trunk/lib/StaticAnalyzer/Core/CheckerManager.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/CheckerManager.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/CheckerManager.cpp
@@ -439,7 +439,8 @@
 void CheckerManager::runCheckersForEndFunction(NodeBuilderContext ,
ExplodedNodeSet ,
ExplodedNode *Pred,
-   ExprEngine ) {
+   ExprEngine ,
+   const ReturnStmt *RS) {
   // We define the builder outside of the loop bacause if at least one checkers
   // creates a sucsessor for Pred, we do not need to generate an
   // autotransition for it.
@@ -449,7 +450,7 @@
   Pred->getLocationContext(),
   checkFn.Checker);
 CheckerContext C(Bldr, Eng, Pred, L);
-checkFn(C);
+checkFn(RS, C);
   }
 }
 
Index: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -2297,9 +2297,9 @@
 
 // Notify checkers.
 for (const auto I : AfterRemovedDead)
-  getCheckerManager().runCheckersForEndFunction(BC, Dst, I, *this);
+  getCheckerManager().runCheckersForEndFunction(BC, Dst, I, *this, RS);
   } else {
-getCheckerManager().runCheckersForEndFunction(BC, Dst, Pred, *this);
+getCheckerManager().runCheckersForEndFunction(BC, Dst, Pred, *this, RS);
   }
 
   Engine.enqueueEndOfFunction(Dst, RS);
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/TraversalChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/TraversalChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/TraversalChecker.cpp
@@ -30,7 +30,7 @@
 public:
   void checkBranchCondition(const Stmt *Condition, CheckerContext ) const;
   void checkBeginFunction(CheckerContext ) const;
-  void checkEndFunction(CheckerContext ) const;
+  void checkEndFunction(const ReturnStmt *RS, CheckerContext ) const;
 };
 }
 
@@ -56,7 +56,8 @@
   llvm::outs() << "--BEGIN FUNCTION--\n";
 }
 
-void TraversalDumper::checkEndFunction(CheckerContext ) const {
+void TraversalDumper::checkEndFunction(const ReturnStmt *RS,
+   CheckerContext ) const {
   llvm::outs() << "--END FUNCTION--\n";
 }
 
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
@@ -2743,7 +2743,7 @@
 
   void checkDeadSymbols(SymbolReaper , CheckerContext ) const;
   void checkBeginFunction(CheckerContext ) const;
-  void checkEndFunction(CheckerContext ) const;
+  void checkEndFunction(const ReturnStmt *RS, CheckerContext ) const;
 
   ProgramStateRef updateSymbol(ProgramStateRef state, SymbolRef sym,
RefVal V, ArgEffect E, RefVal::Kind ,
@@ -3991,7 +3991,8 @@
   Ctx.addTransition(state);
 }
 
-void RetainCountChecker::checkEndFunction(CheckerContext ) const {
+void RetainCountChecker::checkEndFunction(const ReturnStmt *RS,
+  CheckerContext ) const {
   ProgramStateRef state = Ctx.getState();
   RefBindingsTy B = state->get();
   ExplodedNode *Pred = Ctx.getPredecessor();
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp

r337215 - [analyzer] Make checkEndFunction() give access to the return statement.

[Reka Kovacs via cfe-commits]

Author: rkovacs
Date: Mon Jul 16 13:47:45 2018
New Revision: 337215

URL: http://llvm.org/viewvc/llvm-project?rev=337215=rev
Log:
[analyzer] Make checkEndFunction() give access to the return statement.

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

Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/Checker.h
cfe/trunk/include/clang/StaticAnalyzer/Core/CheckerManager.h
cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/MisusedMovedObjectChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/TestAfterDivZeroChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/TraversalChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Core/CheckerManager.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/Checker.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/Checker.h?rev=337215=337214=337215=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/Checker.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/Checker.h Mon Jul 16 13:47:45 
2018
@@ -254,9 +254,9 @@ public:
 
 class EndFunction {
   template 
-  static void _checkEndFunction(void *checker,
+  static void _checkEndFunction(void *checker, const ReturnStmt *RS,
 CheckerContext ) {
-((const CHECKER *)checker)->checkEndFunction(C);
+((const CHECKER *)checker)->checkEndFunction(RS, C);
   }
 
 public:

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/CheckerManager.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/CheckerManager.h?rev=337215=337214=337215=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/CheckerManager.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/CheckerManager.h Mon Jul 16 
13:47:45 2018
@@ -296,7 +296,8 @@ public:
   void runCheckersForEndFunction(NodeBuilderContext ,
  ExplodedNodeSet ,
  ExplodedNode *Pred,
- ExprEngine );
+ ExprEngine ,
+ const ReturnStmt *RS);
 
   /// Run checkers for branch condition.
   void runCheckersForBranchCondition(const Stmt *condition,
@@ -438,7 +439,8 @@ public:
 
   using CheckBeginFunctionFunc = CheckerFn;
 
-  using CheckEndFunctionFunc = CheckerFn;
+  using CheckEndFunctionFunc =
+  CheckerFn;
   
   using CheckBranchConditionFunc =
   CheckerFn;
@@ -496,7 +498,7 @@ public:
 
   void _registerForEndAnalysis(CheckEndAnalysisFunc checkfn);
 
-  void _registerForBeginFunction(CheckEndFunctionFunc checkfn);
+  void _registerForBeginFunction(CheckBeginFunctionFunc checkfn);
   void _registerForEndFunction(CheckEndFunctionFunc checkfn);
 
   void _registerForBranchCondition(CheckBranchConditionFunc checkfn);

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp?rev=337215=337214=337215=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp Mon Jul 16 
13:47:45 2018
@@ -126,7 +126,7 @@ public:
  const CallEvent *Call,
  PointerEscapeKind Kind) const;
   void checkPreStmt(const ReturnStmt *RS, CheckerContext ) const;
-  void checkEndFunction(CheckerContext ) const;
+  void checkEndFunction(const ReturnStmt *RS, CheckerContext ) const;
 
 private:
   void diagnoseMissingReleases(CheckerContext ) const;
@@ -398,7 +398,7 @@ void ObjCDeallocChecker::checkPostObjCMe
 /// Check for missing releases even when -dealloc does not call
 /// '[super dealloc]'.
 void ObjCDeallocChecker::checkEndFunction(
-CheckerContext ) const {
+const ReturnStmt *RS, CheckerContext ) const {
   diagnoseMissingReleases(C);
 }
 

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp?rev=337215=337214=337215=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp Mon Jul 16 

[PATCH] D48560: [clangd] JSON <-> XPC conversions

[Jan Korous via Phabricator via cfe-commits]

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

Hi Sam, no worries! Thanks for making time to take a look! I am going to rebase 
all my patches on top of JSON library moved to llvm/Support and process your 
comments.




Comment at: clangd/xpc/CMakeLists.txt:1
+set(LLVM_LINK_COMPONENTS
+  support

sammccall wrote:
> I think it'd be helpful for some of us confused linux people to have a brief 
> README in this directory giving some context about XPC in clangd.
> 
> Anything you want to write about the architecture is great but the essentials 
> would be:
>  - alternative transport layer to JSON-RPC
>  - semantically equivalent, uses same LSP request/response message types (?)
>  - MacOS only. Feature is guarded by `CLANGD_BUILD_XPC_SUPPORT`, including 
> whole `xpc/` dir.
That's a good idea, thanks.



Comment at: clangd/xpc/CMakeLists.txt:15
+
+# FIXME: Factor out json::Expr from clangDaemon
+target_link_libraries(ClangdXpcTests

sammccall wrote:
> this is done :-)
Yes, I know - thanks for the good work with lifting the JSON library! I am 
going to rebase my patches.



Comment at: xpc/XPCJSONConversions.cpp:22
+case json::Expr::Boolean: return 
xpc_bool_create(json.asBoolean().getValue());
+case json::Expr::Number:  return 
xpc_double_create(json.asNumber().getValue());
+case json::Expr::String:  return 
xpc_string_create(json.asString().getValue().str().c_str());

sammccall wrote:
> The underlying JSON library actually has both `int64_t` and `double` 
> representations, and I see XPC has both as well.
> If `double` works for all the data sent over this channel, then this approach 
> is simplest. But for completeness, this is also possible:
> ```
> if (auto I = json.asInteger())
>   return xpc_int64_create(*I);
> return xpc_double_create(*json.asNumber());
> ```
> 
> (I don't know of any reason clangd would use large integers today unless they 
> arrived as incoming JSON-RPC request IDs or similar)
I was initially thinking along the same line but got stuck on the fact that 
JSON standard (modelled by llvm::json::Value) doesn't distinguish numeric types 
(it only has Number).

Relevant part of llvm::json::Value interface is:
```
llvm::Optional getAsNumber() const {
if (LLVM_LIKELY(Type == T_Double))
  return as();
if (LLVM_LIKELY(Type == T_Integer))
  return as();
return llvm::None;
  }
  // Succeeds if the Value is a Number, and exactly representable as int64_t.
  llvm::Optional getAsInteger() const {
if (LLVM_LIKELY(Type == T_Integer))
  return as();
if (LLVM_LIKELY(Type == T_Double)) {
  double D = as();
  if (LLVM_LIKELY(std::modf(D, ) == 0.0 &&
  D >= double(std::numeric_limits::min()) &&
  D <= double(std::numeric_limits::max(
return D;
}
return llvm::None;
  }
```
while both
```enum ValueType```
and
```template T () const```
are private which makes sense since those are implementation details. 

Basically it seems to me there's no reliable way how to tell if Value is double 
or not (my example is 2.0) which means we couldn't preserve the "static" 
numeric type (it would be value dependent). I don't think changing the JSON 
library because of this would be a good idea. It's not a big deal for us and 
it's possible we'll skip JSON completely in XPC transport layer in the future.



Comment at: xpc/XPCJSONConversions.cpp:40
+  }
+  // Get pointers only now so there's no possible re-allocation of keys.
+  for(const auto& k : keys)

sammccall wrote:
> (you could also pre-size the vector, or use a deque)
Ok, I'll preallocate necessary space. I remember Bjarne Stroustrup saying he 
doesn't bother with reserve() anymore but it (most probably) won't do any harm 
here.

http://www.stroustrup.com/bs_faq2.html#slow-containers
"People sometimes worry about the cost of std::vector growing incrementally. I 
used to worry about that and used reserve() to optimize the growth. After 
measuring my code and repeatedly having trouble finding the performance 
benefits of reserve() in real programs, I stopped using it except where it is 
needed to avoid iterator invalidation (a rare case in my code). Again: measure 
before you optimize."



Comment at: xpc/XPCJSONConversions.cpp:69
+  xpcObj,
+  ^bool(size_t /* index */, xpc_object_t value) {
+result.emplace_back(xpcToJson(value));

sammccall wrote:
> this looks like objC syntax, I'm not familiar with it, but should this file 
> be `.mm`?
> 
> Alternatively, it seems like you could write a C++ loop with 
> `xpc_array_get_value` (though I don't know if there are performance concerns).
> 
> (and dictionary)
Oh, sorry. These are actually C blocks - a nonstandard C extension.
https://en.wikipedia.org/wiki/Blocks_(C_language_extension)

r337214 - [ASTMatchers] Quickfix for tests.

[George Karpenkov via cfe-commits]

Author: george.karpenkov
Date: Mon Jul 16 13:42:37 2018
New Revision: 337214

URL: http://llvm.org/viewvc/llvm-project?rev=337214=rev
Log:
[ASTMatchers] Quickfix for tests.

Modified:
cfe/trunk/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Modified: cfe/trunk/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp?rev=337214=337213=337214=diff
==
--- cfe/trunk/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp (original)
+++ cfe/trunk/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp Mon Jul 16 
13:42:37 2018
@@ -424,28 +424,28 @@ TEST(Matcher, AnyArgument) {
 
 TEST(Matcher, HasReceiver) {
   EXPECT_TRUE(matchesObjC(
-  "@interface NSString @end"
+  "@interface NSString @end "
   "void f(NSString *x) {"
-  "[x containsString]"
+  "[x containsString];"
   "}",
   objcMessageExpr(hasReceiver(declRefExpr(to(varDecl(hasName("x";
 
   EXPECT_FALSE(matchesObjC(
-  "@interface NSString +(NSString *) stringWithFormat; @end"
+  "@interface NSString +(NSString *) stringWithFormat; @end "
   "void f() { [NSString stringWithFormat]; }",
   objcMessageExpr(hasReceiver(declRefExpr(to(varDecl(hasName("x";
 }
 
 TEST(Matcher, isInstanceMessage) {
   EXPECT_TRUE(matchesObjC(
-  "@interface NSString @end"
+  "@interface NSString @end "
   "void f(NSString *x) {"
-  "[x containsString]"
+  "[x containsString];"
   "}",
   objcMessageExpr(isInstanceMessage(;
 
   EXPECT_FALSE(matchesObjC(
-  "@interface NSString +(NSString *) stringWithFormat; @end"
+  "@interface NSString +(NSString *) stringWithFormat; @end "
   "void f() { [NSString stringWithFormat]; }",
   objcMessageExpr(isInstanceMessage(;
 


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

[PATCH] D49265: [Tooling] Add operator== to CompileCommand

[Simon Marchi via Phabricator via cfe-commits]

simark updated this revision to Diff 155753.
simark added a comment.

- Add test
- Make operator== a function instead of method
- Add operator!= (so I can use EXPECT_NE in the test, and because it may be 
useful in general)


Repository:
  rC Clang

https://reviews.llvm.org/D49265

Files:
  include/clang/Tooling/CompilationDatabase.h
  unittests/Tooling/CompilationDatabaseTest.cpp


Index: unittests/Tooling/CompilationDatabaseTest.cpp
===
--- unittests/Tooling/CompilationDatabaseTest.cpp
+++ unittests/Tooling/CompilationDatabaseTest.cpp
@@ -736,5 +736,28 @@
   EXPECT_EQ(getCommand("foo/bar/baz/shout.C"), "clang -D 
FOO/BAR/BAZ/SHOUT.cc");
 }
 
+TEST(CompileCommandTest, EqualityOperator) {
+  CompileCommand CCRef("/foo/bar", "hello.c", {"a", "b"}, "hello.o");
+  CompileCommand CCTest = CCRef;
+
+  EXPECT_EQ(CCRef, CCTest);
+
+  CCTest = CCRef;
+  CCTest.Directory = "/foo/baz";
+  EXPECT_NE(CCRef, CCTest);
+
+  CCTest = CCRef;
+  CCTest.Filename = "bonjour.c";
+  EXPECT_NE(CCRef, CCTest);
+
+  CCTest = CCRef;
+  CCTest.CommandLine.push_back("c");
+  EXPECT_NE(CCRef, CCTest);
+
+  CCTest = CCRef;
+  CCTest.Output = "bonjour.o";
+  EXPECT_NE(CCRef, CCTest);
+}
+
 } // end namespace tooling
 } // end namespace clang
Index: include/clang/Tooling/CompilationDatabase.h
===
--- include/clang/Tooling/CompilationDatabase.h
+++ include/clang/Tooling/CompilationDatabase.h
@@ -59,6 +59,15 @@
 
   /// The output file associated with the command.
   std::string Output;
+
+  friend bool operator==(const CompileCommand , const CompileCommand ) 
{
+return LHS.Directory == RHS.Directory && LHS.Filename == RHS.Filename &&
+   LHS.CommandLine == RHS.CommandLine && LHS.Output == RHS.Output;
+  }
+
+  friend bool operator!=(const CompileCommand , const CompileCommand ) 
{
+return !(LHS == RHS);
+  }
 };
 
 /// Interface for compilation databases.


Index: unittests/Tooling/CompilationDatabaseTest.cpp
===
--- unittests/Tooling/CompilationDatabaseTest.cpp
+++ unittests/Tooling/CompilationDatabaseTest.cpp
@@ -736,5 +736,28 @@
   EXPECT_EQ(getCommand("foo/bar/baz/shout.C"), "clang -D FOO/BAR/BAZ/SHOUT.cc");
 }
 
+TEST(CompileCommandTest, EqualityOperator) {
+  CompileCommand CCRef("/foo/bar", "hello.c", {"a", "b"}, "hello.o");
+  CompileCommand CCTest = CCRef;
+
+  EXPECT_EQ(CCRef, CCTest);
+
+  CCTest = CCRef;
+  CCTest.Directory = "/foo/baz";
+  EXPECT_NE(CCRef, CCTest);
+
+  CCTest = CCRef;
+  CCTest.Filename = "bonjour.c";
+  EXPECT_NE(CCRef, CCTest);
+
+  CCTest = CCRef;
+  CCTest.CommandLine.push_back("c");
+  EXPECT_NE(CCRef, CCTest);
+
+  CCTest = CCRef;
+  CCTest.Output = "bonjour.o";
+  EXPECT_NE(CCRef, CCTest);
+}
+
 } // end namespace tooling
 } // end namespace clang
Index: include/clang/Tooling/CompilationDatabase.h
===
--- include/clang/Tooling/CompilationDatabase.h
+++ include/clang/Tooling/CompilationDatabase.h
@@ -59,6 +59,15 @@
 
   /// The output file associated with the command.
   std::string Output;
+
+  friend bool operator==(const CompileCommand , const CompileCommand ) {
+return LHS.Directory == RHS.Directory && LHS.Filename == RHS.Filename &&
+   LHS.CommandLine == RHS.CommandLine && LHS.Output == RHS.Output;
+  }
+
+  friend bool operator!=(const CompileCommand , const CompileCommand ) {
+return !(LHS == RHS);
+  }
 };
 
 /// Interface for compilation databases.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48856: [analyzer] Fix overly eager suppression of NPE when the value used is returned from a macro

[George Karpenkov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC337213: [analyzer] Bugfix for an overly eager suppression 
for null pointer return from… (authored by george.karpenkov, committed by ).
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D48856?vs=153815=155752#toc

Repository:
  rC Clang

https://reviews.llvm.org/D48856

Files:
  lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  test/Analysis/diagnostics/macro-null-return-suppression.cpp

Index: test/Analysis/diagnostics/macro-null-return-suppression.cpp
===
--- test/Analysis/diagnostics/macro-null-return-suppression.cpp
+++ test/Analysis/diagnostics/macro-null-return-suppression.cpp
@@ -43,3 +43,26 @@
 DEREF_IN_MACRO(0) // expected-warning{{Dereference of null pointer}}
   // expected-note@-1{{'p' initialized to a null}}
   // expected-note@-2{{Dereference of null pointer}}
+
+// Warning should not be suppressed if the null returned by the macro
+// is not related to the warning.
+#define RETURN_NULL() (0)
+extern int* returnFreshPointer();
+int noSuppressMacroUnrelated() {
+  int *x = RETURN_NULL();
+  x = returnFreshPointer();  // expected-note{{Value assigned to 'x'}}
+  if (x) {} // expected-note{{Taking false branch}}
+// expected-note@-1{{Assuming 'x' is null}}
+  return *x; // expected-warning{{Dereference of null pointer}}
+ // expected-note@-1{{Dereference}}
+}
+
+// Value haven't changed by the assignment, but the null pointer
+// did not come from the macro.
+int noSuppressMacroUnrelatedOtherReason() {
+  int *x = RETURN_NULL();
+  x = returnFreshPointer();  
+  x = 0; // expected-note{{Null pointer value stored to 'x'}}
+  return *x; // expected-warning{{Dereference of null pointer}}
+ // expected-note@-1{{Dereference}}
+}
Index: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -228,6 +228,38 @@
   return EInfo.isFunctionMacroExpansion();
 }
 
+/// \return Whether \c RegionOfInterest was modified at \p N,
+/// where \p ReturnState is a state associated with the return
+/// from the current frame.
+static bool wasRegionOfInterestModifiedAt(
+const SubRegion *RegionOfInterest,
+const ExplodedNode *N,
+SVal ValueAfter) {
+  ProgramStateRef State = N->getState();
+  ProgramStateManager  = N->getState()->getStateManager();
+
+  if (!N->getLocationAs()
+  && !N->getLocationAs()
+  && !N->getLocationAs())
+return false;
+
+  // Writing into region of interest.
+  if (auto PS = N->getLocationAs())
+if (auto *BO = PS->getStmtAs())
+  if (BO->isAssignmentOp() && RegionOfInterest->isSubRegionOf(
+N->getSVal(BO->getLHS()).getAsRegion()))
+return true;
+
+  // SVal after the state is possibly different.
+  SVal ValueAtN = N->getState()->getSVal(RegionOfInterest);
+  if (!Mgr.getSValBuilder().areEqual(State, ValueAtN, ValueAfter).isConstrainedTrue() &&
+  (!ValueAtN.isUndef() || !ValueAfter.isUndef()))
+return true;
+
+  return false;
+}
+
+
 namespace {
 
 /// Put a diagnostic on return statement of all inlined functions
@@ -346,7 +378,7 @@
   FramesModifyingCalculated.insert(
 N->getLocationContext()->getStackFrame());
 
-  if (wasRegionOfInterestModifiedAt(N, LastReturnState, ValueAtReturn)) {
+  if (wasRegionOfInterestModifiedAt(RegionOfInterest, N, ValueAtReturn)) {
 const StackFrameContext *SCtx = N->getStackFrame();
 while (!SCtx->inTopFrame()) {
   auto p = FramesModifyingRegion.insert(SCtx);
@@ -365,33 +397,6 @@
 } while (N);
   }
 
-  /// \return Whether \c RegionOfInterest was modified at \p N,
-  /// where \p ReturnState is a state associated with the return
-  /// from the current frame.
-  bool wasRegionOfInterestModifiedAt(const ExplodedNode *N,
- ProgramStateRef ReturnState,
- SVal ValueAtReturn) {
-if (!N->getLocationAs()
-&& !N->getLocationAs()
-&& !N->getLocationAs())
-  return false;
-
-// Writing into region of interest.
-if (auto PS = N->getLocationAs())
-  if (auto *BO = PS->getStmtAs())
-if (BO->isAssignmentOp() && RegionOfInterest->isSubRegionOf(
-N->getSVal(BO->getLHS()).getAsRegion()))
-  return true;
-
-// SVal after the state is possibly different.
-SVal ValueAtN = N->getState()->getSVal(RegionOfInterest);
-if (!ReturnState->areEqual(ValueAtN, ValueAtReturn).isConstrainedTrue() &&
-(!ValueAtN.isUndef() || !ValueAtReturn.isUndef()))
-  return true;
-
-return false;
-  }
-
   /// Get parameters associated with runtime definition in order
   

r337213 - [analyzer] Bugfix for an overly eager suppression for null pointer return from macros.

[George Karpenkov via cfe-commits]

Author: george.karpenkov
Date: Mon Jul 16 13:33:25 2018
New Revision: 337213

URL: http://llvm.org/viewvc/llvm-project?rev=337213=rev
Log:
[analyzer] Bugfix for an overly eager suppression for null pointer return from 
macros.

Only suppress those cases where the null which came from the macro is
relevant to the bug, and was not overwritten in between.

rdar://41497323

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

Modified:
cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
cfe/trunk/test/Analysis/diagnostics/macro-null-return-suppression.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp?rev=337213=337212=337213=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Mon Jul 16 
13:33:25 2018
@@ -228,6 +228,38 @@ static bool isFunctionMacroExpansion(Sou
   return EInfo.isFunctionMacroExpansion();
 }
 
+/// \return Whether \c RegionOfInterest was modified at \p N,
+/// where \p ReturnState is a state associated with the return
+/// from the current frame.
+static bool wasRegionOfInterestModifiedAt(
+const SubRegion *RegionOfInterest,
+const ExplodedNode *N,
+SVal ValueAfter) {
+  ProgramStateRef State = N->getState();
+  ProgramStateManager  = N->getState()->getStateManager();
+
+  if (!N->getLocationAs()
+  && !N->getLocationAs()
+  && !N->getLocationAs())
+return false;
+
+  // Writing into region of interest.
+  if (auto PS = N->getLocationAs())
+if (auto *BO = PS->getStmtAs())
+  if (BO->isAssignmentOp() && RegionOfInterest->isSubRegionOf(
+N->getSVal(BO->getLHS()).getAsRegion()))
+return true;
+
+  // SVal after the state is possibly different.
+  SVal ValueAtN = N->getState()->getSVal(RegionOfInterest);
+  if (!Mgr.getSValBuilder().areEqual(State, ValueAtN, 
ValueAfter).isConstrainedTrue() &&
+  (!ValueAtN.isUndef() || !ValueAfter.isUndef()))
+return true;
+
+  return false;
+}
+
+
 namespace {
 
 /// Put a diagnostic on return statement of all inlined functions
@@ -346,7 +378,7 @@ private:
   FramesModifyingCalculated.insert(
 N->getLocationContext()->getStackFrame());
 
-  if (wasRegionOfInterestModifiedAt(N, LastReturnState, ValueAtReturn)) {
+  if (wasRegionOfInterestModifiedAt(RegionOfInterest, N, ValueAtReturn)) {
 const StackFrameContext *SCtx = N->getStackFrame();
 while (!SCtx->inTopFrame()) {
   auto p = FramesModifyingRegion.insert(SCtx);
@@ -365,33 +397,6 @@ private:
 } while (N);
   }
 
-  /// \return Whether \c RegionOfInterest was modified at \p N,
-  /// where \p ReturnState is a state associated with the return
-  /// from the current frame.
-  bool wasRegionOfInterestModifiedAt(const ExplodedNode *N,
- ProgramStateRef ReturnState,
- SVal ValueAtReturn) {
-if (!N->getLocationAs()
-&& !N->getLocationAs()
-&& !N->getLocationAs())
-  return false;
-
-// Writing into region of interest.
-if (auto PS = N->getLocationAs())
-  if (auto *BO = PS->getStmtAs())
-if (BO->isAssignmentOp() && RegionOfInterest->isSubRegionOf(
-
N->getSVal(BO->getLHS()).getAsRegion()))
-  return true;
-
-// SVal after the state is possibly different.
-SVal ValueAtN = N->getState()->getSVal(RegionOfInterest);
-if (!ReturnState->areEqual(ValueAtN, ValueAtReturn).isConstrainedTrue() &&
-(!ValueAtN.isUndef() || !ValueAtReturn.isUndef()))
-  return true;
-
-return false;
-  }
-
   /// Get parameters associated with runtime definition in order
   /// to get the correct parameter name.
   ArrayRef getCallParameters(CallEventRef<> Call) {
@@ -524,25 +529,28 @@ private:
   }
 };
 
+/// Suppress null-pointer-dereference bugs where dereferenced null was returned
+/// the macro.
 class MacroNullReturnSuppressionVisitor final : public BugReporterVisitor {
   const SubRegion *RegionOfInterest;
+  const SVal ValueAtDereference;
 
-public:
-  MacroNullReturnSuppressionVisitor(const SubRegion *R) : RegionOfInterest(R) 
{}
-
-  static void *getTag() {
-static int Tag = 0;
-return static_cast();
-  }
+  // Do not invalidate the reports where the value was modified
+  // after it got assigned to from the macro.
+  bool WasModified = false;
 
-  void Profile(llvm::FoldingSetNodeID ) const override {
-ID.AddPointer(getTag());
-  }
+public:
+  MacroNullReturnSuppressionVisitor(const SubRegion *R,
+const SVal V) : RegionOfInterest(R),
+ValueAtDereference(V) {}
 
   std::shared_ptr VisitNode(const ExplodedNode *N,

[PATCH] D48911: [analyzer] Fix GCDAntipatternChecker to only fire when the semaphore is initialized to zero

[George Karpenkov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC337212: [analyzer] Fix GCDAntipatternChecker to only fire 
when the semaphore is… (authored by george.karpenkov, committed by ).
Herald added a subscriber: cfe-commits.

Repository:
  rC Clang

https://reviews.llvm.org/D48911

Files:
  lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp
  test/Analysis/gcdantipatternchecker_test.m


Index: test/Analysis/gcdantipatternchecker_test.m
===
--- test/Analysis/gcdantipatternchecker_test.m
+++ test/Analysis/gcdantipatternchecker_test.m
@@ -333,3 +333,13 @@
   }];
   dispatch_semaphore_wait(sema1, 100); // expected-warning{{Waiting on a 
callback using a semaphore}}
 }
+
+void no_warn_on_nonzero_semaphore(MyInterface1 *M) {
+  dispatch_semaphore_t sema1 = dispatch_semaphore_create(1);
+
+  [M acceptBlock:^{
+  dispatch_semaphore_signal(sema1);
+  }];
+  dispatch_semaphore_wait(sema1, 100); // no-warning
+}
+
Index: lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp
+++ lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp
@@ -93,7 +93,9 @@
 static auto findGCDAntiPatternWithSemaphore() -> decltype(compoundStmt()) {
 
   const char *SemaphoreBinding = "semaphore_name";
-  auto SemaphoreCreateM = callExpr(callsName("dispatch_semaphore_create"));
+  auto SemaphoreCreateM = callExpr(allOf(
+  callsName("dispatch_semaphore_create"),
+  hasArgument(0, ignoringParenCasts(integerLiteral(equals(0));
 
   auto SemaphoreBindingM = anyOf(
   forEachDescendant(


Index: test/Analysis/gcdantipatternchecker_test.m
===
--- test/Analysis/gcdantipatternchecker_test.m
+++ test/Analysis/gcdantipatternchecker_test.m
@@ -333,3 +333,13 @@
   }];
   dispatch_semaphore_wait(sema1, 100); // expected-warning{{Waiting on a callback using a semaphore}}
 }
+
+void no_warn_on_nonzero_semaphore(MyInterface1 *M) {
+  dispatch_semaphore_t sema1 = dispatch_semaphore_create(1);
+
+  [M acceptBlock:^{
+  dispatch_semaphore_signal(sema1);
+  }];
+  dispatch_semaphore_wait(sema1, 100); // no-warning
+}
+
Index: lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp
+++ lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp
@@ -93,7 +93,9 @@
 static auto findGCDAntiPatternWithSemaphore() -> decltype(compoundStmt()) {
 
   const char *SemaphoreBinding = "semaphore_name";
-  auto SemaphoreCreateM = callExpr(callsName("dispatch_semaphore_create"));
+  auto SemaphoreCreateM = callExpr(allOf(
+  callsName("dispatch_semaphore_create"),
+  hasArgument(0, ignoringParenCasts(integerLiteral(equals(0));
 
   auto SemaphoreBindingM = anyOf(
   forEachDescendant(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r337212 - [analyzer] Fix GCDAntipatternChecker to only fire when the semaphore is initialized to zero

[George Karpenkov via cfe-commits]

Author: george.karpenkov
Date: Mon Jul 16 13:32:57 2018
New Revision: 337212

URL: http://llvm.org/viewvc/llvm-project?rev=337212=rev
Log:
[analyzer] Fix GCDAntipatternChecker to only fire when the semaphore is 
initialized to zero

Initializing a semaphore with a different constant most likely signals a 
different intent

rdar://41802552

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

Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp
cfe/trunk/test/Analysis/gcdantipatternchecker_test.m

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp?rev=337212=337211=337212=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp Mon Jul 16 
13:32:57 2018
@@ -93,7 +93,9 @@ static bool isTest(const Decl *D) {
 static auto findGCDAntiPatternWithSemaphore() -> decltype(compoundStmt()) {
 
   const char *SemaphoreBinding = "semaphore_name";
-  auto SemaphoreCreateM = callExpr(callsName("dispatch_semaphore_create"));
+  auto SemaphoreCreateM = callExpr(allOf(
+  callsName("dispatch_semaphore_create"),
+  hasArgument(0, ignoringParenCasts(integerLiteral(equals(0));
 
   auto SemaphoreBindingM = anyOf(
   forEachDescendant(

Modified: cfe/trunk/test/Analysis/gcdantipatternchecker_test.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/gcdantipatternchecker_test.m?rev=337212=337211=337212=diff
==
--- cfe/trunk/test/Analysis/gcdantipatternchecker_test.m (original)
+++ cfe/trunk/test/Analysis/gcdantipatternchecker_test.m Mon Jul 16 13:32:57 
2018
@@ -333,3 +333,13 @@ void dispatch_group_and_semaphore_use(My
   }];
   dispatch_semaphore_wait(sema1, 100); // expected-warning{{Waiting on a 
callback using a semaphore}}
 }
+
+void no_warn_on_nonzero_semaphore(MyInterface1 *M) {
+  dispatch_semaphore_t sema1 = dispatch_semaphore_create(1);
+
+  [M acceptBlock:^{
+  dispatch_semaphore_signal(sema1);
+  }];
+  dispatch_semaphore_wait(sema1, 100); // no-warning
+}
+


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

[PATCH] D49166: [analyzer] Provide a symmetric method for generating a PathDiagnosticLocation from Decl

[George Karpenkov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC337211: [analyzer] Provide a symmetric method for generating 
a PathDiagnosticLocation… (authored by george.karpenkov, committed by ).
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D49166?vs=154917=155750#toc

Repository:
  rC Clang

https://reviews.llvm.org/D49166

Files:
  include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h


Index: include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
===
--- include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
+++ include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
@@ -216,6 +216,15 @@
   static PathDiagnosticLocation createBegin(const Decl *D,
 const SourceManager );
 
+  /// Create a location for the beginning of the declaration.
+  /// The third argument is ignored, useful for generic treatment
+  /// of statements and declarations.
+  static PathDiagnosticLocation
+  createBegin(const Decl *D, const SourceManager ,
+  const LocationOrAnalysisDeclContext LAC) {
+return createBegin(D, SM);
+  }
+
   /// Create a location for the beginning of the statement.
   static PathDiagnosticLocation createBegin(const Stmt *S,
 const SourceManager ,


Index: include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
===
--- include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
+++ include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
@@ -216,6 +216,15 @@
   static PathDiagnosticLocation createBegin(const Decl *D,
 const SourceManager );
 
+  /// Create a location for the beginning of the declaration.
+  /// The third argument is ignored, useful for generic treatment
+  /// of statements and declarations.
+  static PathDiagnosticLocation
+  createBegin(const Decl *D, const SourceManager ,
+  const LocationOrAnalysisDeclContext LAC) {
+return createBegin(D, SM);
+  }
+
   /// Create a location for the beginning of the statement.
   static PathDiagnosticLocation createBegin(const Stmt *S,
 const SourceManager ,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r337211 - [analyzer] Provide a symmetric method for generating a PathDiagnosticLocation from Decl

[George Karpenkov via cfe-commits]

Author: george.karpenkov
Date: Mon Jul 16 13:32:32 2018
New Revision: 337211

URL: http://llvm.org/viewvc/llvm-project?rev=337211=rev
Log:
[analyzer] Provide a symmetric method for generating a PathDiagnosticLocation 
from Decl

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

Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h

Modified: 
cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h?rev=337211=337210=337211=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h 
(original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h 
Mon Jul 16 13:32:32 2018
@@ -216,6 +216,15 @@ public:
   static PathDiagnosticLocation createBegin(const Decl *D,
 const SourceManager );
 
+  /// Create a location for the beginning of the declaration.
+  /// The third argument is ignored, useful for generic treatment
+  /// of statements and declarations.
+  static PathDiagnosticLocation
+  createBegin(const Decl *D, const SourceManager ,
+  const LocationOrAnalysisDeclContext LAC) {
+return createBegin(D, SM);
+  }
+
   /// Create a location for the beginning of the statement.
   static PathDiagnosticLocation createBegin(const Stmt *S,
 const SourceManager ,


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

[PATCH] D41458: [libc++][C++17] Elementary string conversions for integral types

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added inline comments.



Comment at: include/charconv:244
+static _LIBCPP_INLINE_VISIBILITY char const*
+read(char const* __p, char const* __ep, type& __a, type& __b)
+{

lichray wrote:
> mclow.lists wrote:
> > Same comment as above about `read` and `inner_product` - they need to be 
> > "ugly names"
> Unlike `traits` which is a template parameter name in the standard, `read` 
> and `inner_product` are function names in the standard, which means the users 
> cannot make a macro for them (and there is no guarantee about what name you 
> make **not** get by including certain headers), so we don't need to use ugly 
> names here, am I right?
I understand your reasoning, but I don't agree. 

Just last month, I had to rename a function in `vector` from `allocate` to 
`__vallocate` because it confused our "is this an allocator" detection. The 
function in question was private, so it shouldn't have mattered, but GCC has a 
bug where sometimes it partially ignores access restrictions in non-deduced 
contexts, and then throws a hard error when it comes back to a different 
context. The easiest workaround was to rename the function in `vector`.

Since then, I've been leery of public names that match others. This is pretty 
obscure, since it's in a private namespace, but I'd feel better if they were 
`__read` and `__inner_product`.



Repository:
  rCXX libc++

https://reviews.llvm.org/D41458



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

[PATCH] D49333: [ASTMatchers] Introduce Objective-C matchers `hasReceiver` and `isInstanceMessage` for ObjCMessageExpr

[George Karpenkov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC337209: [ASTMatchers] Introduce Objective-C matchers 
`hasReceiver` and… (authored by george.karpenkov, committed by ).
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D49333?vs=155734=155747#toc

Repository:
  rC Clang

https://reviews.llvm.org/D49333

Files:
  docs/LibASTMatchersReference.html
  include/clang/ASTMatchers/ASTMatchers.h
  lib/ASTMatchers/Dynamic/Registry.cpp
  unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -283,6 +283,7 @@
   REGISTER_MATCHER(hasParent);
   REGISTER_MATCHER(hasQualifier);
   REGISTER_MATCHER(hasRangeInit);
+  REGISTER_MATCHER(hasReceiver);
   REGISTER_MATCHER(hasReceiverType);
   REGISTER_MATCHER(hasReplacementType);
   REGISTER_MATCHER(hasReturnValue);
@@ -349,6 +350,7 @@
   REGISTER_MATCHER(isImplicit);
   REGISTER_MATCHER(isExpansionInFileMatching);
   REGISTER_MATCHER(isExpansionInMainFile);
+  REGISTER_MATCHER(isInstanceMessage);
   REGISTER_MATCHER(isInstantiated);
   REGISTER_MATCHER(isExpansionInSystemHeader);
   REGISTER_MATCHER(isInteger);
Index: unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -422,6 +422,35 @@
   EXPECT_TRUE(matches("void x(long) { int y; x(y); }", ImplicitCastedArgument));
 }
 
+TEST(Matcher, HasReceiver) {
+  EXPECT_TRUE(matchesObjC(
+  "@interface NSString @end"
+  "void f(NSString *x) {"
+  "[x containsString]"
+  "}",
+  objcMessageExpr(hasReceiver(declRefExpr(to(varDecl(hasName("x";
+
+  EXPECT_FALSE(matchesObjC(
+  "@interface NSString +(NSString *) stringWithFormat; @end"
+  "void f() { [NSString stringWithFormat]; }",
+  objcMessageExpr(hasReceiver(declRefExpr(to(varDecl(hasName("x";
+}
+
+TEST(Matcher, isInstanceMessage) {
+  EXPECT_TRUE(matchesObjC(
+  "@interface NSString @end"
+  "void f(NSString *x) {"
+  "[x containsString]"
+  "}",
+  objcMessageExpr(isInstanceMessage(;
+
+  EXPECT_FALSE(matchesObjC(
+  "@interface NSString +(NSString *) stringWithFormat; @end"
+  "void f() { [NSString stringWithFormat]; }",
+  objcMessageExpr(isInstanceMessage(;
+
+}
+
 TEST(ForEachArgumentWithParam, ReportsNoFalsePositives) {
   StatementMatcher ArgumentY =
 declRefExpr(to(varDecl(hasName("y".bind("arg");
Index: docs/LibASTMatchersReference.html
===
--- docs/LibASTMatchersReference.html
+++ docs/LibASTMatchersReference.html
@@ -3268,6 +3268,19 @@
 
 
 
+Matcherhttp://clang.llvm.org/doxygen/classclang_1_1ObjCMessageExpr.html;>ObjCMessageExprisInstanceMessage
+Returns true when the Objective-C message is sent to an instance.
+
+Example
+matcher = objcMessagaeExpr(isInstanceMessage())
+matches
+  NSString *x = @"hello";
+  [x containsString:@"h"]
+but not
+  [NSString stringWithFormat:@"format"]
+
+
+
 Matcherhttp://clang.llvm.org/doxygen/classclang_1_1ObjCMessageExpr.html;>ObjCMessageExprmatchesSelectorstd::string RegExp
 Matches ObjC selectors whose name contains
 a substring matched by the given RegExp.
@@ -5875,6 +5888,18 @@
 
 
 
+Matcherhttp://clang.llvm.org/doxygen/classclang_1_1ObjCMessageExpr.html;>ObjCMessageExprhasReceiverMatcherhttp://clang.llvm.org/doxygen/classclang_1_1Expr.html;>Expr InnerMatcher
+Matches if the Objective-C message is sent to an instance,
+and the inner matcher matches on that instance.
+
+For example the method call in
+  NSString *x = @"hello";
+  [x containsString:@"h"]
+is matched by
+objcMessageExpr(hasReceiver(declRefExpr(to(varDecl(hasName("x"))
+
+
+
 Matcherhttp://clang.llvm.org/doxygen/classclang_1_1ObjCMessageExpr.html;>ObjCMessageExprhasReceiverTypeMatcherhttp://clang.llvm.org/doxygen/classclang_1_1QualType.html;>QualType InnerMatcher
 Matches on the receiver of an ObjectiveC Message expression.
 
Index: include/clang/ASTMatchers/ASTMatchers.h
===
--- include/clang/ASTMatchers/ASTMatchers.h
+++ include/clang/ASTMatchers/ASTMatchers.h
@@ -2736,6 +2736,41 @@
   return InnerMatcher.matches(TypeDecl, Finder, Builder);
 }
 
+/// Returns true when the Objective-C message is sent to an instance.
+///
+/// Example
+/// matcher = objcMessagaeExpr(isInstanceMessage())
+/// matches
+/// \code
+///   NSString *x = @"hello";
+///   [x containsString:@"h"];
+/// \endcode
+/// but not
+/// \code
+///   [NSString stringWithFormat:@"format"];
+/// \endcode
+AST_MATCHER(ObjCMessageExpr, isInstanceMessage) {
+  return Node.isInstanceMessage();
+}
+
+/// Matches if the Objective-C 

r337209 - [ASTMatchers] Introduce Objective-C matchers `hasReceiver` and `isInstanceMessage` for ObjCMessageExpr

[George Karpenkov via cfe-commits]

Author: george.karpenkov
Date: Mon Jul 16 13:22:12 2018
New Revision: 337209

URL: http://llvm.org/viewvc/llvm-project?rev=337209=rev
Log:
[ASTMatchers] Introduce Objective-C matchers `hasReceiver` and 
`isInstanceMessage` for ObjCMessageExpr

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

Modified:
cfe/trunk/docs/LibASTMatchersReference.html
cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp
cfe/trunk/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Modified: cfe/trunk/docs/LibASTMatchersReference.html
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LibASTMatchersReference.html?rev=337209=337208=337209=diff
==
--- cfe/trunk/docs/LibASTMatchersReference.html (original)
+++ cfe/trunk/docs/LibASTMatchersReference.html Mon Jul 16 13:22:12 2018
@@ -3268,6 +3268,19 @@ represent an error condition in the tree
 
 
 
+Matcherhttp://clang.llvm.org/doxygen/classclang_1_1ObjCMessageExpr.html;>ObjCMessageExprisInstanceMessage
+Returns true when 
the Objective-C message is sent to an instance.
+
+Example
+matcher = objcMessagaeExpr(isInstanceMessage())
+matches
+  NSString *x = @"hello";
+  [x containsString:@"h"]
+but not
+  [NSString stringWithFormat:@"format"]
+
+
+
 Matcherhttp://clang.llvm.org/doxygen/classclang_1_1ObjCMessageExpr.html;>ObjCMessageExprmatchesSelectorstd::string 
RegExp
 Matches ObjC 
selectors whose name contains
 a substring matched by the given RegExp.
@@ -5875,6 +5888,18 @@ Example matches y in x(y)
 
 
 
+Matcherhttp://clang.llvm.org/doxygen/classclang_1_1ObjCMessageExpr.html;>ObjCMessageExprhasReceiverMatcherhttp://clang.llvm.org/doxygen/classclang_1_1Expr.html;>Expr 
InnerMatcher
+Matches if the 
Objective-C message is sent to an instance,
+and the inner matcher matches on that instance.
+
+For example the method call in
+  NSString *x = @"hello";
+  [x containsString:@"h"]
+is matched by
+objcMessageExpr(hasReceiver(declRefExpr(to(varDecl(hasName("x"))
+
+
+
 Matcherhttp://clang.llvm.org/doxygen/classclang_1_1ObjCMessageExpr.html;>ObjCMessageExprhasReceiverTypeMatcherhttp://clang.llvm.org/doxygen/classclang_1_1QualType.html;>QualType
 InnerMatcher
 Matches on the 
receiver of an ObjectiveC Message expression.
 

Modified: cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h?rev=337209=337208=337209=diff
==
--- cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h (original)
+++ cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h Mon Jul 16 13:22:12 2018
@@ -2736,6 +2736,41 @@ AST_MATCHER_P(ObjCMessageExpr, hasReceiv
   return InnerMatcher.matches(TypeDecl, Finder, Builder);
 }
 
+/// Returns true when the Objective-C message is sent to an instance.
+///
+/// Example
+/// matcher = objcMessagaeExpr(isInstanceMessage())
+/// matches
+/// \code
+///   NSString *x = @"hello";
+///   [x containsString:@"h"];
+/// \endcode
+/// but not
+/// \code
+///   [NSString stringWithFormat:@"format"];
+/// \endcode
+AST_MATCHER(ObjCMessageExpr, isInstanceMessage) {
+  return Node.isInstanceMessage();
+}
+
+/// Matches if the Objective-C message is sent to an instance,
+/// and the inner matcher matches on that instance.
+///
+/// For example the method call in
+/// \code
+///   NSString *x = @"hello";
+///   [x containsString:@"h"];
+/// \endcode
+/// is matched by
+/// objcMessageExpr(hasReceiver(declRefExpr(to(varDecl(hasName("x"))
+AST_MATCHER_P(ObjCMessageExpr, hasReceiver, internal::Matcher,
+  InnerMatcher) {
+  const Expr *ReceiverNode = Node.getInstanceReceiver();
+  return (ReceiverNode != nullptr &&
+  InnerMatcher.matches(*ReceiverNode->IgnoreParenImpCasts(), Finder,
+   Builder));
+}
+
 /// Matches when BaseName == Selector.getAsString()
 ///
 ///  matcher = objCMessageExpr(hasSelector("loadHTMLString:baseURL:"));

Modified: cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp?rev=337209=337208=337209=diff
==
--- cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp (original)
+++ cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp Mon Jul 16 13:22:12 2018
@@ -283,6 +283,7 @@ RegistryMaps::RegistryMaps() {
   REGISTER_MATCHER(hasParent);
   REGISTER_MATCHER(hasQualifier);
   REGISTER_MATCHER(hasRangeInit);
+  REGISTER_MATCHER(hasReceiver);
   REGISTER_MATCHER(hasReceiverType);
   REGISTER_MATCHER(hasReplacementType);
   REGISTER_MATCHER(hasReturnValue);
@@ -349,6 +350,7 @@ RegistryMaps::RegistryMaps() {
   REGISTER_MATCHER(isImplicit);
   REGISTER_MATCHER(isExpansionInFileMatching);
   REGISTER_MATCHER(isExpansionInMainFile);
+  REGISTER_MATCHER(isInstanceMessage);
   

[PATCH] D49398: clang-cl: Postpone Wmsvc-not-found emission until link.exe gets used.

[Zachary Turner via Phabricator via cfe-commits]

zturner added inline comments.



Comment at: lib/Driver/ToolChains/MSVC.cpp:467-468
   if (Linker.equals_lower("link")) {
+if (!TC.FoundMSVCInstall())
+  C.getDriver().Diag(clang::diag::warn_drv_msvc_not_found);
+

It looks like it's possible for this warning to be emitted even when 
`FindVisualStudioExecutable` succeeds (after looking in the install location it 
checks `PATH`).  Would it make more sense to put this check after the call to 
`FindVisualStudioExecutable`, but only if it fails?


https://reviews.llvm.org/D49398



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

r337207 - [OPENMP] Fix checks for declare target link entries.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Mon Jul 16 13:05:25 2018
New Revision: 337207

URL: http://llvm.org/viewvc/llvm-project?rev=337207=rev
Log:
[OPENMP] Fix checks for declare target link entries.

If the declare target link entries are created but not used, the
compiler will produce an error message. Patch improves handling of such
situations + improves checks for possibly lost declare target variables.

Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.h
cfe/trunk/test/OpenMP/declare_target_link_codegen.cpp

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=337207=337206=337207=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Mon Jul 16 13:05:25 2018
@@ -3980,16 +3980,39 @@ void CGOpenMPRuntime::createOffloadEntri
 } else if (const auto *CE =
dyn_cast(E)) {
-  if (!CE->getAddress()) {
-unsigned DiagID = CGM.getDiags().getCustomDiagID(
-DiagnosticsEngine::Error,
-"Offloading entry for declare target variable is incorrect: the "
-"address is invalid.");
-CGM.getDiags().Report(DiagID);
-continue;
+  OffloadEntriesInfoManagerTy::OMPTargetGlobalVarEntryKind Flags =
+  
static_cast(
+  CE->getFlags());
+  switch (Flags) {
+  case OffloadEntriesInfoManagerTy::OMPTargetGlobalVarEntryTo: {
+if (!CE->getAddress()) {
+  unsigned DiagID = CGM.getDiags().getCustomDiagID(
+  DiagnosticsEngine::Error,
+  "Offloading entry for declare target variable is incorrect: the "
+  "address is invalid.");
+  CGM.getDiags().Report(DiagID);
+  continue;
+}
+break;
+  }
+  case OffloadEntriesInfoManagerTy::OMPTargetGlobalVarEntryLink:
+assert(((CGM.getLangOpts().OpenMPIsDevice && !CE->getAddress()) ||
+(!CGM.getLangOpts().OpenMPIsDevice && CE->getAddress())) &&
+   "Declaret target link address is set.");
+if (CGM.getLangOpts().OpenMPIsDevice)
+  continue;
+if (!CE->getAddress()) {
+  unsigned DiagID = CGM.getDiags().getCustomDiagID(
+  DiagnosticsEngine::Error,
+  "Offloading entry for declare target variable is incorrect: the "
+  "address is invalid.");
+  CGM.getDiags().Report(DiagID);
+  continue;
+}
+break;
   }
   createOffloadEntry(CE->getAddress(), CE->getAddress(),
- CE->getVarSize().getQuantity(), CE->getFlags(),
+ CE->getVarSize().getQuantity(), Flags,
  CE->getLinkage());
 } else {
   llvm_unreachable("Unsupported entry kind.");
@@ -7889,14 +7912,15 @@ void CGOpenMPRuntime::registerTargetGlob
   Linkage = CGM.getLLVMLinkageVarDefinition(VD, /*IsConstant=*/false);
   break;
 case OMPDeclareTargetDeclAttr::MT_Link:
-  // Map type 'to' because we do not map the original variable but the
-  // reference.
-  Flags = OffloadEntriesInfoManagerTy::OMPTargetGlobalVarEntryTo;
-  if (!CGM.getLangOpts().OpenMPIsDevice) {
+  Flags = OffloadEntriesInfoManagerTy::OMPTargetGlobalVarEntryLink;
+  if (CGM.getLangOpts().OpenMPIsDevice) {
+VarName = Addr->getName();
+Addr = nullptr;
+  } else {
+VarName = getAddrOfDeclareTargetLink(VD).getName();
 Addr =
 cast(getAddrOfDeclareTargetLink(VD).getPointer());
   }
-  VarName = Addr->getName();
   VarSize = CGM.getPointerSize();
   Linkage = llvm::GlobalValue::WeakAnyLinkage;
   break;

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.h?rev=337207=337206=337207=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.h (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.h Mon Jul 16 13:05:25 2018
@@ -522,6 +522,8 @@ private:
 enum OMPTargetGlobalVarEntryKind : uint32_t {
   /// Mark the entry as a to declare target.
   OMPTargetGlobalVarEntryTo = 0x0,
+  /// Mark the entry as a to declare target link.
+  OMPTargetGlobalVarEntryLink = 0x1,
 };
 
 /// Device global variable entries info.

Modified: cfe/trunk/test/OpenMP/declare_target_link_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/declare_target_link_codegen.cpp?rev=337207=337206=337207=diff
==
--- cfe/trunk/test/OpenMP/declare_target_link_codegen.cpp (original)
+++ cfe/trunk/test/OpenMP/declare_target_link_codegen.cpp Mon Jul 16 13:05:25 
2018
@@ -23,8 +23,9 @@
 // 

[libcxx] r337205 - Fix PR38160 - init_priority attribute not supported by GCC on Apple.

[Eric Fiselier via cfe-commits]

Author: ericwf
Date: Mon Jul 16 13:01:59 2018
New Revision: 337205

URL: http://llvm.org/viewvc/llvm-project?rev=337205=rev
Log:
Fix PR38160 - init_priority attribute not supported by GCC on Apple.

This patch guards the use of __attribute__((init_priority(101)))
within memory_resource.cpp when building with compilers that don't
support it. Specifically GCC on Apple platforms, and MSVC.

Modified:
libcxx/trunk/src/experimental/memory_resource.cpp

Modified: libcxx/trunk/src/experimental/memory_resource.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/experimental/memory_resource.cpp?rev=337205=337204=337205=diff
==
--- libcxx/trunk/src/experimental/memory_resource.cpp (original)
+++ libcxx/trunk/src/experimental/memory_resource.cpp Mon Jul 16 13:01:59 2018
@@ -68,12 +68,23 @@ union ResourceInitHelper {
   _LIBCPP_CONSTEXPR_AFTER_CXX11 ResourceInitHelper() : resources() {}
   ~ResourceInitHelper() {}
 };
+
+// Detect if the init_priority attribute is supported.
+#if (defined(_LIBCPP_COMPILER_GCC) && defined(__APPLE__)) \
+  || defined(_LIBCPP_COMPILER_MSVC)
+// GCC on Apple doesn't support the init priority attribute,
+// and MSVC doesn't support any GCC attributes.
+# define _LIBCPP_INIT_PRIORITY_MAX
+#else
+# define _LIBCPP_INIT_PRIORITY_MAX __attribute__((init_priority(101)))
+#endif
+
 // When compiled in C++14 this initialization should be a constant expression.
 // Only in C++11 is "init_priority" needed to ensure initialization order.
 #if _LIBCPP_STD_VER > 11
 _LIBCPP_SAFE_STATIC
 #endif
-ResourceInitHelper res_init  __attribute__((init_priority (101)));
+ResourceInitHelper res_init _LIBCPP_INIT_PRIORITY_MAX;
 
 } // end namespace
 


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

[PATCH] D49398: clang-cl: Postpone Wmsvc-not-found emission until link.exe gets used.

[Nico Weber via Phabricator via cfe-commits]

thakis created this revision.
thakis added a reviewer: zturner.

Wmsvc-not-found was added in r297851 to help diagnose why link.exe can't be 
executed. However, it's emitted even when using -fuse-ld=lld, and in cross 
builds there's no way to get rid of the warning other than disabling it.

Instead, emit it when we look up link.exe. That way, when passing -fuse-ld=lld 
it will never be printed.

(We might want to eventually default to lld one day, at least when running on a 
non-Win host, but that's for another day.)

Fixes PR38016.


https://reviews.llvm.org/D49398

Files:
  lib/Driver/ToolChains/MSVC.cpp
  lib/Driver/ToolChains/MSVC.h


Index: lib/Driver/ToolChains/MSVC.h
===
--- lib/Driver/ToolChains/MSVC.h
+++ lib/Driver/ToolChains/MSVC.h
@@ -123,6 +123,8 @@
 
   void printVerboseInfo(raw_ostream ) const override;
 
+  bool FoundMSVCInstall() const { return !VCToolChainPath.empty(); }
+
 protected:
   void AddSystemIncludeWithSubfolder(const llvm::opt::ArgList ,
  llvm::opt::ArgStringList ,
Index: lib/Driver/ToolChains/MSVC.cpp
===
--- lib/Driver/ToolChains/MSVC.cpp
+++ lib/Driver/ToolChains/MSVC.cpp
@@ -464,6 +464,9 @@
 Linker = "lld-link";
 
   if (Linker.equals_lower("link")) {
+if (!TC.FoundMSVCInstall())
+  C.getDriver().Diag(clang::diag::warn_drv_msvc_not_found);
+
 // If we're using the MSVC linker, it's not sufficient to just use link
 // from the program PATH, because other environments like GnuWin32 install
 // their own link.exe which may come first.
@@ -684,8 +687,6 @@
 }
 
 Tool *MSVCToolChain::buildLinker() const {
-  if (VCToolChainPath.empty())
-getDriver().Diag(clang::diag::warn_drv_msvc_not_found);
   return new tools::visualstudio::Linker(*this);
 }
 


Index: lib/Driver/ToolChains/MSVC.h
===
--- lib/Driver/ToolChains/MSVC.h
+++ lib/Driver/ToolChains/MSVC.h
@@ -123,6 +123,8 @@
 
   void printVerboseInfo(raw_ostream ) const override;
 
+  bool FoundMSVCInstall() const { return !VCToolChainPath.empty(); }
+
 protected:
   void AddSystemIncludeWithSubfolder(const llvm::opt::ArgList ,
  llvm::opt::ArgStringList ,
Index: lib/Driver/ToolChains/MSVC.cpp
===
--- lib/Driver/ToolChains/MSVC.cpp
+++ lib/Driver/ToolChains/MSVC.cpp
@@ -464,6 +464,9 @@
 Linker = "lld-link";
 
   if (Linker.equals_lower("link")) {
+if (!TC.FoundMSVCInstall())
+  C.getDriver().Diag(clang::diag::warn_drv_msvc_not_found);
+
 // If we're using the MSVC linker, it's not sufficient to just use link
 // from the program PATH, because other environments like GnuWin32 install
 // their own link.exe which may come first.
@@ -684,8 +687,6 @@
 }
 
 Tool *MSVCToolChain::buildLinker() const {
-  if (VCToolChainPath.empty())
-getDriver().Diag(clang::diag::warn_drv_msvc_not_found);
   return new tools::visualstudio::Linker(*this);
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49338: Implement - P0122R7

[Stephan T. Lavavej via Phabricator via cfe-commits]

STL_MSFT added inline comments.



Comment at: test/std/containers/views/span.comparison/op.eq.pass.cpp:23
+#include 
+#include 
+

The comparison tests appear to be unnecessarily including ``.



Comment at: test/std/containers/views/span.cons/assign.pass.cpp:25
+{
+static_assert(noexcept(std::declval() = rhs), "");
+lhs = rhs;

Technically need `` for declval.



Comment at: test/std/containers/views/span.cons/assign.pass.cpp:73
+
+//  No for loops in constexpr land :-(
+static_assert(doAssign(spans[0], spans[0]), "");

span is C++20, so you can use C++14 extended constexpr's for-loops. Just write 
a helper function.



Comment at: test/std/containers/views/span.cons/container.fail.cpp:44
+
+constexpr T const *getV() const {return _;} // for checking
+T v_;

This file is inconsistent about left vs right const (see line immediately 
above).



Comment at: test/std/containers/views/span.cons/deduct.pass.cpp:45
+using S = decltype(s);
+static_assert(std::is_same_v>, "");
+assert((std::equal(std::begin(arr), std::end(arr), s.begin(), s.end(;

Technically need `` for is_same_v.



Comment at: test/std/containers/views/span.cons/default.pass.cpp:61
+std::span s2;
+return
+assert(s1.data() == nullptr && s1.size() == 0);

This return appears to be spurious.



Comment at: test/std/containers/views/span.cons/span.pass.cpp:96
+std::spans2(s1); // static -> dynamic
+return
+assert(s1.data() == nullptr && s1.size() == 0);

Spurious return occurs in multiple files.



Comment at: test/std/containers/views/span.iterators/begin.pass.cpp:31
+{
+ret &= ( b ==  s.end());
+ret &= (cb == s.cend());

Using bitwise &= on a bool isn't very cromulent.



Comment at: test/std/containers/views/span.iterators/rbegin.pass.cpp:115
+std::string s;
+testRuntimeSpan(std::span(, (std::ptrdiff_t) 0));
+testRuntimeSpan(std::span(, 1));

static_cast would be nicer than C cast.



Comment at: test/std/containers/views/span.objectrep/as_bytes.pass.cpp:31
+{
+static_assert(noexcept(std::as_bytes(sp)));
+

This is a C++17 terse static assert unlike your others. Should you consistently 
use this form, or the other one?



Comment at: 
test/std/containers/views/span.objectrep/as_writeable_bytes.pass.cpp:42
+
+assert((void *) spBytes.data() == (void *) sp.data());
+assert(spBytes.size() == sp.size_bytes());

static_cast?



Comment at: test/std/containers/views/span.sub/last.pass.cpp:44
+ && s1.size() == s2.size()
+ && std::equal(s1.begin(), s1.end(), sp.end() - Count);
+}

Need ``.



Comment at: test/std/containers/views/span.sub/subspan.pass.cpp:45
+ && s1.size() == s2.size()
+ && std::equal(s1.begin(), s1.end(), sp.begin() + Offset);
+}

Also need ``, presumably occurs in more files.



Comment at: test/std/containers/views/types.pass.cpp:41
+{
+typedef std::iterator_traits ItT;
+

Need ``.


https://reviews.llvm.org/D49338



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

[PATCH] D49330: [compiler-rt] Include -lm when using compiler-rt, due to dependencies in some __div methods.

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

Probably compiler-rt should be fixed so it doesn't need libm, rather than 
fixing clang to add -lm.  (All the functions it currently uses can be 
implemented with simple bit manipulation.)


Repository:
  rC Clang

https://reviews.llvm.org/D49330



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

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

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

And finally (sorry about all the mails), this should probably be mentioned in 
the release notes (docs/ReleaseNotes.rst in the clang repo) since it's a 
notable new feature :-)


Repository:
  rC Clang

https://reviews.llvm.org/D46652



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

[PATCH] D49396: [WebAssembly] Support for atomic.wait / atomic.wake builtins

[Heejin Ahn via Phabricator via cfe-commits]

aheejin updated this revision to Diff 155738.
aheejin added a comment.

- test fix


Repository:
  rC Clang

https://reviews.llvm.org/D49396

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


Index: test/CodeGen/builtins-wasm.c
===
--- test/CodeGen/builtins-wasm.c
+++ test/CodeGen/builtins-wasm.c
@@ -50,3 +50,21 @@
 // WEBASSEMBLY32: call void @llvm.wasm.rethrow()
 // WEBASSEMBLY64: call void @llvm.wasm.rethrow()
 }
+
+unsigned int f8(void *addr, int expected, long long timeout) {
+  return __builtin_wasm_atomic_wait_i32(addr, expected, timeout);
+// WEBASSEMBLY32: call i32 @llvm.wasm.atomic.wait.i32(i8* %{{.*}}, i32 
%{{.*}}, i64 %{{.*}})
+// WEBASSEMBLY64: call i32 @llvm.wasm.atomic.wait.i32(i8* %{{.*}}, i32 
%{{.*}}, i64 %{{.*}})
+}
+
+unsigned int f9(void *addr, long long expected, long long timeout) {
+  return __builtin_wasm_atomic_wait_i64(addr, expected, timeout);
+// WEBASSEMBLY32: call i32 @llvm.wasm.atomic.wait.i64(i8* %{{.*}}, i64 
%{{.*}}, i64 %{{.*}})
+// WEBASSEMBLY64: call i32 @llvm.wasm.atomic.wait.i64(i8* %{{.*}}, i64 
%{{.*}}, i64 %{{.*}})
+}
+
+unsigned long long f10(void *addr, long long wake_count) {
+  return __builtin_wasm_atomic_wake(addr, wake_count);
+// WEBASSEMBLY32: call i64 @llvm.wasm.atomic.wake(i8* %{{.*}}, i64 %{{.*}})
+// WEBASSEMBLY64: call i64 @llvm.wasm.atomic.wake(i8* %{{.*}}, i64 %{{.*}})
+}
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -12054,6 +12054,26 @@
 Value *Callee = CGM.getIntrinsic(Intrinsic::wasm_rethrow);
 return Builder.CreateCall(Callee);
   }
+  case WebAssembly::BI__builtin_wasm_atomic_wait_i32: {
+Value *Addr = EmitScalarExpr(E->getArg(0));
+Value *Expected = EmitScalarExpr(E->getArg(1));
+Value *Timeout = EmitScalarExpr(E->getArg(2));
+Value *Callee = CGM.getIntrinsic(Intrinsic::wasm_atomic_wait_i32);
+return Builder.CreateCall(Callee, {Addr, Expected, Timeout});
+  }
+  case WebAssembly::BI__builtin_wasm_atomic_wait_i64: {
+Value *Addr = EmitScalarExpr(E->getArg(0));
+Value *Expected = EmitScalarExpr(E->getArg(1));
+Value *Timeout = EmitScalarExpr(E->getArg(2));
+Value *Callee = CGM.getIntrinsic(Intrinsic::wasm_atomic_wait_i64);
+return Builder.CreateCall(Callee, {Addr, Expected, Timeout});
+  }
+  case WebAssembly::BI__builtin_wasm_atomic_wake: {
+Value *Addr = EmitScalarExpr(E->getArg(0));
+Value *WakeCount = EmitScalarExpr(E->getArg(1));
+Value *Callee = CGM.getIntrinsic(Intrinsic::wasm_atomic_wake);
+return Builder.CreateCall(Callee, {Addr, WakeCount});
+  }
 
   default:
 return nullptr;
Index: include/clang/Basic/BuiltinsWebAssembly.def
===
--- include/clang/Basic/BuiltinsWebAssembly.def
+++ include/clang/Basic/BuiltinsWebAssembly.def
@@ -34,4 +34,9 @@
 BUILTIN(__builtin_wasm_throw, "vUiv*", "r")
 BUILTIN(__builtin_wasm_rethrow, "v", "r")
 
+// Atomic wait and wake.
+BUILTIN(__builtin_wasm_atomic_wait_i32, "Uiv*iLLi", "n")
+BUILTIN(__builtin_wasm_atomic_wait_i64, "Uiv*LLiLLi", "n")
+BUILTIN(__builtin_wasm_atomic_wake, "ULLiv*LLi", "n")
+
 #undef BUILTIN


Index: test/CodeGen/builtins-wasm.c
===
--- test/CodeGen/builtins-wasm.c
+++ test/CodeGen/builtins-wasm.c
@@ -50,3 +50,21 @@
 // WEBASSEMBLY32: call void @llvm.wasm.rethrow()
 // WEBASSEMBLY64: call void @llvm.wasm.rethrow()
 }
+
+unsigned int f8(void *addr, int expected, long long timeout) {
+  return __builtin_wasm_atomic_wait_i32(addr, expected, timeout);
+// WEBASSEMBLY32: call i32 @llvm.wasm.atomic.wait.i32(i8* %{{.*}}, i32 %{{.*}}, i64 %{{.*}})
+// WEBASSEMBLY64: call i32 @llvm.wasm.atomic.wait.i32(i8* %{{.*}}, i32 %{{.*}}, i64 %{{.*}})
+}
+
+unsigned int f9(void *addr, long long expected, long long timeout) {
+  return __builtin_wasm_atomic_wait_i64(addr, expected, timeout);
+// WEBASSEMBLY32: call i32 @llvm.wasm.atomic.wait.i64(i8* %{{.*}}, i64 %{{.*}}, i64 %{{.*}})
+// WEBASSEMBLY64: call i32 @llvm.wasm.atomic.wait.i64(i8* %{{.*}}, i64 %{{.*}}, i64 %{{.*}})
+}
+
+unsigned long long f10(void *addr, long long wake_count) {
+  return __builtin_wasm_atomic_wake(addr, wake_count);
+// WEBASSEMBLY32: call i64 @llvm.wasm.atomic.wake(i8* %{{.*}}, i64 %{{.*}})
+// WEBASSEMBLY64: call i64 @llvm.wasm.atomic.wake(i8* %{{.*}}, i64 %{{.*}})
+}
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -12054,6 +12054,26 @@
 Value *Callee = CGM.getIntrinsic(Intrinsic::wasm_rethrow);
 return Builder.CreateCall(Callee);
   }
+  case WebAssembly::BI__builtin_wasm_atomic_wait_i32: {
+Value *Addr = EmitScalarExpr(E->getArg(0));
+Value *Expected = 

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

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Also, were you planning on also adding support for the (filename-less version 
of) hdrstop pragma? After this change, that should probably be fairly 
straightforward.


Repository:
  rC Clang

https://reviews.llvm.org/D46652



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

[PATCH] D49396: [WebAssembly] Support for atomic.wait / atomic.wake builtins

[Heejin Ahn via Phabricator via cfe-commits]

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

Add support for atomic.wait / atomic.wake builtins based on the Wasm
thread proposal.


Repository:
  rC Clang

https://reviews.llvm.org/D49396

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


Index: test/CodeGen/builtins-wasm.c
===
--- test/CodeGen/builtins-wasm.c
+++ test/CodeGen/builtins-wasm.c
@@ -50,3 +50,20 @@
 // WEBASSEMBLY32: call void @llvm.wasm.rethrow()
 // WEBASSEMBLY64: call void @llvm.wasm.rethrow()
 }
+
+unsigned int f8(void *addr, int expected, long long timeout) {
+  return __builtin_wasm_atomic_wait_i32(addr, expected, timeout);
+// WEBASSEMBLY32: call i32 @llvm.wasm.atomic.wait.i32(i8* %{{.*}}, i32 
%{{.*}}, i64 %{{.*}})
+// WEBASSEMBLY64: call i32 @llvm.wasm.atomic.wait.i32(i8* %{{.*}}, i32 
%{{.*}}, i64 %{{.*}})
+}
+
+unsigned int f9(void *addr, long long expected, long long timeout) {
+  return __builtin_wasm_atomic_wait_i64(addr, expected, timeout);
+// WEBASSEMBLY32: call i32 @llvm.wasm.atomic.wait.i64(i8* %{{.*}}, i64 
%{{.*}}, i64 %{{.*}})
+// WEBASSEMBLY64: call i32 @llvm.wasm.atomic.wait.i64(i8* %{{.*}}, i64 
%{{.*}}, i64 %{{.*}})
+}
+
+unsigned long long f10(void *addr, long long wake_count) {
+  return __builtin_wasm_atomic_wake(addr, wake_count);
+// WEBASSEMBLY32: call i64 @llvm.wasm.atomic.wake(i8* %{{.*}}, i64 %{{.*}})
+}
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -12054,6 +12054,26 @@
 Value *Callee = CGM.getIntrinsic(Intrinsic::wasm_rethrow);
 return Builder.CreateCall(Callee);
   }
+  case WebAssembly::BI__builtin_wasm_atomic_wait_i32: {
+Value *Addr = EmitScalarExpr(E->getArg(0));
+Value *Expected = EmitScalarExpr(E->getArg(1));
+Value *Timeout = EmitScalarExpr(E->getArg(2));
+Value *Callee = CGM.getIntrinsic(Intrinsic::wasm_atomic_wait_i32);
+return Builder.CreateCall(Callee, {Addr, Expected, Timeout});
+  }
+  case WebAssembly::BI__builtin_wasm_atomic_wait_i64: {
+Value *Addr = EmitScalarExpr(E->getArg(0));
+Value *Expected = EmitScalarExpr(E->getArg(1));
+Value *Timeout = EmitScalarExpr(E->getArg(2));
+Value *Callee = CGM.getIntrinsic(Intrinsic::wasm_atomic_wait_i64);
+return Builder.CreateCall(Callee, {Addr, Expected, Timeout});
+  }
+  case WebAssembly::BI__builtin_wasm_atomic_wake: {
+Value *Addr = EmitScalarExpr(E->getArg(0));
+Value *WakeCount = EmitScalarExpr(E->getArg(1));
+Value *Callee = CGM.getIntrinsic(Intrinsic::wasm_atomic_wake);
+return Builder.CreateCall(Callee, {Addr, WakeCount});
+  }
 
   default:
 return nullptr;
Index: include/clang/Basic/BuiltinsWebAssembly.def
===
--- include/clang/Basic/BuiltinsWebAssembly.def
+++ include/clang/Basic/BuiltinsWebAssembly.def
@@ -34,4 +34,9 @@
 BUILTIN(__builtin_wasm_throw, "vUiv*", "r")
 BUILTIN(__builtin_wasm_rethrow, "v", "r")
 
+// Atomic wait and wake.
+BUILTIN(__builtin_wasm_atomic_wait_i32, "Uiv*iLLi", "n")
+BUILTIN(__builtin_wasm_atomic_wait_i64, "Uiv*LLiLLi", "n")
+BUILTIN(__builtin_wasm_atomic_wake, "ULLiv*LLi", "n")
+
 #undef BUILTIN


Index: test/CodeGen/builtins-wasm.c
===
--- test/CodeGen/builtins-wasm.c
+++ test/CodeGen/builtins-wasm.c
@@ -50,3 +50,20 @@
 // WEBASSEMBLY32: call void @llvm.wasm.rethrow()
 // WEBASSEMBLY64: call void @llvm.wasm.rethrow()
 }
+
+unsigned int f8(void *addr, int expected, long long timeout) {
+  return __builtin_wasm_atomic_wait_i32(addr, expected, timeout);
+// WEBASSEMBLY32: call i32 @llvm.wasm.atomic.wait.i32(i8* %{{.*}}, i32 %{{.*}}, i64 %{{.*}})
+// WEBASSEMBLY64: call i32 @llvm.wasm.atomic.wait.i32(i8* %{{.*}}, i32 %{{.*}}, i64 %{{.*}})
+}
+
+unsigned int f9(void *addr, long long expected, long long timeout) {
+  return __builtin_wasm_atomic_wait_i64(addr, expected, timeout);
+// WEBASSEMBLY32: call i32 @llvm.wasm.atomic.wait.i64(i8* %{{.*}}, i64 %{{.*}}, i64 %{{.*}})
+// WEBASSEMBLY64: call i32 @llvm.wasm.atomic.wait.i64(i8* %{{.*}}, i64 %{{.*}}, i64 %{{.*}})
+}
+
+unsigned long long f10(void *addr, long long wake_count) {
+  return __builtin_wasm_atomic_wake(addr, wake_count);
+// WEBASSEMBLY32: call i64 @llvm.wasm.atomic.wake(i8* %{{.*}}, i64 %{{.*}})
+}
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -12054,6 +12054,26 @@
 Value *Callee = CGM.getIntrinsic(Intrinsic::wasm_rethrow);
 return Builder.CreateCall(Callee);
   }
+  case WebAssembly::BI__builtin_wasm_atomic_wait_i32: {
+Value *Addr = EmitScalarExpr(E->getArg(0));
+Value *Expected = 

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

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Sorry about missing this. Looks great, thanks for doing this. This was a pretty 
big omission, glad to see it's done now :-)

I always imagined we might reuse the precompiled preamble stuff 
(Lexer::ComputePreamble() etc), but it's pretty clear as-is.


Repository:
  rC Clang

https://reviews.llvm.org/D46652



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

[PATCH] D49265: [Tooling] Add operator== to CompileCommand

[Simon Marchi via Phabricator via cfe-commits]

simark added a comment.

In https://reviews.llvm.org/D49265#1163740, @dblaikie wrote:

> Any chance this can/should be unit tested? (also, in general (though might
>  not matter in this instance), symmetric operators like == should be
>  implemented as non-members (though they can still be friends and if they
>  are, can be defined inline in the class definition as a member could be),
>  so any implicit conversions apply equally to the LHS as the RHS of the
>  expression)


Of course, I'm on it.


Repository:
  rC Clang

https://reviews.llvm.org/D49265



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

[PATCH] D49387: [analyzer] Make checkEndFunction() give access to the return statement

[Artem Dergachev via Phabricator via cfe-commits]

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

Looks great, thanks!




Comment at: include/clang/StaticAnalyzer/Core/CheckerManager.h:501
 
-  void _registerForBeginFunction(CheckEndFunctionFunc checkfn);
+  void _registerForBeginFunction(CheckBeginFunctionFunc checkfn);
   void _registerForEndFunction(CheckEndFunctionFunc checkfn);

:D


Repository:
  rC Clang

https://reviews.llvm.org/D49387



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

[PATCH] D48559: [clangd] refactoring for XPC transport layer [NFCI]

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Hi Jan,

Thanks for putting this together, and apologies - this is one of the places 
where we don't have nice abstractions/layering, so adding XPC was harder than 
it should be.

As I mentioned on the other review, I think maybe this patch isn't invasive 
enough, we could do a more thorough job of making the json transport a 
standalone, first-class thing. This makes it easier to swap out for XPC but 
also would improve the layering in the core clangd classes.

I put together a sketch of a `Transport` interface in 
https://reviews.llvm.org/D49389 (that patch is extremely unfinished and won't 
compile, but the idea might be interesting).
The idea is that we should be able to implement that class for XPC and then it 
should just drop-in as a replacement for the JSON one.
I've indicated there where XPC and JSON implementation can share code (assuming 
you wouldn't rather "tighten up" the protocol and eliminate some JSON-RPC 
noise).

If you like this direction I'm happy for you to pick the bits you like, or I 
can finish it as a refactoring and we can make sure it works for XPC.
If not, that's fine too - happy to look at other ways to reduce duplication 
between them.




Comment at: DispatcherCommon.h:38
+  // Return a context that's aware of the enclosing request, identified by 
Span.
+  static Context stash(const trace::Span );
+

I think I wrote this bit... it's a hack that I'm not sure deserves to be 
visible in a header (it was bad enough in JSONRPCDispatcher.cpp!)

Rather than exposing it so we can use it twice, I'd hope we can manage to keep 
it hidden so that JSON and XPC can share it.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D48559



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

[PATCH] D49389: [clangd] Transport abstraction WIP

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
Herald added subscribers: cfe-commits, jkorous, MaskRay, ioeric, ilya-biryukov.

Ideas about abstracting JSON transport away to allow XPC and improve layering.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D49389

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/Protocol.h
  clangd/Transport.cpp
  clangd/Transport.h

Index: clangd/Transport.h
===
--- /dev/null
+++ clangd/Transport.h
@@ -0,0 +1,90 @@
+//===--- Transport.h - sending and receiving LSP messages ---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// The language server protocol is usually implemented by writing messages as
+// JSON-RPC over the stdin/stdout of a subprocess. However other communications
+// mechanisms are possible, such as XPC on mac (see xpc/ directory).
+//
+// The Transport interface allows the mechanism to be replaced, and the JSONRPC
+// Transport is the standard implementation.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_TRANSPORT_H_
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_TRANSPORT_H_
+
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/JSON.h"
+#include "llvm/Support/raw_ostream.h"
+
+namespace clang {
+namespace clangd {
+
+// A transport is responsible for maintaining the connection to a client
+// application, and reading/writing structured messages to it.
+//
+// Transports have limited thread safety requirements:
+//  - messages will not be sent concurrently
+//  - messages MAY be sent while loop() is reading, or its callback is active
+class Transport {
+public:
+  virtual ~Transport() = default;
+
+  // Called by Clangd to send messages to the client.
+  // (Because JSON and XPC are so similar, these are concrete and delegate to
+  // sendMessage. We could change this to support more diverse transports).
+  void notify(llvm::StringRef Method, llvm::json::Value Params);
+  void call(llvm::StringRef Method, llvm::json::Value Params,
+llvm::json::Value ID);
+  void reply(llvm::json::Value ID, llvm::Expected Result);
+
+  // Implemented by Clangd to handle incoming messages. (See loop() below).
+  class MessageHandler {
+  public:
+virtual ~MessageHandler() = 0;
+virtual bool notify(llvm::StringRef Method, llvm::json::Value ) = 0;
+virtual bool call(llvm::StringRef Method, llvm::json::Value Params,
+  llvm::json::Value ID) = 0;
+virtual bool reply(llvm::json::Value ID,
+   llvm::Expected Result) = 0;
+  };
+  // Called by Clangd to receive messages from the client.
+  // The transport should in turn invoke the handler to process messages.
+  // If handler returns true, the transport should immedately return success.
+  // Otherwise, it returns an error when the transport becomes unusable.
+  // (Because JSON and XPC are so similar, they share handleMessage()).
+  virtual llvm::Error loop(MessageHandler &) = 0;
+
+protected:
+  // Common implementation for notify(), call(), and reply().
+  virtual void sendMessage(llvm::json::Value) = 0;
+  // Delegates to notify(), call(), and reply().
+  bool handleMessage(llvm::json::Value, MessageHandler&);
+};
+
+// Controls the way JSON-RPC messages are encoded (both input and output).
+enum JSONStreamStyle {
+  // Encoding per the LSP specification, with mandatory Content-Length header.
+  Standard,
+  // Messages are delimited by a '---' line. Comment lines start with #.
+  Delimited
+};
+
+// Returns a Transport that speaks JSON-RPC over a pair of streams.
+// The input stream must be opened in binary mode.
+std::unique_ptr
+newJSONTransport(std::FILE *In, llvm::raw_ostream ,
+ JSONStreamStyle = JSONStreamStyle::Standard);
+
+} // namespace clangd
+} // namespace clang
+
+#endif
+
+
Index: clangd/Transport.cpp
===
--- /dev/null
+++ clangd/Transport.cpp
@@ -0,0 +1,225 @@
+//===--- Transport.cpp - sending and receiving LSP messages -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// The JSON-RPC transport is implemented here.
+// The alternative Mac-only XPC transport is in the xpc/ directory.
+//
+//===--===//
+#include "Transport.h"
+#include "Logger.h"
+#include "Protocol.h"
+#include "llvm/Support/Errno.h"
+
+using namespace llvm;
+namespace clang {
+namespace 

[PATCH] D22391: [Sema] Add warning for implicitly casting a null constant to a non null pointer type

[Jordan Rose via Phabricator via cfe-commits]

jordan_rose added inline comments.



Comment at: test/Sema/conditional-expr.c:20
   vp = 0 ? (double *)0 : (void *)0;
-  ip = 0 ? (double *)0 : (void *)0; // expected-warning {{incompatible pointer 
types assigning to 'int *' from 'double *'}}
+  ip = 0 ? (double *)0 : (void *)0; // expected-warning {{incompatible pointer 
types assigning to 'int *' from 'double * _Nullable'}}
 

ahatanak wrote:
> ahatanak wrote:
> > jordan_rose wrote:
> > > This seems like an unfortunate change to make, since most people do not 
> > > bother with nullability.
> > Yes, this is unfortunate, but I'm not sure what's the right way to avoid 
> > printing nullability specifiers in the diagnostic message. Do you have any 
> > suggestions?
> It looks like I can use PrintingPolicy to print the nullability specifier 
> only when needed.
> 
> I think it's also possible to add a flag to Expr that indicates the Expr is 
> possibly null. For example, when an Expr is an IntegerLiteral of value 0, or 
> a CastExpr or a ConditionalOperator has a subexpression whose flag is set. 
> This could be a better solution than the current solution in this patch.
Whew. I hadn't had the chance to look at PrintingPolicy and was afraid you'd 
have to add a new flag to diagnostics or something to specify whether 
nullability was relevant.

An additional flag on Expr seems like overkill to me, given that 
`isNullPointerConstant` already exists. But I don't work in Clang these days, 
so maybe I'm wrong and it is something worth caching. 


Repository:
  rC Clang

https://reviews.llvm.org/D22391



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

Re: r336467 - [OPENMP] Fix PR38026: Link -latomic when -fopenmp is used.

[Jonas Hahnfeld via cfe-commits]

[ Moving discussion from https://reviews.llvm.org/D49386 to the relevant 
comment on cfe-commits, CC'ing Hal who commented on the original issue ]


Is this change really a good idea? It always requires libatomic for all 
OpenMP applications, even if there is no 'omp atomic' directive or all 
of them can be lowered to atomic instructions that don't require a 
runtime library. I'd argue that it's a larger restriction than the 
problem it solves.
Per https://clang.llvm.org/docs/Toolchain.html#libatomic-gnu the user is 
expected to manually link -latomic whenever Clang can't lower atomic 
instructions - including C11 atomics and C++ atomics. In my opinion 
OpenMP is just another abstraction that doesn't require a special 
treatment.


Thoughts?
Jonas

On 2018-07-06 23:13, Alexey Bataev via cfe-commits wrote:

Author: abataev
Date: Fri Jul  6 14:13:41 2018
New Revision: 336467

URL: http://llvm.org/viewvc/llvm-project?rev=336467=rev
Log:
[OPENMP] Fix PR38026: Link -latomic when -fopenmp is used.

On Linux atomic constructs in OpenMP require libatomic library. Patch
links libatomic when -fopenmp is used.

Modified:
cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
cfe/trunk/test/OpenMP/linking.c

Modified: cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Gnu.cpp?rev=336467=336466=336467=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Gnu.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Gnu.cpp Fri Jul  6 14:13:41 2018
@@ -479,6 +479,7 @@ void tools::gnutools::Linker::ConstructJ

   bool WantPthread = Args.hasArg(options::OPT_pthread) ||
  Args.hasArg(options::OPT_pthreads);
+  bool WantAtomic = false;

   // FIXME: Only pass GompNeedsRT = true for platforms with 
libgomp that
   // require librt. Most modern Linux platforms do, but some may 
not.

@@ -487,13 +488,16 @@ void tools::gnutools::Linker::ConstructJ
/* GompNeedsRT= */ true))
 // OpenMP runtimes implies pthreads when using the GNU 
toolchain.

 // FIXME: Does this really make sense for all GNU toolchains?
-WantPthread = true;
+WantAtomic = WantPthread = true;

   AddRunTimeLibs(ToolChain, D, CmdArgs, Args);

   if (WantPthread && !isAndroid)
 CmdArgs.push_back("-lpthread");

+  if (WantAtomic)
+CmdArgs.push_back("-latomic");
+
   if (Args.hasArg(options::OPT_fsplit_stack))
 CmdArgs.push_back("--wrap=pthread_create");


Modified: cfe/trunk/test/OpenMP/linking.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/linking.c?rev=336467=336466=336467=diff
==
--- cfe/trunk/test/OpenMP/linking.c (original)
+++ cfe/trunk/test/OpenMP/linking.c Fri Jul  6 14:13:41 2018
@@ -8,14 +8,14 @@
 // RUN:   | FileCheck --check-prefix=CHECK-LD-32 %s
 // CHECK-LD-32: "{{.*}}ld{{(.exe)?}}"
 // CHECK-LD-32: "-l[[DEFAULT_OPENMP_LIB:[^"]*]]"
-// CHECK-LD-32: "-lpthread" "-lc"
+// CHECK-LD-32: "-lpthread" "-latomic" "-lc"
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: -fopenmp -target x86_64-unknown-linux -rtlib=platform \
 // RUN:   | FileCheck --check-prefix=CHECK-LD-64 %s
 // CHECK-LD-64: "{{.*}}ld{{(.exe)?}}"
 // CHECK-LD-64: "-l[[DEFAULT_OPENMP_LIB:[^"]*]]"
-// CHECK-LD-64: "-lpthread" "-lc"
+// CHECK-LD-64: "-lpthread" "-latomic" "-lc"
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: -fopenmp=libgomp -target i386-unknown-linux 
-rtlib=platform \

@@ -27,7 +27,7 @@
 // SIMD-ONLY2-NOT: liomp
 // CHECK-GOMP-LD-32: "{{.*}}ld{{(.exe)?}}"
 // CHECK-GOMP-LD-32: "-lgomp" "-lrt"
-// CHECK-GOMP-LD-32: "-lpthread" "-lc"
+// CHECK-GOMP-LD-32: "-lpthread" "-latomic" "-lc"

 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1
-fopenmp-simd -target i386-unknown-linux -rtlib=platform | FileCheck
--check-prefix SIMD-ONLY2 %s
 // SIMD-ONLY2-NOT: lgomp
@@ -39,21 +39,21 @@
 // RUN:   | FileCheck --check-prefix=CHECK-GOMP-LD-64 %s
 // CHECK-GOMP-LD-64: "{{.*}}ld{{(.exe)?}}"
 // CHECK-GOMP-LD-64: "-lgomp" "-lrt"
-// CHECK-GOMP-LD-64: "-lpthread" "-lc"
+// CHECK-GOMP-LD-64: "-lpthread" "-latomic" "-lc"
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: -fopenmp -target i386-unknown-linux -rtlib=platform \
 // RUN:   | FileCheck --check-prefix=CHECK-IOMP5-LD-32 %s
 // CHECK-IOMP5-LD-32: "{{.*}}ld{{(.exe)?}}"
 // CHECK-IOMP5-LD-32: "-l[[DEFAULT_OPENMP_LIB:[^"]*]]"
-// CHECK-IOMP5-LD-32: "-lpthread" "-lc"
+// CHECK-IOMP5-LD-32: "-lpthread" "-latomic" "-lc"
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: -fopenmp -target x86_64-unknown-linux -rtlib=platform \
 // RUN:   | FileCheck --check-prefix=CHECK-IOMP5-LD-64 %s
 // CHECK-IOMP5-LD-64: "{{.*}}ld{{(.exe)?}}"
 // CHECK-IOMP5-LD-64: "-l[[DEFAULT_OPENMP_LIB:[^"]*]]"

[PATCH] D22391: [Sema] Add warning for implicitly casting a null constant to a non null pointer type

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added inline comments.



Comment at: test/Sema/conditional-expr.c:20
   vp = 0 ? (double *)0 : (void *)0;
-  ip = 0 ? (double *)0 : (void *)0; // expected-warning {{incompatible pointer 
types assigning to 'int *' from 'double *'}}
+  ip = 0 ? (double *)0 : (void *)0; // expected-warning {{incompatible pointer 
types assigning to 'int *' from 'double * _Nullable'}}
 

ahatanak wrote:
> jordan_rose wrote:
> > This seems like an unfortunate change to make, since most people do not 
> > bother with nullability.
> Yes, this is unfortunate, but I'm not sure what's the right way to avoid 
> printing nullability specifiers in the diagnostic message. Do you have any 
> suggestions?
It looks like I can use PrintingPolicy to print the nullability specifier only 
when needed.

I think it's also possible to add a flag to Expr that indicates the Expr is 
possibly null. For example, when an Expr is an IntegerLiteral of value 0, or a 
CastExpr or a ConditionalOperator has a subexpression whose flag is set. This 
could be a better solution than the current solution in this patch.


Repository:
  rC Clang

https://reviews.llvm.org/D22391



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

r337196 - [OPENMP] Fix syntactic errors in error messages.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Mon Jul 16 11:12:18 2018
New Revision: 337196

URL: http://llvm.org/viewvc/llvm-project?rev=337196=rev
Log:
[OPENMP] Fix syntactic errors in error messages.

Fixed spelling of the offloading error messages.

Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
cfe/trunk/test/OpenMP/target_messages.cpp

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=337196=337195=337196=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Mon Jul 16 11:12:18 2018
@@ -3970,7 +3970,7 @@ void CGOpenMPRuntime::createOffloadEntri
   if (!CE->getID() || !CE->getAddress()) {
 unsigned DiagID = CGM.getDiags().getCustomDiagID(
 DiagnosticsEngine::Error,
-"Offloading entry for target region is incorect: either the "
+"Offloading entry for target region is incorrect: either the "
 "address or the ID is invalid.");
 CGM.getDiags().Report(DiagID);
 continue;
@@ -3983,7 +3983,7 @@ void CGOpenMPRuntime::createOffloadEntri
   if (!CE->getAddress()) {
 unsigned DiagID = CGM.getDiags().getCustomDiagID(
 DiagnosticsEngine::Error,
-"Offloading entry for declare target varible is inccorect: the "
+"Offloading entry for declare target variable is incorrect: the "
 "address is invalid.");
 CGM.getDiags().Report(DiagID);
 continue;

Modified: cfe/trunk/test/OpenMP/target_messages.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/target_messages.cpp?rev=337196=337195=337196=diff
==
--- cfe/trunk/test/OpenMP/target_messages.cpp (original)
+++ cfe/trunk/test/OpenMP/target_messages.cpp Mon Jul 16 11:12:18 2018
@@ -14,7 +14,7 @@
 
 // RUN: %clang_cc1 -fopenmp -x c++ -triple powerpc64le-unknown-unknown 
-fopenmp-targets=powerpc64le-ibm-linux-gnu -emit-llvm-bc %s -o %t-ppc-host.bc 
-DREGION_HOST
 // RUN: not %clang_cc1 -fopenmp -x c++ -triple powerpc64le-unknown-unknown 
-fopenmp-targets=powerpc64le-ibm-linux-gnu -emit-llvm %s -fopenmp-is-device 
-fopenmp-host-ir-file-path %t-ppc-host.bc -o - -DREGION_DEVICE 2>&1 | FileCheck 
%s --check-prefix NO-REGION
-// NO-REGION: Offloading entry for target region is incorect: either the 
address or the ID is invalid.
+// NO-REGION: Offloading entry for target region is incorrect: either the 
address or the ID is invalid.
 
 #if defined(REGION_HOST) || defined(REGION_DEVICE)
 void foo() {


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

[PATCH] D49356: [clang-tidy: modernize] Fix modernize-use-equals-default with {} brackets list initialization: patch

[Idriss via Phabricator via cfe-commits]

IdrissRio updated this revision to Diff 155715.
IdrissRio added a comment.

Fixed the formatting error with clang-format


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D49356

Files:
  clang-tidy/modernize/UseEqualsDefaultCheck.cpp
  test/clang-tidy/modernize-use-equals-default-copy.cpp


Index: test/clang-tidy/modernize-use-equals-default-copy.cpp
===
--- test/clang-tidy/modernize-use-equals-default-copy.cpp
+++ test/clang-tidy/modernize-use-equals-default-copy.cpp
@@ -497,3 +497,11 @@
 STRUCT_WITH_COPY_ASSIGN(unsigned char, Hex8CopyAssign)
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use '= default' to define a 
trivial copy-assignment operator
 // CHECK-MESSAGES: :[[@LINE-9]]:40: note:
+
+// Use of braces
+struct UOB{
+  UOB(const UOB ):j{Other.j}{}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default' to define a 
trivial copy constructor [modernize-use-equals-default]
+  // CHECK-FIXES: UOB(const UOB )= default;
+  int j;
+};
Index: clang-tidy/modernize/UseEqualsDefaultCheck.cpp
===
--- clang-tidy/modernize/UseEqualsDefaultCheck.cpp
+++ clang-tidy/modernize/UseEqualsDefaultCheck.cpp
@@ -97,6 +97,7 @@
 isMemberInitializer(), forField(equalsNode(Field)),
 withInitializer(anyOf(
 AccessToFieldInParam,
+initListExpr(has(AccessToFieldInParam)),
 cxxConstructExpr(allOf(
 
hasDeclaration(cxxConstructorDecl(isCopyConstructor())),
 argumentCountIs(1),


Index: test/clang-tidy/modernize-use-equals-default-copy.cpp
===
--- test/clang-tidy/modernize-use-equals-default-copy.cpp
+++ test/clang-tidy/modernize-use-equals-default-copy.cpp
@@ -497,3 +497,11 @@
 STRUCT_WITH_COPY_ASSIGN(unsigned char, Hex8CopyAssign)
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use '= default' to define a trivial copy-assignment operator
 // CHECK-MESSAGES: :[[@LINE-9]]:40: note:
+
+// Use of braces
+struct UOB{
+  UOB(const UOB ):j{Other.j}{}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default' to define a trivial copy constructor [modernize-use-equals-default]
+  // CHECK-FIXES: UOB(const UOB )= default;
+  int j;
+};
Index: clang-tidy/modernize/UseEqualsDefaultCheck.cpp
===
--- clang-tidy/modernize/UseEqualsDefaultCheck.cpp
+++ clang-tidy/modernize/UseEqualsDefaultCheck.cpp
@@ -97,6 +97,7 @@
 isMemberInitializer(), forField(equalsNode(Field)),
 withInitializer(anyOf(
 AccessToFieldInParam,
+initListExpr(has(AccessToFieldInParam)),
 cxxConstructExpr(allOf(
 hasDeclaration(cxxConstructorDecl(isCopyConstructor())),
 argumentCountIs(1),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48862: [OpenEmbedded] Fix lib paths for OpenEmbedded targets

[Mandeep Singh Grang via Phabricator via cfe-commits]

mgrang added a comment.

Ping 2 for reviews please.


https://reviews.llvm.org/D48862



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

[PATCH] D49387: [analyzer] Make checkEndFunction() give access to the return statement

[Reka Kovacs via Phabricator via cfe-commits]

rnkovacs created this revision.
rnkovacs added reviewers: NoQ, xazax.hun, george.karpenkov.
Herald added subscribers: mikhail.ramalho, a.sidorin, dkrupp, szepet, 
baloghadamsoftware, whisperity.

Repository:
  rC Clang

https://reviews.llvm.org/D49387

Files:
  include/clang/StaticAnalyzer/Core/Checker.h
  include/clang/StaticAnalyzer/Core/CheckerManager.h
  lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
  lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp
  lib/StaticAnalyzer/Checkers/MisusedMovedObjectChecker.cpp
  lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
  lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
  lib/StaticAnalyzer/Checkers/TestAfterDivZeroChecker.cpp
  lib/StaticAnalyzer/Checkers/TraversalChecker.cpp
  lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp
  lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
  lib/StaticAnalyzer/Core/CheckerManager.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp

Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -2297,9 +2297,9 @@
 
 // Notify checkers.
 for (const auto I : AfterRemovedDead)
-  getCheckerManager().runCheckersForEndFunction(BC, Dst, I, *this);
+  getCheckerManager().runCheckersForEndFunction(BC, Dst, I, *this, RS);
   } else {
-getCheckerManager().runCheckersForEndFunction(BC, Dst, Pred, *this);
+getCheckerManager().runCheckersForEndFunction(BC, Dst, Pred, *this, RS);
   }
 
   Engine.enqueueEndOfFunction(Dst, RS);
Index: lib/StaticAnalyzer/Core/CheckerManager.cpp
===
--- lib/StaticAnalyzer/Core/CheckerManager.cpp
+++ lib/StaticAnalyzer/Core/CheckerManager.cpp
@@ -439,7 +439,8 @@
 void CheckerManager::runCheckersForEndFunction(NodeBuilderContext ,
ExplodedNodeSet ,
ExplodedNode *Pred,
-   ExprEngine ) {
+   ExprEngine ,
+   const ReturnStmt *RS) {
   // We define the builder outside of the loop bacause if at least one checkers
   // creates a sucsessor for Pred, we do not need to generate an
   // autotransition for it.
@@ -449,7 +450,7 @@
   Pred->getLocationContext(),
   checkFn.Checker);
 CheckerContext C(Bldr, Eng, Pred, L);
-checkFn(C);
+checkFn(RS, C);
   }
 }
 
Index: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
+++ lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
@@ -48,7 +48,7 @@
   DefaultBool IsPureOnly;
 
   void checkBeginFunction(CheckerContext ) const;
-  void checkEndFunction(CheckerContext ) const;
+  void checkEndFunction(const ReturnStmt *RS, CheckerContext ) const;
   void checkPreCall(const CallEvent , CheckerContext ) const;
 
 private:
@@ -167,7 +167,8 @@
 }
 
 // The EndFunction callback when leave a constructor or a destructor.
-void VirtualCallChecker::checkEndFunction(CheckerContext ) const {
+void VirtualCallChecker::checkEndFunction(const ReturnStmt *RS,
+  CheckerContext ) const {
   registerCtorDtorCallInState(false, C);
 }
 
Index: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp
+++ lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp
@@ -47,7 +47,7 @@
 
   UninitializedObjectChecker()
   : BT_uninitField(new BuiltinBug(this, "Uninitialized fields")) {}
-  void checkEndFunction(CheckerContext ) const;
+  void checkEndFunction(const ReturnStmt *RS, CheckerContext ) const;
 };
 
 /// Represents a field chain. A field chain is a vector of fields where the
@@ -241,7 +241,7 @@
 //===--===//
 
 void UninitializedObjectChecker::checkEndFunction(
-CheckerContext ) const {
+const ReturnStmt *RS, CheckerContext ) const {
 
   const auto *CtorDecl = dyn_cast_or_null(
   Context.getLocationContext()->getDecl());
Index: lib/StaticAnalyzer/Checkers/TraversalChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/TraversalChecker.cpp
+++ lib/StaticAnalyzer/Checkers/TraversalChecker.cpp
@@ -30,7 +30,7 @@
 public:
   void checkBranchCondition(const Stmt *Condition, CheckerContext ) const;
   void checkBeginFunction(CheckerContext ) const;
-  void checkEndFunction(CheckerContext ) const;
+  void checkEndFunction(const ReturnStmt *RS, CheckerContext ) const;
 };
 }
 
@@ -56,7 +56,8 @@
   llvm::outs() << 

[PATCH] D48436: [analyzer][UninitializedObjectChecker] Fixed a false negative by no longer filtering out certain constructor calls

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

Szelethus added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:674
+  const LocationContext *LC = Context.getLocationContext();
+  while ((LC = LC->getParent())) {
+

Szelethus wrote:
> NoQ wrote:
> > george.karpenkov wrote:
> > > nit: could we have `while (LC)` followed by `LC = LC->getParent()` ? Do 
> > > you intentionally skip the first location context?
> > I guess the predicate we're checking is trivially true for the current 
> > location context.
> Completely missed this inline, sorry!
> 
> As @NoQ said, since this checker only fires after a constructor call, the 
> first location context will surely be that, and I'm only checking whether the 
> current ctor was called by another.
I'm actually not sure how you imagined that. The loop condition is `LC`, but if 
I assign it to its parent right after that, I won't be sure that it's non-null. 
The reason why I placed it there is that early exits could restart the loop at 
"random" places.



Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:681-689
+Optional OtherObject =
+getObjectVal(OtherCtor, Context);
+if (!OtherObject)
+  continue;
+
+// If the CurrentObject is a field of OtherObject, it will be analyzed
+// during the analysis of OtherObject.

NoQ wrote:
> It seems safer to look at `CXXConstructExpr::getConstructionKind()`.
> 
> Taking a `LazyCompoundVal` and converting it back to a region is definitely a 
> bad idea because the region within `LazyCompoundVal` is completely immaterial 
> and carries no meaning for the value represented by this `SVal`; it's not 
> necessarily the region you're looking for.
Looking at the singleton test case, I think I need to get that region somehow, 
and I'm not too sure what you meant under using 
`CXXConstructExpr::getConstructionKind()`. One thing I could do, is similar to 
how `getObjectVal` works:
```
  Loc Tmp = Context.getSValBuilder().getCXXThis(OtherCtor->getParent(),
Context.getStackFrame());

  auto OtherThis = Context.getState()->getSVal(Tmp).castAs();

  OtherThis.getRegion(); // Hooray!
```

I have tested it, and it works wonders. Is this a "safe-to-use" region?


https://reviews.llvm.org/D48436



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

[PATCH] D41458: [libc++][C++17] Elementary string conversions for integral types

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added inline comments.



Comment at: include/charconv:372
+
+auto __len = __last - __p;
+if (__value != 0 || !__len)

mclow.lists wrote:
> Are you missing an edge case here? What happens if `__last == __first && 
> __value == 0`?
> 
Nevermind. Apparently I can't look two lines down.


Repository:
  rCXX libc++

https://reviews.llvm.org/D41458



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

[PATCH] D49058: [analyzer] Move DanglingInternalBufferChecker out of alpha

[Reka Kovacs via Phabricator via cfe-commits]

rnkovacs added a comment.

In https://reviews.llvm.org/D49058#1159533, @george.karpenkov wrote:

> @rnkovacs Do you have evaluation statistics handy for this checker? How many 
> bugs it finds, on which projects? How many of those are real bugs?


In its present form, it does not produce many reports, so it might be worth 
waiting for patches like https://reviews.llvm.org/D49360 and 
https://reviews.llvm.org/D49361 that add more functionality.
A recent run on LibreOffice showed one report by this checker, and it seems 
like a real bug (in an external package called GPGME).
Can be seen here 
,
 or if it does not work, line 135 here 
 
(quite a one-liner).


https://reviews.llvm.org/D49058



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

r337191 - [OPENMP, NVPTX] Globalize only captured variables.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Mon Jul 16 09:49:20 2018
New Revision: 337191

URL: http://llvm.org/viewvc/llvm-project?rev=337191=rev
Log:
[OPENMP, NVPTX] Globalize only captured variables.

Sometimes we can try to globalize non-variable declarations, which may
lead to compiler crash.

Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
cfe/trunk/test/OpenMP/nvptx_target_codegen.cpp

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp?rev=337191=337190=337191=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp Mon Jul 16 09:49:20 2018
@@ -204,7 +204,7 @@ class CheckVarsEscapingDeclContext final
 
   void markAsEscaped(const ValueDecl *VD) {
 // Do not globalize declare target variables.
-if (isDeclareTargetDeclaration(VD))
+if (!isa(VD) || isDeclareTargetDeclaration(VD))
   return;
 VD = cast(VD->getCanonicalDecl());
 // Variables captured by value must be globalized.

Modified: cfe/trunk/test/OpenMP/nvptx_target_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/nvptx_target_codegen.cpp?rev=337191=337190=337191=diff
==
--- cfe/trunk/test/OpenMP/nvptx_target_codegen.cpp (original)
+++ cfe/trunk/test/OpenMP/nvptx_target_codegen.cpp Mon Jul 16 09:49:20 2018
@@ -9,12 +9,12 @@
 #define HEADER
 
 // Check that the execution mode of all 6 target regions is set to Generic 
Mode.
-// CHECK-DAG: {{@__omp_offloading_.+l102}}_exec_mode = weak constant i8 1
-// CHECK-DAG: {{@__omp_offloading_.+l179}}_exec_mode = weak constant i8 1
-// CHECK-DAG: {{@__omp_offloading_.+l289}}_exec_mode = weak constant i8 1
-// CHECK-DAG: {{@__omp_offloading_.+l326}}_exec_mode = weak constant i8 1
-// CHECK-DAG: {{@__omp_offloading_.+l344}}_exec_mode = weak constant i8 1
-// CHECK-DAG: {{@__omp_offloading_.+l309}}_exec_mode = weak constant i8 1
+// CHECK-DAG: {{@__omp_offloading_.+l103}}_exec_mode = weak constant i8 1
+// CHECK-DAG: {{@__omp_offloading_.+l180}}_exec_mode = weak constant i8 1
+// CHECK-DAG: {{@__omp_offloading_.+l290}}_exec_mode = weak constant i8 1
+// CHECK-DAG: {{@__omp_offloading_.+l328}}_exec_mode = weak constant i8 1
+// CHECK-DAG: {{@__omp_offloading_.+l346}}_exec_mode = weak constant i8 1
+// CHECK-DAG: {{@__omp_offloading_.+l311}}_exec_mode = weak constant i8 1
 
 __thread int id;
 
@@ -24,6 +24,7 @@ template
 struct TT{
   tx X;
   ty Y;
+  tx [](int i) { return X; }
 };
 
 int foo(int n) {
@@ -35,7 +36,7 @@ int foo(int n) {
   double cn[5][n];
   TT d;
 
-  // CHECK-LABEL: define {{.*}}void {{@__omp_offloading_.+foo.+l102}}_worker()
+  // CHECK-LABEL: define {{.*}}void {{@__omp_offloading_.+foo.+l103}}_worker()
   // CHECK-DAG: [[OMP_EXEC_STATUS:%.+]] = alloca i8,
   // CHECK-DAG: [[OMP_WORK_FN:%.+]] = alloca i8*,
   // CHECK: store i8* null, i8** [[OMP_WORK_FN]],
@@ -66,7 +67,7 @@ int foo(int n) {
   // CHECK: [[EXIT]]
   // CHECK: ret void
 
-  // CHECK: define {{.*}}void [[T1:@__omp_offloading_.+foo.+l102]]()
+  // CHECK: define {{.*}}void [[T1:@__omp_offloading_.+foo.+l103]]()
   // CHECK-DAG: [[TID:%.+]] = call i32 @llvm.nvvm.read.ptx.sreg.tid.x()
   // CHECK-DAG: [[NTH:%.+]] = call i32 @llvm.nvvm.read.ptx.sreg.ntid.x()
   // CHECK-DAG: [[WS:%.+]] = call i32 @llvm.nvvm.read.ptx.sreg.warpsize()
@@ -108,7 +109,7 @@ int foo(int n) {
   {
   }
 
-  // CHECK-LABEL: define {{.*}}void {{@__omp_offloading_.+foo.+l179}}_worker()
+  // CHECK-LABEL: define {{.*}}void {{@__omp_offloading_.+foo.+l180}}_worker()
   // CHECK-DAG: [[OMP_EXEC_STATUS:%.+]] = alloca i8,
   // CHECK-DAG: [[OMP_WORK_FN:%.+]] = alloca i8*,
   // CHECK: store i8* null, i8** [[OMP_WORK_FN]],
@@ -139,7 +140,7 @@ int foo(int n) {
   // CHECK: [[EXIT]]
   // CHECK: ret void
 
-  // CHECK: define {{.*}}void 
[[T2:@__omp_offloading_.+foo.+l179]](i[[SZ:32|64]] [[ARG1:%[a-zA-Z_]+]], 
i[[SZ:32|64]] [[ID:%[a-zA-Z_]+]])
+  // CHECK: define {{.*}}void 
[[T2:@__omp_offloading_.+foo.+l180]](i[[SZ:32|64]] [[ARG1:%[a-zA-Z_]+]], 
i[[SZ:32|64]] [[ID:%[a-zA-Z_]+]])
   // CHECK: [[AA_ADDR:%.+]] = alloca i[[SZ]],
   // CHECK: store i[[SZ]] [[ARG1]], i[[SZ]]* [[AA_ADDR]],
   // CHECK: [[AA_CADDR:%.+]] = bitcast i[[SZ]]* [[AA_ADDR]] to i16*
@@ -182,7 +183,7 @@ int foo(int n) {
 id = aa;
   }
 
-  // CHECK-LABEL: define {{.*}}void {{@__omp_offloading_.+foo.+l289}}_worker()
+  // CHECK-LABEL: define {{.*}}void {{@__omp_offloading_.+foo.+l290}}_worker()
   // CHECK-DAG: [[OMP_EXEC_STATUS:%.+]] = alloca i8,
   // CHECK-DAG: [[OMP_WORK_FN:%.+]] = alloca i8*,
   // CHECK: store i8* null, i8** [[OMP_WORK_FN]],
@@ -213,7 +214,7 @@ int foo(int n) {
   // CHECK: [[EXIT]]
   // CHECK: ret void
 
-  // CHECK: define {{.*}}void [[T3:@__omp_offloading_.+foo.+l289]](i[[SZ]]
+  // CHECK: define {{.*}}void 

Re: [PATCH] D49265: [Tooling] Add operator== to CompileCommand

[David Blaikie via cfe-commits]

Any chance this can/should be unit tested? (also, in general (though might
not matter in this instance), symmetric operators like == should be
implemented as non-members (though they can still be friends and if they
are, can be defined inline in the class definition as a member could be),
so any implicit conversions apply equally to the LHS as the RHS of the
expression)

On Thu, Jul 12, 2018 at 1:48 PM Simon Marchi via Phabricator via
cfe-commits  wrote:

> simark created this revision.
> Herald added subscribers: cfe-commits, ioeric, ilya-biryukov.
>
> It does the obvious thing of comparing all fields.  This will be needed
> for a clangd patch I have in the pipeline.
>
>
> Repository:
>   rC Clang
>
> https://reviews.llvm.org/D49265
>
> Files:
>   include/clang/Tooling/CompilationDatabase.h
>
>
> Index: include/clang/Tooling/CompilationDatabase.h
> ===
> --- include/clang/Tooling/CompilationDatabase.h
> +++ include/clang/Tooling/CompilationDatabase.h
> @@ -59,6 +59,11 @@
>
>/// The output file associated with the command.
>std::string Output;
> +
> +  bool operator==(const CompileCommand ) const {
> +return Directory == RHS.Directory && Filename == RHS.Filename &&
> +   CommandLine == RHS.CommandLine && Output == RHS.Output;
> +  }
>  };
>
>  /// Interface for compilation databases.
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49265: [Tooling] Add operator== to CompileCommand

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

Any chance this can/should be unit tested? (also, in general (though might
not matter in this instance), symmetric operators like == should be
implemented as non-members (though they can still be friends and if they
are, can be defined inline in the class definition as a member could be),
so any implicit conversions apply equally to the LHS as the RHS of the
expression)


Repository:
  rC Clang

https://reviews.llvm.org/D49265



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

[PATCH] D41458: [libc++][C++17] Elementary string conversions for integral types

[Zhihao Yuan via Phabricator via cfe-commits]

lichray added inline comments.



Comment at: include/charconv:244
+static _LIBCPP_INLINE_VISIBILITY char const*
+read(char const* __p, char const* __ep, type& __a, type& __b)
+{

mclow.lists wrote:
> Same comment as above about `read` and `inner_product` - they need to be 
> "ugly names"
Unlike `traits` which is a template parameter name in the standard, `read` and 
`inner_product` are function names in the standard, which means the users 
cannot make a macro for them (and there is no guarantee about what name you 
make **not** get by including certain headers), so we don't need to use ugly 
names here, am I right?



Comment at: include/charconv:358
+
+auto __gen_digit = [](_Tp __c) {
+return "0123456789abcdefghijklmnopqrstuvwxyz"[__c];

mclow.lists wrote:
> I just want you to reassure me here - this lambda gets inlined, right?
Yes -- I read my code in assembly.



Comment at: include/charconv:359
+auto __gen_digit = [](_Tp __c) {
+return "0123456789abcdefghijklmnopqrstuvwxyz"[__c];
+};

mclow.lists wrote:
> Thinking some more - did this used to do more? Because I don't see why having 
> a lambda here is a benefit, as compared to:
> 
> const char *__digits = "0123456789abcdefghijklmnopqrstuvwxyz";
> 
> and
> *--p = digits[__c];
> 
I use a lambda here because it may do more in the future.  If someone wants to 
let it support non-ASCII platforms, then they only need to make a patch against 
this lambda rather than changing the sites of uses.  After all, there is 
nothing wrong to abstract out anything into a function, I think...


Repository:
  rCXX libc++

https://reviews.llvm.org/D41458



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

[PATCH] D41458: [libc++][C++17] Elementary string conversions for integral types

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added inline comments.



Comment at: include/charconv:359
+auto __gen_digit = [](_Tp __c) {
+return "0123456789abcdefghijklmnopqrstuvwxyz"[__c];
+};

Thinking some more - did this used to do more? Because I don't see why having a 
lambda here is a benefit, as compared to:

const char *__digits = "0123456789abcdefghijklmnopqrstuvwxyz";

and
*--p = digits[__c];



Repository:
  rCXX libc++

https://reviews.llvm.org/D41458



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

[PATCH] D41458: [libc++][C++17] Elementary string conversions for integral types

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added inline comments.



Comment at: include/charconv:244
+static _LIBCPP_INLINE_VISIBILITY char const*
+read(char const* __p, char const* __ep, type& __a, type& __b)
+{

Same comment as above about `read` and `inner_product` - they need to be "ugly 
names"



Comment at: include/charconv:358
+
+auto __gen_digit = [](_Tp __c) {
+return "0123456789abcdefghijklmnopqrstuvwxyz"[__c];

I just want you to reassure me here - this lambda gets inlined, right?


Repository:
  rCXX libc++

https://reviews.llvm.org/D41458



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

[PATCH] D45805: [libcxx] Remove redundant specializations in type_traits.

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a subscriber: mclow.lists.
ldionne added inline comments.



Comment at: include/type_traits:595
-template
-struct __and_<_B0, _B1> : conditional<_B0::value, _B1, _B0>::type {};
-

It is not impossible that this was a compile-time optimization for the 
(probably very common) case where there's only 2 arguments. I'd like to have 
Marshall's input on that, since he seems to have written that code. Otherwise, 
LGTM. Ping @mclow.lists 


Repository:
  rCXX libc++

https://reviews.llvm.org/D45805



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

[PATCH] D41458: [libc++][C++17] Elementary string conversions for integral types

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added inline comments.



Comment at: include/charconv:372
+
+auto __len = __last - __p;
+if (__value != 0 || !__len)

Are you missing an edge case here? What happens if `__last == __first && 
__value == 0`?




Comment at: test/support/charconv_test_helpers.h:40
+constexpr bool
+_fits_in(T, true_type /* non-narrowing*/, ...)
+{

We don't need to use ugly names here in the test suite.



Repository:
  rCXX libc++

https://reviews.llvm.org/D41458



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

[PATCH] D49338: Implement - P0122R7

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added a comment.

Need to add an entry to `include/CMakeLists.txt` as well.


https://reviews.llvm.org/D49338



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

r337185 - Restore "[ThinLTO] Ensure we always select the same function copy to import"

[Teresa Johnson via cfe-commits]

Author: tejohnson
Date: Mon Jul 16 08:30:36 2018
New Revision: 337185

URL: http://llvm.org/viewvc/llvm-project?rev=337185=rev
Log:
Restore "[ThinLTO] Ensure we always select the same function copy to import"

This reverts commit r337082, restoring r337051, since the LLVM side
patch has been restored.

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

Modified: cfe/trunk/lib/CodeGen/BackendUtil.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/BackendUtil.cpp?rev=337185=337184=337185=diff
==
--- cfe/trunk/lib/CodeGen/BackendUtil.cpp (original)
+++ cfe/trunk/lib/CodeGen/BackendUtil.cpp Mon Jul 16 08:30:36 2018
@@ -1127,9 +1127,8 @@ static void runThinLTOBackend(ModuleSumm
 // e.g. record required linkage changes.
 if (Summary->modulePath() == M->getModuleIdentifier())
   continue;
-// Doesn't matter what value we plug in to the map, just needs an entry
-// to provoke importing by thinBackend.
-ImportList[Summary->modulePath()][GUID] = 1;
+// Add an entry to provoke importing by thinBackend.
+ImportList[Summary->modulePath()].insert(GUID);
   }
 
   std::vector> OwnedImports;


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

[PATCH] D49296: [ASTImporter] Fix import of unnamed structs

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 155678.
martong added a comment.

RecordDecl* -> RecordDecl *


Repository:
  rC Clang

https://reviews.llvm.org/D49296

Files:
  lib/AST/ASTStructuralEquivalence.cpp
  unittests/AST/ASTImporterTest.cpp
  unittests/AST/StructuralEquivalenceTest.cpp

Index: unittests/AST/StructuralEquivalenceTest.cpp
===
--- unittests/AST/StructuralEquivalenceTest.cpp
+++ unittests/AST/StructuralEquivalenceTest.cpp
@@ -42,6 +42,21 @@
 return std::make_tuple(D0, D1);
   }
 
+  std::tuple makeTuDecls(
+  const std::string , const std::string , Language Lang) {
+this->Code0 = SrcCode0;
+this->Code1 = SrcCode1;
+ArgVector Args = getBasicRunOptionsForLanguage(Lang);
+
+const char *const InputFileName = "input.cc";
+
+AST0 = tooling::buildASTFromCodeWithArgs(Code0, Args, InputFileName);
+AST1 = tooling::buildASTFromCodeWithArgs(Code1, Args, InputFileName);
+
+return std::make_tuple(AST0->getASTContext().getTranslationUnitDecl(),
+   AST1->getASTContext().getTranslationUnitDecl());
+  }
+
   // Get a pair of node pointers into the synthesized AST from the given code
   // snippets. The same matcher is used for both snippets.
   template 
@@ -62,15 +77,15 @@
 return makeDecls(SrcCode0, SrcCode1, Lang, Matcher);
   }
 
-  bool testStructuralMatch(NamedDecl *D0, NamedDecl *D1) {
+  bool testStructuralMatch(Decl *D0, Decl *D1) {
 llvm::DenseSet> NonEquivalentDecls;
 StructuralEquivalenceContext Ctx(
 D0->getASTContext(), D1->getASTContext(), NonEquivalentDecls,
 StructuralEquivalenceKind::Default, false, false);
 return Ctx.IsStructurallyEquivalent(D0, D1);
   }
 
-  bool testStructuralMatch(std::tuple t) {
+  bool testStructuralMatch(std::tuple t) {
 return testStructuralMatch(get<0>(t), get<1>(t));
   }
 };
@@ -468,6 +483,11 @@
 }
 
 struct StructuralEquivalenceRecordTest : StructuralEquivalenceTest {
+  // FIXME Use a common getRecordDecl with ASTImporterTest.cpp!
+  RecordDecl *getRecordDecl(FieldDecl *FD) {
+auto *ET = cast(FD->getType().getTypePtr());
+return cast(ET->getNamedType().getTypePtr())->getDecl();
+  };
 };
 
 TEST_F(StructuralEquivalenceRecordTest, Name) {
@@ -535,6 +555,71 @@
   EXPECT_TRUE(testStructuralMatch(t));
 }
 
+TEST_F(StructuralEquivalenceRecordTest, UnnamedRecordsShouldBeInequivalent) {
+  auto t = makeTuDecls(
+  R"(
+  struct A {
+struct {
+  struct A *next;
+} entry0;
+struct {
+  struct A *next;
+} entry1;
+  };
+  )",
+  "", Lang_C);
+  auto *TU = get<0>(t);
+  auto *Entry0 =
+  FirstDeclMatcher().match(TU, fieldDecl(hasName("entry0")));
+  auto *Entry1 =
+  FirstDeclMatcher().match(TU, fieldDecl(hasName("entry1")));
+  auto *R0 = getRecordDecl(Entry0);
+  auto *R1 = getRecordDecl(Entry1);
+
+  ASSERT_NE(R0, R1);
+  EXPECT_TRUE(testStructuralMatch(R0, R0));
+  EXPECT_TRUE(testStructuralMatch(R1, R1));
+  EXPECT_FALSE(testStructuralMatch(R0, R1));
+}
+
+TEST_F(StructuralEquivalenceRecordTest,
+   UnnamedRecordsShouldBeInequivalentEvenIfTheSecondIsBeingDefined) {
+  auto Code =
+  R"(
+  struct A {
+struct {
+  struct A *next;
+} entry0;
+struct {
+  struct A *next;
+} entry1;
+  };
+  )";
+  auto t = makeTuDecls(Code, Code, Lang_C);
+
+  auto *FromTU = get<0>(t);
+  auto *Entry1 =
+  FirstDeclMatcher().match(FromTU, fieldDecl(hasName("entry1")));
+
+  auto *ToTU = get<1>(t);
+  auto *Entry0 =
+  FirstDeclMatcher().match(ToTU, fieldDecl(hasName("entry0")));
+  auto *A =
+  FirstDeclMatcher().match(ToTU, recordDecl(hasName("A")));
+  A->startDefinition(); // Set isBeingDefined, getDefinition() will return a
+// nullptr. This may be the case during ASTImport.
+
+  auto *R0 = getRecordDecl(Entry0);
+  auto *R1 = getRecordDecl(Entry1);
+  ASSERT_TRUE(R0);
+  ASSERT_TRUE(R1);
+  ASSERT_NE(R0, R1);
+  EXPECT_TRUE(testStructuralMatch(R0, R0));
+  EXPECT_TRUE(testStructuralMatch(R1, R1));
+  EXPECT_FALSE(testStructuralMatch(R0, R1));
+}
+
+
 TEST_F(StructuralEquivalenceTest, CompareSameDeclWithMultiple) {
   auto t = makeNamedDecls(
   "struct A{ }; struct B{ }; void foo(A a, A b);",
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -273,6 +273,11 @@
   }
 };
 
+template  RecordDecl *getRecordDecl(T *D) {
+  auto *ET = cast(D->getType().getTypePtr());
+  return cast(ET->getNamedType().getTypePtr())->getDecl();
+};
+
 // This class provides generic methods to write tests which can check internal
 // attributes of AST nodes like getPreviousDecl(), isVirtual(), etc. Also,
 // this fixture makes it possible to import from several "From" contexts.
@@ -1755,11 +1760,6 @@
   )",
   

[PATCH] D49376: [NEON] Define half-precision vrnd intrinsics only when available

[Ivan Kosarev via Phabricator via cfe-commits]

kosarev created this revision.
kosarev added reviewers: SjoerdMeijer, jgreenhalgh, rengolin.
kosarev added a project: clang.
Herald added a reviewer: javed.absar.

https://reviews.llvm.org/D49376

Files:
  include/clang/Basic/arm_neon.td
  test/Sema/arm-no-fp16.c


Index: test/Sema/arm-no-fp16.c
===
--- test/Sema/arm-no-fp16.c
+++ test/Sema/arm-no-fp16.c
@@ -9,3 +9,59 @@
 float32x4_t test_vcvt_f32_f16(float16x4_t a) {
   return vcvt_f32_f16(a); // expected-warning{{implicit declaration of 
function 'vcvt_f32_f16'}} expected-error{{returning 'int' from a function with 
incompatible result type 'float32x4_t'}}
 }
+
+float16x4_t test_vrnda_f16(float16x4_t a) {
+  return vrnda_f16(a); // expected-warning{{implicit declaration of function 
'vrnda_f16'}} expected-error{{returning 'int' from a function with incompatible 
result type 'float16x4_t'}}
+}
+
+float16x8_t test_vrndaq_f16(float16x8_t a) {
+  return vrndaq_f16(a); // expected-warning{{implicit declaration of function 
'vrndaq_f16'}} expected-error{{returning 'int' from a function with 
incompatible result type 'float16x8_t'}}
+}
+
+float16x4_t test_vrnd_f16(float16x4_t a) {
+  return vrnd_f16(a); // expected-warning{{implicit declaration of function 
'vrnd_f16'}} expected-error{{returning 'int' from a function with incompatible 
result type 'float16x4_t'}}
+}
+
+float16x8_t test_vrndq_f16(float16x8_t a) {
+  return vrndq_f16(a); // expected-warning{{implicit declaration of function 
'vrndq_f16'}} expected-error{{returning 'int' from a function with incompatible 
result type 'float16x8_t'}}
+}
+
+float16x4_t test_vrndi_f16(float16x4_t a) {
+  return vrndi_f16(a); // expected-warning{{implicit declaration of function 
'vrndi_f16'}} expected-error{{returning 'int' from a function with incompatible 
result type 'float16x4_t'}}
+}
+
+float16x8_t test_vrndiq_f16(float16x8_t a) {
+  return vrndiq_f16(a); // expected-warning{{implicit declaration of function 
'vrndiq_f16'}} expected-error{{returning 'int' from a function with 
incompatible result type 'float16x8_t'}}
+}
+
+float16x4_t test_vrndm_f16(float16x4_t a) {
+  return vrndm_f16(a); // expected-warning{{implicit declaration of function 
'vrndm_f16'}} expected-error{{returning 'int' from a function with incompatible 
result type 'float16x4_t'}}
+}
+
+float16x8_t test_vrndmq_f16(float16x8_t a) {
+  return vrndmq_f16(a); // expected-warning{{implicit declaration of function 
'vrndmq_f16'}} expected-error{{returning 'int' from a function with 
incompatible result type 'float16x8_t'}}
+}
+
+float16x4_t test_vrndn_f16(float16x4_t a) {
+  return vrndn_f16(a); // expected-warning{{implicit declaration of function 
'vrndn_f16'}} expected-error{{returning 'int' from a function with incompatible 
result type 'float16x4_t'}}
+}
+
+float16x8_t test_vrndnq_f16(float16x8_t a) {
+  return vrndnq_f16(a); // expected-warning{{implicit declaration of function 
'vrndnq_f16'}} expected-error{{returning 'int' from a function with 
incompatible result type 'float16x8_t'}}
+}
+
+float16x4_t test_vrndp_f16(float16x4_t a) {
+  return vrndp_f16(a); // expected-warning{{implicit declaration of function 
'vrndp_f16'}} expected-error{{returning 'int' from a function with incompatible 
result type 'float16x4_t'}}
+}
+
+float16x8_t test_vrndpq_f16(float16x8_t a) {
+  return vrndpq_f16(a); // expected-warning{{implicit declaration of function 
'vrndpq_f16'}} expected-error{{returning 'int' from a function with 
incompatible result type 'float16x8_t'}}
+}
+
+float16x4_t test_vrndx_f16(float16x4_t a) {
+  return vrndx_f16(a); // expected-warning{{implicit declaration of function 
'vrndx_f16'}} expected-error{{returning 'int' from a function with incompatible 
result type 'float16x4_t'}}
+}
+
+float16x8_t test_vrndxq_f16(float16x8_t a) {
+  return vrndxq_f16(a); // expected-warning{{implicit declaration of function 
'vrndxq_f16'}} expected-error{{returning 'int' from a function with 
incompatible result type 'float16x8_t'}}
+}
Index: include/clang/Basic/arm_neon.td
===
--- include/clang/Basic/arm_neon.td
+++ include/clang/Basic/arm_neon.td
@@ -1416,12 +1416,14 @@
   def VCVTP_U16: SInst<"vcvtp_u16", "ud", "hQh">;
 
   // Vector rounding
-  def FRINTZH  : SInst<"vrnd",  "dd", "hQh">;
-  def FRINTNH  : SInst<"vrndn", "dd", "hQh">;
-  def FRINTAH  : SInst<"vrnda", "dd", "hQh">;
-  def FRINTPH  : SInst<"vrndp", "dd", "hQh">;
-  def FRINTMH  : SInst<"vrndm", "dd", "hQh">;
-  def FRINTXH  : SInst<"vrndx", "dd", "hQh">;
+  let ArchGuard = "__ARM_ARCH >= 8 && defined(__ARM_FEATURE_DIRECTED_ROUNDING) 
&& defined(__ARM_FEATURE_FP16_VECTOR_ARITHMETIC)" in {
+def FRINTZH  : SInst<"vrnd",  "dd", "hQh">;
+def FRINTNH  : SInst<"vrndn", "dd", "hQh">;
+def FRINTAH  : SInst<"vrnda", "dd", "hQh">;
+def FRINTPH  : SInst<"vrndp", "dd", "hQh">;
+def FRINTMH  : 

[PATCH] D49375: [NEON] Define half-precision vmaxnm intrinsics only when available

[Ivan Kosarev via Phabricator via cfe-commits]

kosarev created this revision.
kosarev added reviewers: SjoerdMeijer, jgreenhalgh, rengolin.
kosarev added a project: clang.
Herald added a reviewer: javed.absar.

https://reviews.llvm.org/D49375

Files:
  include/clang/Basic/arm_neon.td
  test/Sema/arm-no-fp16.c


Index: test/Sema/arm-no-fp16.c
===
--- test/Sema/arm-no-fp16.c
+++ test/Sema/arm-no-fp16.c
@@ -1,4 +1,6 @@
-// RUN: %clang_cc1 -triple thumbv7-none-eabi %s -target-feature +neon 
-target-feature -fp16 -fsyntax-only -verify
+// RUN: %clang_cc1 -triple thumbv7-none-eabi %s -target-feature +neon \
+// RUN:   -fallow-half-arguments-and-returns -target-feature -fp16 \
+// RUN:   -fsyntax-only -verify
 
 #include 
 
@@ -9,3 +11,19 @@
 float32x4_t test_vcvt_f32_f16(float16x4_t a) {
   return vcvt_f32_f16(a); // expected-warning{{implicit declaration of 
function 'vcvt_f32_f16'}} expected-error{{returning 'int' from a function with 
incompatible result type 'float32x4_t'}}
 }
+
+float16x4_t test_vmaxnm_f16(float16x4_t a, float16x4_t b) {
+  return vmaxnm_f16(a, b); // expected-warning{{implicit declaration of 
function 'vmaxnm_f16'}} expected-error{{returning 'int' from a function with 
incompatible result type 'float16x4_t'}}
+}
+
+float16x8_t test_vmaxnmq_f16(float16x8_t a, float16x8_t b) {
+  return vmaxnmq_f16(a, b); // expected-warning{{implicit declaration of 
function 'vmaxnmq_f16'}} expected-error{{returning 'int' from a function with 
incompatible result type 'float16x8_t'}}
+}
+
+float16x4_t test_vminnm_f16(float16x4_t a, float16x4_t b) {
+  return vminnm_f16(a, b); // expected-warning{{implicit declaration of 
function 'vminnm_f16'}} expected-error{{returning 'int' from a function with 
incompatible result type 'float16x4_t'}}
+}
+
+float16x8_t test_vminnmq_f16(float16x8_t a, float16x8_t b) {
+  return vminnmq_f16(a, b); // expected-warning{{implicit declaration of 
function 'vminnmq_f16'}} expected-error{{returning 'int' from a function with 
incompatible result type 'float16x8_t'}}
+}
Index: include/clang/Basic/arm_neon.td
===
--- include/clang/Basic/arm_neon.td
+++ include/clang/Basic/arm_neon.td
@@ -1463,8 +1463,10 @@
   // Max/Min
   def VMAXH : SInst<"vmax", "ddd", "hQh">;
   def VMINH : SInst<"vmin", "ddd", "hQh">;
-  def FMAXNMH   : SInst<"vmaxnm", "ddd", "hQh">;
-  def FMINNMH   : SInst<"vminnm", "ddd", "hQh">;
+  let ArchGuard = "__ARM_ARCH >= 8 && defined(__ARM_FEATURE_NUMERIC_MAXMIN) && 
defined(__ARM_FEATURE_FP16_VECTOR_ARITHMETIC)" in {
+def FMAXNMH   : SInst<"vmaxnm", "ddd", "hQh">;
+def FMINNMH   : SInst<"vminnm", "ddd", "hQh">;
+  }
 
   // Multiplication/Division
   def VMULH : SOpInst<"vmul", "ddd", "hQh", OP_MUL>;


Index: test/Sema/arm-no-fp16.c
===
--- test/Sema/arm-no-fp16.c
+++ test/Sema/arm-no-fp16.c
@@ -1,4 +1,6 @@
-// RUN: %clang_cc1 -triple thumbv7-none-eabi %s -target-feature +neon -target-feature -fp16 -fsyntax-only -verify
+// RUN: %clang_cc1 -triple thumbv7-none-eabi %s -target-feature +neon \
+// RUN:   -fallow-half-arguments-and-returns -target-feature -fp16 \
+// RUN:   -fsyntax-only -verify
 
 #include 
 
@@ -9,3 +11,19 @@
 float32x4_t test_vcvt_f32_f16(float16x4_t a) {
   return vcvt_f32_f16(a); // expected-warning{{implicit declaration of function 'vcvt_f32_f16'}} expected-error{{returning 'int' from a function with incompatible result type 'float32x4_t'}}
 }
+
+float16x4_t test_vmaxnm_f16(float16x4_t a, float16x4_t b) {
+  return vmaxnm_f16(a, b); // expected-warning{{implicit declaration of function 'vmaxnm_f16'}} expected-error{{returning 'int' from a function with incompatible result type 'float16x4_t'}}
+}
+
+float16x8_t test_vmaxnmq_f16(float16x8_t a, float16x8_t b) {
+  return vmaxnmq_f16(a, b); // expected-warning{{implicit declaration of function 'vmaxnmq_f16'}} expected-error{{returning 'int' from a function with incompatible result type 'float16x8_t'}}
+}
+
+float16x4_t test_vminnm_f16(float16x4_t a, float16x4_t b) {
+  return vminnm_f16(a, b); // expected-warning{{implicit declaration of function 'vminnm_f16'}} expected-error{{returning 'int' from a function with incompatible result type 'float16x4_t'}}
+}
+
+float16x8_t test_vminnmq_f16(float16x8_t a, float16x8_t b) {
+  return vminnmq_f16(a, b); // expected-warning{{implicit declaration of function 'vminnmq_f16'}} expected-error{{returning 'int' from a function with incompatible result type 'float16x8_t'}}
+}
Index: include/clang/Basic/arm_neon.td
===
--- include/clang/Basic/arm_neon.td
+++ include/clang/Basic/arm_neon.td
@@ -1463,8 +1463,10 @@
   // Max/Min
   def VMAXH : SInst<"vmax", "ddd", "hQh">;
   def VMINH : SInst<"vmin", "ddd", "hQh">;
-  def FMAXNMH   : SInst<"vmaxnm", "ddd", "hQh">;
-  def FMINNMH   : 

r337172 - [ASTImporter] Changed constant int to unsigned int in test code.

[Balazs Keri via cfe-commits]

Author: balazske
Date: Mon Jul 16 07:05:18 2018
New Revision: 337172

URL: http://llvm.org/viewvc/llvm-project?rev=337172=rev
Log:
[ASTImporter] Changed constant int to unsigned int in test code.

Modified:
cfe/trunk/unittests/AST/ASTImporterTest.cpp

Modified: cfe/trunk/unittests/AST/ASTImporterTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/ASTImporterTest.cpp?rev=337172=337171=337172=diff
==
--- cfe/trunk/unittests/AST/ASTImporterTest.cpp (original)
+++ cfe/trunk/unittests/AST/ASTImporterTest.cpp Mon Jul 16 07:05:18 2018
@@ -2378,7 +2378,7 @@ private:
 auto *ToClass = FirstDeclMatcher().match(
 ToTU, ClassMatcher);
 
-ASSERT_EQ(DeclCounter().match(ToClass, MethodMatcher), 1);
+ASSERT_EQ(DeclCounter().match(ToClass, MethodMatcher), 1u);
 
 {
   CXXMethodDecl *Method =
@@ -2386,7 +2386,7 @@ private:
   ToClass->removeDecl(Method);
 }
 
-ASSERT_EQ(DeclCounter().match(ToClass, MethodMatcher), 0);
+ASSERT_EQ(DeclCounter().match(ToClass, MethodMatcher), 0u);
 
 Decl *ImportedClass = nullptr;
 {


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

[PATCH] D49300: [ASTImporter] Fix poisonous structural equivalence cache

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Hi Aleksei,

Unfortunately, it seems that it is very to hard synthesize a lit or unit test 
for this fix.
I have found this flaw during the CTU analysis of Bitcoin, where the AST was 
immense.

Because of the lack of tests, now I am trying to persuade you based on some 
theory.
Let's consider this simple BFS algorithm from the `s` source:

  void bfs(Graph G, int s)
  {
Queue queue = new Queue();
marked[s] = true; // Mark the source
queue.enqueue(s); // and put it on the queue.
while (!q.isEmpty()) {
  int v = queue.dequeue(); // Remove next vertex from the queue.
  for (int w : G.adj(v))
if (!marked[w]) // For every unmarked adjacent vertex,
{
  marked[w] = true;
  queue.enqueue(w);
}
}
  }

I believe , the structural equivalence check could be implemented as a parallel 
BFS on a pair of graphs.
And, I think that must have been the original approach at the beginning.
Indeed, it has it's queue, which holds pairs of nodes, one from each graph, 
this is the `DeclsToCheck` and it's pair is in `TentativeEquivalences`.
`TentativeEquivalences` also plays the role of the marking (`marked`) 
functionality above, we use it to check whether we've already seen a pair of 
nodes.

We put in the elements into the queue only in the toplevel decl check function:

  static bool IsStructurallyEquivalent(StructuralEquivalenceContext ,
   Decl *D1, Decl *D2);

The `while` loop where we iterate over the children is implemented in 
`Finish()`.
And `Finish` is called only from the two **member** functions which check the 
equivalency of two Decls or two Types. ASTImporter (and other clients) call 
only these functions.

The `static` implementation functions are called from `Finish`, these push the 
children nodes to the queue.
So far so good, this is almost like the BFS.
However, if we let a static implementation function to call `Finish` via 
another **member** function that means we end up with two nested while loops 
each of them working on the same queue. This is wrong and nobody can reason 
about it's doing.

So, now `TentativeEquivalences` plays two roles. It is used to store the second 
half of the decls which we want to compare, plus it plays a role in closing the 
recursion. On a long term, I think, (after this change) we could refactor 
structural equivalency to be more alike to the traditional BFS.


Repository:
  rC Clang

https://reviews.llvm.org/D49300



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