r345862 - Replace two fallthrough annotations after covered switch with unreachable

2018-11-01 Thread Reid Kleckner via cfe-commits
Author: rnk
Date: Thu Nov  1 10:51:48 2018
New Revision: 345862

URL: http://llvm.org/viewvc/llvm-project?rev=345862&view=rev
Log:
Replace two fallthrough annotations after covered switch with unreachable

Both preceding switches handle all possible enumerators, so the
fallthrough is actually unreachable. This strengthens that to an
assertion.

The first instance had a comment from 2010 indicating that fallthrough
was possible, but that was back when we had a unary operator for
offsetof. Now it is its own expression kind, so the annotation was
stale.

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

Modified: cfe/trunk/lib/AST/ExprConstant.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=345862&r1=345861&r2=345862&view=diff
==
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)
+++ cfe/trunk/lib/AST/ExprConstant.cpp Thu Nov  1 10:51:48 2018
@@ -11143,9 +11143,7 @@ static ICEDiag CheckICE(const Expr* E, c
 case UO_Imag:
   return CheckICE(Exp->getSubExpr(), Ctx);
 }
-
-// OffsetOf falls through here.
-LLVM_FALLTHROUGH;
+llvm_unreachable("invalid unary operator class");
   }
   case Expr::OffsetOfExprClass: {
 // Note that per C99, offsetof must be an ICE. And AFAIK, using
@@ -11249,7 +11247,7 @@ static ICEDiag CheckICE(const Expr* E, c
   return Worst(LHSResult, RHSResult);
 }
 }
-LLVM_FALLTHROUGH;
+llvm_unreachable("invalid binary operator kind");
   }
   case Expr::ImplicitCastExprClass:
   case Expr::CStyleCastExprClass:


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


[PATCH] D53985: Use C++11 fallthrough attribute syntax when available and add a break

2018-11-01 Thread Louis Dionne via Phabricator via cfe-commits
ldionne accepted this revision.
ldionne added a comment.
This revision is now accepted and ready to land.

Would it make sense to add the GNU spelling to the attribute in Clang?


https://reviews.llvm.org/D53985



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


[PATCH] D53514: os_log: make buffer size an integer constant expression.

2018-11-01 Thread Eli Friedman via Phabricator via cfe-commits
efriedma accepted this revision.
efriedma added a comment.

LGTM


Repository:
  rC Clang

https://reviews.llvm.org/D53514



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


[PATCH] D53985: Use C++11 fallthrough attribute syntax when available and add a break

2018-11-01 Thread Reid Kleckner via Phabricator via cfe-commits
rnk created this revision.
rnk added reviewers: EricWF, ldionne.
Herald added subscribers: erik.pilkington, christof.

This silences the two -Wimplicit-fallthrough warnings clang finds in
ItaniumDemangle.h in libc++abi.

Clang does not have a GNU attribute spelling for this attribute, so this
is necessary.

I will commit the same change to the LLVM demangler soon.


https://reviews.llvm.org/D53985

Files:
  libcxx/include/__config
  libcxxabi/src/demangle/ItaniumDemangle.h


Index: libcxxabi/src/demangle/ItaniumDemangle.h
===
--- libcxxabi/src/demangle/ItaniumDemangle.h
+++ libcxxabi/src/demangle/ItaniumDemangle.h
@@ -2812,6 +2812,7 @@
   SoFar = make(SSK);
   if (!SoFar)
 return nullptr;
+  break;
 default:
   break;
 }
Index: libcxx/include/__config
===
--- libcxx/include/__config
+++ libcxx/include/__config
@@ -1248,8 +1248,12 @@
 #  define _LIBCPP_DIAGNOSE_ERROR(...)
 #endif
 
-#if __has_attribute(fallthough) || _GNUC_VER >= 700
 // Use a function like macro to imply that it must be followed by a semicolon
+#if __cplusplus > 201402L && __has_cpp_attribute(fallthrough)
+#  define _LIBCPP_FALLTHROUGH() [[fallthrough]]
+#elif __has_cpp_attribute(clang::fallthrough)
+#  define _LIBCPP_FALLTHROUGH() [[clang::fallthrough]]
+#elif __has_attribute(fallthough) || _GNUC_VER >= 700
 #  define _LIBCPP_FALLTHROUGH() __attribute__((__fallthrough__))
 #else
 #  define _LIBCPP_FALLTHROUGH() ((void)0)


Index: libcxxabi/src/demangle/ItaniumDemangle.h
===
--- libcxxabi/src/demangle/ItaniumDemangle.h
+++ libcxxabi/src/demangle/ItaniumDemangle.h
@@ -2812,6 +2812,7 @@
   SoFar = make(SSK);
   if (!SoFar)
 return nullptr;
+  break;
 default:
   break;
 }
Index: libcxx/include/__config
===
--- libcxx/include/__config
+++ libcxx/include/__config
@@ -1248,8 +1248,12 @@
 #  define _LIBCPP_DIAGNOSE_ERROR(...)
 #endif
 
-#if __has_attribute(fallthough) || _GNUC_VER >= 700
 // Use a function like macro to imply that it must be followed by a semicolon
+#if __cplusplus > 201402L && __has_cpp_attribute(fallthrough)
+#  define _LIBCPP_FALLTHROUGH() [[fallthrough]]
+#elif __has_cpp_attribute(clang::fallthrough)
+#  define _LIBCPP_FALLTHROUGH() [[clang::fallthrough]]
+#elif __has_attribute(fallthough) || _GNUC_VER >= 700
 #  define _LIBCPP_FALLTHROUGH() __attribute__((__fallthrough__))
 #else
 #  define _LIBCPP_FALLTHROUGH() ((void)0)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D53955: Fix the issue that not recognizing single acronym with prefix as ObjC property name.

2018-11-01 Thread Yan Zhang via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL345858: Fix the issue that not recognizing single acronym 
with prefix as ObjC property… (authored by Wizard, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D53955

Files:
  clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp
  clang-tools-extra/trunk/test/clang-tidy/objc-property-declaration.m


Index: clang-tools-extra/trunk/test/clang-tidy/objc-property-declaration.m
===
--- clang-tools-extra/trunk/test/clang-tidy/objc-property-declaration.m
+++ clang-tools-extra/trunk/test/clang-tidy/objc-property-declaration.m
@@ -38,9 +38,10 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'wrongFormat_' not 
using lowerCamelCase style or not prefixed in a category, according to the 
Apple Coding Guidelines [objc-property-declaration]
 @property(strong, nonatomic) NSString *URLStr;
 @property(assign, nonatomic) int abc_camelCase;
+@property(strong, nonatomic) NSString *abc_URL;
 @end
 
 @interface Foo ()
 @property(assign, nonatomic) int abc_inClassExtension;
 // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 
'abc_inClassExtension' not using lowerCamelCase style or not prefixed in a 
category, according to the Apple Coding Guidelines [objc-property-declaration]
 @end
\ No newline at end of file
Index: clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp
@@ -201,8 +201,7 @@
 
 void PropertyDeclarationCheck::registerMatchers(MatchFinder *Finder) {
   // this check should only be applied to ObjC sources.
-  if (!getLangOpts().ObjC)
-return;
+  if (!getLangOpts().ObjC) return;
 
   if (IncludeDefaultAcronyms) {
 EscapedAcronyms.reserve(llvm::array_lengthof(DefaultSpecialAcronyms) +
@@ -235,9 +234,9 @@
   auto *DeclContext = MatchedDecl->getDeclContext();
   auto *CategoryDecl = llvm::dyn_cast(DeclContext);
 
-  auto AcronymsRegex =
-  llvm::Regex("^" + AcronymsGroupRegex(EscapedAcronyms) + "$");
-  if (AcronymsRegex.match(MatchedDecl->getName())) {
+  auto SingleAcronymRegex =
+  llvm::Regex("^([a-zA-Z]+_)?" + AcronymsGroupRegex(EscapedAcronyms) + 
"$");
+  if (SingleAcronymRegex.match(MatchedDecl->getName())) {
 return;
   }
   if (CategoryDecl != nullptr &&


Index: clang-tools-extra/trunk/test/clang-tidy/objc-property-declaration.m
===
--- clang-tools-extra/trunk/test/clang-tidy/objc-property-declaration.m
+++ clang-tools-extra/trunk/test/clang-tidy/objc-property-declaration.m
@@ -38,9 +38,10 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'wrongFormat_' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
 @property(strong, nonatomic) NSString *URLStr;
 @property(assign, nonatomic) int abc_camelCase;
+@property(strong, nonatomic) NSString *abc_URL;
 @end
 
 @interface Foo ()
 @property(assign, nonatomic) int abc_inClassExtension;
 // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'abc_inClassExtension' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
 @end
\ No newline at end of file
Index: clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp
@@ -201,8 +201,7 @@
 
 void PropertyDeclarationCheck::registerMatchers(MatchFinder *Finder) {
   // this check should only be applied to ObjC sources.
-  if (!getLangOpts().ObjC)
-return;
+  if (!getLangOpts().ObjC) return;
 
   if (IncludeDefaultAcronyms) {
 EscapedAcronyms.reserve(llvm::array_lengthof(DefaultSpecialAcronyms) +
@@ -235,9 +234,9 @@
   auto *DeclContext = MatchedDecl->getDeclContext();
   auto *CategoryDecl = llvm::dyn_cast(DeclContext);
 
-  auto AcronymsRegex =
-  llvm::Regex("^" + AcronymsGroupRegex(EscapedAcronyms) + "$");
-  if (AcronymsRegex.match(MatchedDecl->getName())) {
+  auto SingleAcronymRegex =
+  llvm::Regex("^([a-zA-Z]+_)?" + AcronymsGroupRegex(EscapedAcronyms) + "$");
+  if (SingleAcronymRegex.match(MatchedDecl->getName())) {
 return;
   }
   if (CategoryDecl != nullptr &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] r345858 - Fix the issue that not recognizing single acronym with prefix as ObjC property name.

2018-11-01 Thread Yan Zhang via cfe-commits
Author: wizard
Date: Thu Nov  1 10:36:18 2018
New Revision: 345858

URL: http://llvm.org/viewvc/llvm-project?rev=345858&view=rev
Log:
Fix the issue that not recognizing single acronym with prefix as ObjC property 
name.

Summary: This will make clang-tidy accept property names like xyz_URL (URL is a 
common acronym).

Reviewers: benhamilton, hokein

Reviewed By: benhamilton

Subscribers: jfb, cfe-commits

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

Modified:
clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp
clang-tools-extra/trunk/test/clang-tidy/objc-property-declaration.m

Modified: clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp?rev=345858&r1=345857&r2=345858&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp Thu 
Nov  1 10:36:18 2018
@@ -201,8 +201,7 @@ PropertyDeclarationCheck::PropertyDeclar
 
 void PropertyDeclarationCheck::registerMatchers(MatchFinder *Finder) {
   // this check should only be applied to ObjC sources.
-  if (!getLangOpts().ObjC)
-return;
+  if (!getLangOpts().ObjC) return;
 
   if (IncludeDefaultAcronyms) {
 EscapedAcronyms.reserve(llvm::array_lengthof(DefaultSpecialAcronyms) +
@@ -235,9 +234,9 @@ void PropertyDeclarationCheck::check(con
   auto *DeclContext = MatchedDecl->getDeclContext();
   auto *CategoryDecl = llvm::dyn_cast(DeclContext);
 
-  auto AcronymsRegex =
-  llvm::Regex("^" + AcronymsGroupRegex(EscapedAcronyms) + "$");
-  if (AcronymsRegex.match(MatchedDecl->getName())) {
+  auto SingleAcronymRegex =
+  llvm::Regex("^([a-zA-Z]+_)?" + AcronymsGroupRegex(EscapedAcronyms) + 
"$");
+  if (SingleAcronymRegex.match(MatchedDecl->getName())) {
 return;
   }
   if (CategoryDecl != nullptr &&

Modified: clang-tools-extra/trunk/test/clang-tidy/objc-property-declaration.m
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/objc-property-declaration.m?rev=345858&r1=345857&r2=345858&view=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/objc-property-declaration.m 
(original)
+++ clang-tools-extra/trunk/test/clang-tidy/objc-property-declaration.m Thu Nov 
 1 10:36:18 2018
@@ -38,6 +38,7 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'wrongFormat_' not 
using lowerCamelCase style or not prefixed in a category, according to the 
Apple Coding Guidelines [objc-property-declaration]
 @property(strong, nonatomic) NSString *URLStr;
 @property(assign, nonatomic) int abc_camelCase;
+@property(strong, nonatomic) NSString *abc_URL;
 @end
 
 @interface Foo ()


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


[PATCH] D52998: [benchmark] Disable exceptions in Microsoft STL

2018-11-01 Thread Elizabeth Andrews via Phabricator via cfe-commits
eandrews updated this revision to Diff 172171.
eandrews added a comment.

@kbobyrev  I apologize if I was unclear in the comments. I was asking if the 
changes proposed in the comments are alright with you since they would involve 
modifying `benchmark/CMakelists.txt` (instead of `llvm/CMakeLists.txt` as 
discussed in mailing list). As Zachary mentioned in comments, `_HAS_EXCEPTIONS` 
should be set to 0 only when exceptions are disabled. Since exception handling 
for benchmarks is handled in `benchmark/CMakeLists.txt`, I think it makes most 
sense to add the definition there. I have now uploaded the proposed change for 
review.

I am still working with my company to figure out the corporate CLA Google's 
benchmark project requires for patch submissions. I can submit the patch 
upstream once that is done. If you would prefer to submit the patch upstream 
yourself, please feel free to do so.

Sorry again for the confusion!


https://reviews.llvm.org/D52998

Files:
  utils/benchmark/CMakeLists.txt


Index: utils/benchmark/CMakeLists.txt
===
--- utils/benchmark/CMakeLists.txt
+++ utils/benchmark/CMakeLists.txt
@@ -99,6 +99,7 @@
   if (NOT BENCHMARK_ENABLE_EXCEPTIONS)
 add_cxx_compiler_flag(-EHs-)
 add_cxx_compiler_flag(-EHa-)
+add_definitions(-D_HAS_EXCEPTIONS=0)
   endif()
   # Link time optimisation
   if (BENCHMARK_ENABLE_LTO)


Index: utils/benchmark/CMakeLists.txt
===
--- utils/benchmark/CMakeLists.txt
+++ utils/benchmark/CMakeLists.txt
@@ -99,6 +99,7 @@
   if (NOT BENCHMARK_ENABLE_EXCEPTIONS)
 add_cxx_compiler_flag(-EHs-)
 add_cxx_compiler_flag(-EHa-)
+add_definitions(-D_HAS_EXCEPTIONS=0)
   endif()
   # Link time optimisation
   if (BENCHMARK_ENABLE_LTO)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D53955: Fix the issue that not recognizing single acronym with prefix as ObjC property name.

2018-11-01 Thread Yan Zhang via Phabricator via cfe-commits
Wizard updated this revision to Diff 172172.
Wizard added a comment.

format change


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53955

Files:
  clang-tidy/objc/PropertyDeclarationCheck.cpp
  test/clang-tidy/objc-property-declaration.m


Index: test/clang-tidy/objc-property-declaration.m
===
--- test/clang-tidy/objc-property-declaration.m
+++ test/clang-tidy/objc-property-declaration.m
@@ -38,9 +38,10 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'wrongFormat_' not 
using lowerCamelCase style or not prefixed in a category, according to the 
Apple Coding Guidelines [objc-property-declaration]
 @property(strong, nonatomic) NSString *URLStr;
 @property(assign, nonatomic) int abc_camelCase;
+@property(strong, nonatomic) NSString *abc_URL;
 @end
 
 @interface Foo ()
 @property(assign, nonatomic) int abc_inClassExtension;
 // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 
'abc_inClassExtension' not using lowerCamelCase style or not prefixed in a 
category, according to the Apple Coding Guidelines [objc-property-declaration]
 @end
\ No newline at end of file
Index: clang-tidy/objc/PropertyDeclarationCheck.cpp
===
--- clang-tidy/objc/PropertyDeclarationCheck.cpp
+++ clang-tidy/objc/PropertyDeclarationCheck.cpp
@@ -201,8 +201,7 @@
 
 void PropertyDeclarationCheck::registerMatchers(MatchFinder *Finder) {
   // this check should only be applied to ObjC sources.
-  if (!getLangOpts().ObjC)
-return;
+  if (!getLangOpts().ObjC) return;
 
   if (IncludeDefaultAcronyms) {
 EscapedAcronyms.reserve(llvm::array_lengthof(DefaultSpecialAcronyms) +
@@ -235,9 +234,9 @@
   auto *DeclContext = MatchedDecl->getDeclContext();
   auto *CategoryDecl = llvm::dyn_cast(DeclContext);
 
-  auto AcronymsRegex =
-  llvm::Regex("^" + AcronymsGroupRegex(EscapedAcronyms) + "$");
-  if (AcronymsRegex.match(MatchedDecl->getName())) {
+  auto SingleAcronymRegex =
+  llvm::Regex("^([a-zA-Z]+_)?" + AcronymsGroupRegex(EscapedAcronyms) + 
"$");
+  if (SingleAcronymRegex.match(MatchedDecl->getName())) {
 return;
   }
   if (CategoryDecl != nullptr &&


Index: test/clang-tidy/objc-property-declaration.m
===
--- test/clang-tidy/objc-property-declaration.m
+++ test/clang-tidy/objc-property-declaration.m
@@ -38,9 +38,10 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'wrongFormat_' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
 @property(strong, nonatomic) NSString *URLStr;
 @property(assign, nonatomic) int abc_camelCase;
+@property(strong, nonatomic) NSString *abc_URL;
 @end
 
 @interface Foo ()
 @property(assign, nonatomic) int abc_inClassExtension;
 // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'abc_inClassExtension' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
 @end
\ No newline at end of file
Index: clang-tidy/objc/PropertyDeclarationCheck.cpp
===
--- clang-tidy/objc/PropertyDeclarationCheck.cpp
+++ clang-tidy/objc/PropertyDeclarationCheck.cpp
@@ -201,8 +201,7 @@
 
 void PropertyDeclarationCheck::registerMatchers(MatchFinder *Finder) {
   // this check should only be applied to ObjC sources.
-  if (!getLangOpts().ObjC)
-return;
+  if (!getLangOpts().ObjC) return;
 
   if (IncludeDefaultAcronyms) {
 EscapedAcronyms.reserve(llvm::array_lengthof(DefaultSpecialAcronyms) +
@@ -235,9 +234,9 @@
   auto *DeclContext = MatchedDecl->getDeclContext();
   auto *CategoryDecl = llvm::dyn_cast(DeclContext);
 
-  auto AcronymsRegex =
-  llvm::Regex("^" + AcronymsGroupRegex(EscapedAcronyms) + "$");
-  if (AcronymsRegex.match(MatchedDecl->getName())) {
+  auto SingleAcronymRegex =
+  llvm::Regex("^([a-zA-Z]+_)?" + AcronymsGroupRegex(EscapedAcronyms) + "$");
+  if (SingleAcronymRegex.match(MatchedDecl->getName())) {
 return;
   }
   if (CategoryDecl != nullptr &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D53955: Fix the issue that not recognizing single acronym with prefix as ObjC property name.

2018-11-01 Thread Yan Zhang via Phabricator via cfe-commits
Wizard updated this revision to Diff 172169.
Wizard added a comment.

Format code.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53955

Files:
  clang-tidy/objc/PropertyDeclarationCheck.cpp
  test/clang-tidy/objc-property-declaration.m

Index: test/clang-tidy/objc-property-declaration.m
===
--- test/clang-tidy/objc-property-declaration.m
+++ test/clang-tidy/objc-property-declaration.m
@@ -38,9 +38,10 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'wrongFormat_' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
 @property(strong, nonatomic) NSString *URLStr;
 @property(assign, nonatomic) int abc_camelCase;
+@property(strong, nonatomic) NSString *abc_URL;
 @end
 
 @interface Foo ()
 @property(assign, nonatomic) int abc_inClassExtension;
 // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'abc_inClassExtension' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
 @end
\ No newline at end of file
Index: clang-tidy/objc/PropertyDeclarationCheck.cpp
===
--- clang-tidy/objc/PropertyDeclarationCheck.cpp
+++ clang-tidy/objc/PropertyDeclarationCheck.cpp
@@ -39,92 +39,16 @@
 ///
 /// Keep this list sorted.
 constexpr llvm::StringLiteral DefaultSpecialAcronyms[] = {
-"[2-9]G",
-"ACL",
-"API",
-"APN",
-"APNS",
-"AR",
-"ARGB",
-"ASCII",
-"AV",
-"BGRA",
-"CA",
-"CDN",
-"CF",
-"CG",
-"CI",
-"CRC",
-"CV",
-"CMYK",
-"DNS",
-"FPS",
-"FTP",
-"GIF",
-"GL",
-"GPS",
-"GUID",
-"HD",
-"HDR",
-"HMAC",
-"HTML",
-"HTTP",
-"HTTPS",
-"HUD",
-"ID",
-"JPG",
-"JS",
-"JSON",
-"LAN",
-"LZW",
-"LTR",
-"MAC",
-"MD",
-"MDNS",
-"MIDI",
-"NS",
-"OS",
-"P2P",
-"PDF",
-"PIN",
-"PNG",
-"POI",
-"PSTN",
-"PTR",
-"QA",
-"QOS",
-"RGB",
-"RGBA",
-"RGBX",
-"RIPEMD",
-"ROM",
-"RPC",
-"RTF",
-"RTL",
-"SC",
-"SDK",
-"SHA",
-"SQL",
-"SSO",
-"TCP",
-"TIFF",
-"TOS",
-"TTS",
-"UI",
-"URI",
-"URL",
-"UUID",
-"VC",
-"VO",
-"VOIP",
-"VPN",
-"VR",
-"W",
-"WAN",
-"X",
-"XML",
-"Y",
-"Z",
+"[2-9]G", "ACL",  "API",  "APN","APNS", "AR",   "ARGB", "ASCII", "AV",
+"BGRA",   "CA",   "CDN",  "CF", "CG",   "CI",   "CRC",  "CV","CMYK",
+"DNS","FPS",  "FTP",  "GIF","GL",   "GPS",  "GUID", "HD","HDR",
+"HMAC",   "HTML", "HTTP", "HTTPS",  "HUD",  "ID",   "JPG",  "JS","JSON",
+"LAN","LZW",  "LTR",  "MAC","MD",   "MDNS", "MIDI", "NS","OS",
+"P2P","PDF",  "PIN",  "PNG","POI",  "PSTN", "PTR",  "QA","QOS",
+"RGB","RGBA", "RGBX", "RIPEMD", "ROM",  "RPC",  "RTF",  "RTL",   "SC",
+"SDK","SHA",  "SQL",  "SSO","TCP",  "TIFF", "TOS",  "TTS",   "UI",
+"URI","URL",  "UUID", "VC", "VO",   "VOIP", "VPN",  "VR","W",
+"WAN","X","XML",  "Y",  "Z",
 };
 
 /// For now we will only fix 'CamelCase' or 'abc_CamelCase' property to
@@ -201,8 +125,7 @@
 
 void PropertyDeclarationCheck::registerMatchers(MatchFinder *Finder) {
   // this check should only be applied to ObjC sources.
-  if (!getLangOpts().ObjC)
-return;
+  if (!getLangOpts().ObjC) return;
 
   if (IncludeDefaultAcronyms) {
 EscapedAcronyms.reserve(llvm::array_lengthof(DefaultSpecialAcronyms) +
@@ -235,9 +158,9 @@
   auto *DeclContext = MatchedDecl->getDeclContext();
   auto *CategoryDecl = llvm::dyn_cast(DeclContext);
 
-  auto AcronymsRegex =
-  llvm::Regex("^" + AcronymsGroupRegex(EscapedAcronyms) + "$");
-  if (AcronymsRegex.match(MatchedDecl->getName())) {
+  auto SingleAcronymRegex =
+  llvm::Regex("^([a-zA-Z]+_)?" + AcronymsGroupRegex(EscapedAcronyms) + "$");
+  if (SingleAcronymRegex.match(MatchedDecl->getName())) {
 return;
   }
   if (CategoryDecl != nullptr &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D52296: [Clang] - Add -gsingle-file-split-dwarf option.

2018-11-01 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment.

In https://reviews.llvm.org/D52296#1283691, @grimar wrote:

> Nice :) 
>  So seems the last unresolved question left is the naming of the new option?
>  If we do not want to go with `-gsingle-file-split-dwarf` then what it should 
> be?
>
> What about `-fdebug-fission` as an alias for `-gsplit-dwarf`.
>  And `-fsingle-file-debug-fission` for the new option?
>
> Or, may be:
>
> `-fdebug-fission` for the new option and then
>  `-fdebug-fission -fdebug-split-dwo` would work together as `-gsplit-dwarf`?


Only DWARF supports this, correct?  So I would suggest: 
`-fdwarf-fission[={split,single}]` where the parameter defaults to `split`. Is 
there any particular value in having two separate options?


https://reviews.llvm.org/D52296



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


[PATCH] D53697: [ASTImporter][Structural Eq] Check for isBeingDefined

2018-11-01 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik added a comment.

@martong Hello, if I look at the specific test that was failing 
`TestDataFormatterLibcxxVector.py` it only has the following decorator 
`@add_test_categories(["libc++"])` so that should not prevent it from running 
on Linux if you are also building libc++. If you do a local cmake build and you 
build libc++ as well does it still skip the test?


Repository:
  rC Clang

https://reviews.llvm.org/D53697



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


[PATCH] D53979: [analyzer][CTU] Correctly signal in the function index generation tool if there was an error

2018-11-01 Thread Umann Kristóf via Phabricator via cfe-commits
Szelethus accepted this revision.
Szelethus added a comment.

Makes sense!


https://reviews.llvm.org/D53979



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


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

2018-11-01 Thread Bevin Hansson via Phabricator via cfe-commits
ebevhan added a comment.

In https://reviews.llvm.org/D53738#1283983, @rjmccall wrote:

> In https://reviews.llvm.org/D53738#1283861, @ebevhan wrote:
>
> > In https://reviews.llvm.org/D53738#1283459, @rjmccall wrote:
> >
> > > Well, it could be passed around through most code as some sort of 
> > > abstractly-represented intermediate "type" which could be either a 
> > > QualType or a fixed-point semantics.
> >
> >
> > Sounds to me like you're describing a new `Type` that can contain an 
> > arbitrary fixed-point semantic :)
>
>
> The fact that it doesn't exist in the modeled type system is important.
>
> The arbitrary-type overflow intrinsics have this same problem, and we did not 
> try to solve it by creating a new type class for arbitrary-precision integers 
> and promoting the arguments.


Okay, I had a look at how those were emitted, I wasn't familiar with it until 
now. That's certainly a way of approaching this implementation as well, though 
I think the full precision info should still be in the AST somewhere rather 
than implied from the operand types until CodeGen.

> 
> 
>> It still feels like this just introduces inconsistencies into the form of 
>> the AST. If we do add this extra type object to the BO, won't people wonder 
>> why we use ImplicitCastExpr for non-fixedpoint operations but use the 
>> special `QualTypeOrFPSemantics BinaryOperator::ComputationType;` for 
>> fixedpoint operations even though they both have nearly the same semantic 
>> meaning (converting to a common type before doing the operation)?
> 
> 
> 
> 1. You would have an inconsistency in either case, since e.g. numeric + 
> otherwise always returns the same type as its operands, but this would not.

True, but the CompoundAssignOperator already has that inconsistency with 
ComputationResultType.

> 2. The question is easily answered by pointing at the language spec.  The 
> language does not say that the operands are promoted to a common type; it 
> says the result is determined numerically from the true numeric values of the 
> operands.

I guess. I just see that as a behavioral specification and not necessarily an 
implementation detail. It's perfectly acceptable to implement it as promotion 
to a common type (obviously, as that's what we are going to do), and I don't 
really see this as the spec putting any kind of requirement on how the 
implementation should be done.

> 
> 
 It might just be easier to store the full-precision info in BO directly. 
 BO might be too common to warrant the size increase, though. 
 FixedPointSemantics can probably be optimized to only take 32 bits.
>>> 
>>> What you can definitely do is store a bit in BO saying that there's extra 
>>> storage for the intermediate "type".
>> 
>> Is this similar to how ExtQuals works? How would this be implemented?
> 
> Take a look at how `DeclRefExpr` stores its various optional components.

`TrailingObjects`, then. That certainly wouldn't work if CompoundAssignOperator 
is still a subclass of BinaryOperator, so it would have to be folded in that 
case.

So the implementation would be with a `TrailingObjects` where it can have 2 QualTypes and 1 
FixedPointSemantics, the QualTypes being subsumed from CompoundAssignOperator.

Probably still quite a hefty amount of code that would have to be updated to 
make this change.

> 
> 
 As a side note, comparisons are still a bit up in the air. I don't think 
 we came to a conclusion on whether they should be done in full precision 
 or bitwise. The spec isn't clear.
>>> 
>>> ...bitwise?
>> 
>> The spec uses different wording for the arithmetic operations and comparison 
>> operations, and it's not entirely obvious what it means. For the arithmetic 
>> operators, it has the whole section on finding the full precision common 
>> type, but for comparisons it says:
>> 
>>> When comparing fixed-point values with fixed-point values or integer values,
>>>  the values are compared directly; the values of the operands are not 
>>> converted before the
>>>  comparison is made.
>> 
>> What 'directly' means in conjunction with 'the operands are not converted' 
>> is not clear. It's reasonable to assume that it either means comparing 
>> value-wise (so, the same as finding a common type that fits all values and 
>> then comparing; though this would obviously require conversion), or perhaps 
>> 'directly' means a bitwise (representation) comparison. The latter seems a 
>> bit farfetched to me, though.
> 
> I think this is just like fixed-point arithmetic: you should do an exact 
> numeric comparison, but there's no formal conversion because it would have to 
> be a conversion to some concrete type, which could leave to (mandatory!) 
> inexactness when there's no common type that expresses the full range of both 
> operands.

This is probably the correct interpretation, I just think it could have been 
stated a bit clearer that they pretty much do mean the same thing for the 
comparison operators as for th

[PATCH] D53984: [mips] Fix broken MSA test

2018-11-01 Thread Aleksandar Beserminji via Phabricator via cfe-commits
abeserminji created this revision.
abeserminji added reviewers: atanasyan, smaksimovic, petarj, mstojanovic.
Herald added subscribers: jrtc27, arichardson.

Test //builtins-mips-msa-error.c// wasn't reporting errors.
This patch fixes the test, so further test cases can be added.


Repository:
  rC Clang

https://reviews.llvm.org/D53984

Files:
  test/CodeGen/builtins-mips-msa-error.c

Index: test/CodeGen/builtins-mips-msa-error.c
===
--- test/CodeGen/builtins-mips-msa-error.c
+++ test/CodeGen/builtins-mips-msa-error.c
@@ -1,18 +1,22 @@
 // REQUIRES: mips-registered-target
-// RUN: not %clang_cc1 -triple mips-unknown-linux-gnu -fsyntax-only %s \
+// RUN: %clang_cc1 -triple mips-unknown-linux-gnu -fsyntax-only %s \
 // RUN:-target-feature +msa -target-feature +fp64 \
-// RUN:-mfloat-abi hard -o - 2>&1 | FileCheck %s
+// RUN:-verify -mfloat-abi hard -o - 2>&1
 
 #include 
 
 void test(void) {
   v16i8 v16i8_a = (v16i8) {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15};
+  v16i8 v16i8_b = (v16i8) {16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31};
   v16i8 v16i8_r;
   v8i16 v8i16_a = (v8i16) {0, 1, 2, 3, 4, 5, 6, 7};
+  v8i16 v8i16_b = (v8i16) {8, 9, 10, 11, 12, 13, 14, 15};
   v8i16 v8i16_r;
   v4i32 v4i32_a = (v4i32) {0, 1, 2, 3};
+  v4i32 v4i32_b = (v4i32) {4, 5, 6, 7};
   v4i32 v4i32_r;
   v2i64 v2i64_a = (v2i64) {0, 1};
+  v2i64 v2i64_b = (v2i64) {3, 4};
   v2i64 v2i64_r;
 
   v16u8 v16u8_a = (v16u8) {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15};
@@ -24,388 +28,385 @@
   v2u64 v2u64_a = (v2u64) {0, 1};
   v2u64 v2u64_r;
 
-
   int int_r;
   long long ll_r;
 
-  v16u8_r = __msa_addvi_b(v16u8_a, 32);  // expected-error {{argument should be a value from 0 to 31}}
-  v8u16_r = __msa_addvi_h(v8u16_a, 32);  // expected-error {{argument should be a value from 0 to 31}}
-  v4u32_r = __msa_addvi_w(v4u32_a, 32);  // expected-error {{argument should be a value from 0 to 31}}
-  v2u64_r = __msa_addvi_d(v2u64_a, 32);  // expected-error {{argument should be a value from 0 to 31}}
-
-  v16i8_r = __msa_andi_b(v16i8_a, 256);  // CHECK: warning: argument should be a value from 0 to 255}}
-  v8i16_r = __msa_andi_b(v8i16_a, 256);  // CHECK: warning: argument should be a value from 0 to 255}}
-  v4i32_r = __msa_andi_b(v4i32_a, 256);  // CHECK: warning: argument should be a value from 0 to 255}}
-  v2i64_r = __msa_andi_b(v2i64_a, 256);  // CHECK: warning: argument should be a value from 0 to 255}}
-
-  v16i8_r = __msa_bclri_b(v16i8_a, 8);   // expected-error {{argument should be a value from 0 to 7}}
-  v8i16_r = __msa_bclri_h(v8i16_a, 16);  // expected-error {{argument should be a value from 0 to 15}}
-  v4i32_r = __msa_bclri_w(v4i32_a, 33);  // expected-error {{argument should be a value from 0 to 31}}
-  v2i64_r = __msa_bclri_d(v2i64_a, 64);  // expected-error {{argument should be a value from 0 to 63}}
-
-  v16i8_r = __msa_binsli_b(v16i8_r, v16i8_a, 8); // expected-error {{argument should be a value from 0 to 7}}
-  v8i16_r = __msa_binsli_h(v8i16_r, v8i16_a, 16);// expected-error {{argument should be a value from 0 to 15}}
-  v4i32_r = __msa_binsli_w(v4i32_r, v4i32_a, 32);// expected-error {{argument should be a value from 0 to 31}}
-  v2i64_r = __msa_binsli_d(v2i64_r, v2i64_a, 64);// expected-error {{argument should be a value from 0 to 63}}
-
-  v16i8_r = __msa_binsri_b(v16i8_r, v16i8_a, 8); // expected-error {{argument should be a value from 0 to 7}}
-  v8i16_r = __msa_binsri_h(v8i16_r, v8i16_a, 16);// expected-error {{argument should be a value from 0 to 15}}
-  v4i32_r = __msa_binsri_w(v4i32_r, v4i32_a, 32);// expected-error {{argument should be a value from 0 to 31}}
-  v2i64_r = __msa_binsri_d(v2i64_r, v2i64_a, 64);// expected-error {{argument should be a value from 0 to 63}}
-
-  v16i8_r = __msa_bmnzi_b(v16i8_r, v16i8_a, 256);// expected-error {{argument should be a value from 0 to 255}}
-
-  v16i8_r = __msa_bmzi_b(v16i8_r, v16i8_a, 256); // expected-error {{argument should be a value from 0 to 255}}
-
-  v16i8_r = __msa_bnegi_b(v16i8_a, 8);   // expected-error {{argument should be a value from 0 to 7}}
-  v8i16_r = __msa_bnegi_h(v8i16_a, 16);  // expected-error {{argument should be a value from 0 to 15}}
-  v4i32_r = __msa_bnegi_w(v4i32_a, 32);  // expected-error {{argument should be a value from 0 to 31}}
-  v2i64_r = __msa_bnegi_d(v2i64_a, 64);  // expected-error {{argument should be a value from 0 to 63}}
-
-  v16i8_r = __msa_bseli_b(v16i8_r, v16i8_a, 256);// expected-error {{argument should be a value from 0 to 255}}
-
-  v16i8_r = __msa_bseti_b(v16i8_a, 8);   // expected-error {{argument should be a value from 0 to 7}}
-  v8i16_r = __msa_bs

[PATCH] D53974: [clang-tidy] new check: bugprone-too-small-loop-variable

2018-11-01 Thread Tamás Zolnai via Phabricator via cfe-commits
ztamas added a comment.

In https://reviews.llvm.org/D53974#1284000, @JonasToth wrote:

> For general understanding: Couldn't this check be generalized to comparing 
> integers of different sizes? We tried a 'dont-mix-int-types' check for 
> arithmetic already, its complicated :)
>  But this as a specialization of the category could be done easier (i think).
>
> What do you think?


I don't think so. This comparison is suspicious only inside a loop, not in 
general.
For example see this code:

  long size = 30;
  short index = 100;
  
  if(index < size) {
   // 
  }

You can choose the two values as you want, this comparison will work correctly.
However in a loop condition this comparison means a problem, because the loop 
stops only if the "index" variable gets bigger than the "size" variable.
So the loop context is important here.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53974



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


[PATCH] D53514: os_log: make buffer size an integer constant expression.

2018-11-01 Thread Tim Northover via Phabricator via cfe-commits
t.p.northover updated this revision to Diff 172155.
t.p.northover added a comment.
Herald added a subscriber: mgorny.

Same as previous patch except the OSLog helpers are moved to libclangAST to 
respect dependencies.


Repository:
  rC Clang

https://reviews.llvm.org/D53514

Files:
  clang/include/clang/AST/OSLog.h
  clang/include/clang/Analysis/Analyses/OSLog.h
  clang/lib/AST/CMakeLists.txt
  clang/lib/AST/ExprConstant.cpp
  clang/lib/AST/OSLog.cpp
  clang/lib/Analysis/CMakeLists.txt
  clang/lib/Analysis/OSLog.cpp
  clang/lib/Analysis/PrintfFormatString.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/builtins.c

Index: clang/test/CodeGen/builtins.c
===
--- clang/test/CodeGen/builtins.c
+++ clang/test/CodeGen/builtins.c
@@ -729,25 +729,28 @@
 
 // CHECK-LABEL: define void @test_builtin_os_log_errno
 void test_builtin_os_log_errno() {
-  // CHECK: %[[VLA:.*]] = alloca i8, i64 4, align 16
-  // CHECK: call void @__os_log_helper_16_2_1_0_96(i8* %[[VLA]])
+  // CHECK-NOT: @stacksave
+  // CHECK: %[[BUF:.*]] = alloca [4 x i8], align 1
+  // CHECK: %[[DECAY:.*]] = getelementptr inbounds [4 x i8], [4 x i8]* %[[BUF]], i32 0, i32 0
+  // CHECK: call void @__os_log_helper_1_2_1_0_96(i8* %[[DECAY]])
+  // CHECK-NOT: @stackrestore
 
   char buf[__builtin_os_log_format_buffer_size("%m")];
   __builtin_os_log_format(buf, "%m");
 }
 
-// CHECK-LABEL: define linkonce_odr hidden void @__os_log_helper_16_2_1_0_96
+// CHECK-LABEL: define linkonce_odr hidden void @__os_log_helper_1_2_1_0_96
 // CHECK: (i8* %[[BUFFER:.*]])
 
 // CHECK: %[[BUFFER_ADDR:.*]] = alloca i8*, align 8
 // CHECK: store i8* %[[BUFFER]], i8** %[[BUFFER_ADDR]], align 8
 // CHECK: %[[BUF:.*]] = load i8*, i8** %[[BUFFER_ADDR]], align 8
 // CHECK: %[[SUMMARY:.*]] = getelementptr i8, i8* %[[BUF]], i64 0
-// CHECK: store i8 2, i8* %[[SUMMARY]], align 16
+// CHECK: store i8 2, i8* %[[SUMMARY]], align 1
 // CHECK: %[[NUMARGS:.*]] = getelementptr i8, i8* %[[BUF]], i64 1
 // CHECK: store i8 1, i8* %[[NUMARGS]], align 1
 // CHECK: %[[ARGDESCRIPTOR:.*]] = getelementptr i8, i8* %[[BUF]], i64 2
-// CHECK: store i8 96, i8* %[[ARGDESCRIPTOR]], align 2
+// CHECK: store i8 96, i8* %[[ARGDESCRIPTOR]], align 1
 // CHECK: %[[ARGSIZE:.*]] = getelementptr i8, i8* %[[BUF]], i64 3
 // CHECK: store i8 0, i8* %[[ARGSIZE]], align 1
 // CHECK-NEXT: ret void
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -21,7 +21,7 @@
 #include "TargetInfo.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Decl.h"
-#include "clang/Analysis/Analyses/OSLog.h"
+#include "clang/AST/OSLog.h"
 #include "clang/Basic/TargetBuiltins.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/CodeGen/CGFunctionInfo.h"
@@ -3609,13 +3609,6 @@
   case Builtin::BI__builtin_os_log_format:
 return emitBuiltinOSLogFormat(*E);
 
-  case Builtin::BI__builtin_os_log_format_buffer_size: {
-analyze_os_log::OSLogBufferLayout Layout;
-analyze_os_log::computeOSLogBufferLayout(CGM.getContext(), E, Layout);
-return RValue::get(ConstantInt::get(ConvertType(E->getType()),
-Layout.size().getQuantity()));
-  }
-
   case Builtin::BI__xray_customevent: {
 if (!ShouldXRayInstrumentFunction())
   return RValue::getIgnored();
Index: clang/lib/Analysis/PrintfFormatString.cpp
===
--- clang/lib/Analysis/PrintfFormatString.cpp
+++ clang/lib/Analysis/PrintfFormatString.cpp
@@ -13,7 +13,7 @@
 //===--===//
 
 #include "clang/Analysis/Analyses/FormatString.h"
-#include "clang/Analysis/Analyses/OSLog.h"
+#include "clang/AST/OSLog.h"
 #include "FormatStringParsing.h"
 #include "clang/Basic/TargetInfo.h"
 
Index: clang/lib/Analysis/CMakeLists.txt
===
--- clang/lib/Analysis/CMakeLists.txt
+++ clang/lib/Analysis/CMakeLists.txt
@@ -18,7 +18,6 @@
   ExprMutationAnalyzer.cpp
   FormatString.cpp
   LiveVariables.cpp
-  OSLog.cpp
   ObjCNoReturn.cpp
   PostOrderCFGView.cpp
   PrintfFormatString.cpp
Index: clang/lib/AST/OSLog.cpp
===
--- clang/lib/AST/OSLog.cpp
+++ clang/lib/AST/OSLog.cpp
@@ -1,6 +1,6 @@
 // TODO: header template
 
-#include "clang/Analysis/Analyses/OSLog.h"
+#include "clang/AST/OSLog.h"
 #include "clang/AST/Attr.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -39,6 +39,7 @@
 #include "clang/AST/ASTLambda.h"
 #include "clang/AST/CharUnits.h"
 #include "clang/AST/Expr.h"
+#include "clang/AST/OSLog.h"
 #include "clan

[PATCH] D53982: Output "rule" information in SARIF

2018-11-01 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov requested changes to this revision.
george.karpenkov added a comment.
This revision now requires changes to proceed.

Minor nit: let's create a custom substitution for your diff command, like 
`diff_plist`.
Otherwise LGTM.




Comment at: test/Analysis/diagnostics/sarif-multi-diagnostic-test.c:1
+// RUN: %clang_analyze_cc1 
-analyzer-checker=core,alpha.security.taint,debug.TaintTest %s -verify 
-analyzer-output=sarif -o - | diff -U1 -w -I 
".*file:.*sarif-multi-diagnostic-test.c" -I '"version":' -I 
"2\.0\.0\-csd\.[0-9]*\.beta\." - 
%S/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
+#include "../Inputs/system-header-simulator.h"

In order to avoid duplication in `diff` commands between the tests, it might be 
better to define a custom command in `lit.cfg`, as we did for `diff_plist`
(alternatively, create a custom flag to generate stable output)


https://reviews.llvm.org/D53982



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


[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

2018-11-01 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment.

Since I don't think we have any false positives here, maybe it would be useful 
to add it to -Wall or -Wextra?


https://reviews.llvm.org/D52835



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


[PATCH] D53979: [analyzer][CTU] Correctly signal in the function index generation tool if there was an error

2018-11-01 Thread Gabor Marton via Phabricator via cfe-commits
martong accepted this revision.
martong added a comment.
This revision is now accepted and ready to land.

Looks good to me. It is great to see that we can get rid of so many header and 
lib dependencies.


https://reviews.llvm.org/D53979



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


[PATCH] D53514: os_log: make buffer size an integer constant expression.

2018-11-01 Thread Tim Northover via Phabricator via cfe-commits
t.p.northover reopened this revision.
t.p.northover added a comment.
This revision is now accepted and ready to land.

Turns out I neglected the layering between libclangAST and libclangAnalysis so 
I've reverted for now. For this to work I think we need to move the OSLog 
helpers into libclangAST. I'll put together a new patch and upload it.


Repository:
  rC Clang

https://reviews.llvm.org/D53514



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


[PATCH] D53974: [clang-tidy] new check: bugprone-too-small-loop-variable

2018-11-01 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment.

For general understanding: Couldn't this check be generalized to comparing 
integers of different sizes? We tried a 'dont-mix-int-types' check for 
arithmetic already, its complicated :)
But this as a specialization of the category could be done easier (i think).

What do you think?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53974



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


[PATCH] D52615: Handle -fsanitize-address-poison-class-member-array-new-cookie in the driver and propagate it to cc1

2018-11-01 Thread Filipe Cabecinhas via Phabricator via cfe-commits
filcab updated this revision to Diff 172149.
filcab added a comment.

Expanded a bit more on the documentation, mentioning that user code might be 
technically allowed to access those array cookies, but that users might not 
want to allow it.


Repository:
  rC Clang

https://reviews.llvm.org/D52615

Files:
  docs/ClangCommandLineReference.rst
  docs/UsersManual.rst
  include/clang/Driver/Options.td
  include/clang/Driver/SanitizerArgs.h
  include/clang/Frontend/CodeGenOptions.def
  lib/CodeGen/ItaniumCXXABI.cpp
  lib/Driver/SanitizerArgs.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/address-sanitizer-and-array-cookie.cpp
  test/Driver/fsanitize.c

Index: test/Driver/fsanitize.c
===
--- test/Driver/fsanitize.c
+++ test/Driver/fsanitize.c
@@ -208,6 +208,24 @@
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-ASAN-WITHOUT-USE-AFTER-SCOPE
 // CHECK-ASAN-WITHOUT-USE-AFTER-SCOPE: -cc1{{.*}}address-use-after-scope
 
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=address -fsanitize-address-poison-custom-array-cookie %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE
+// RUN: %clang_cl --target=x86_64-windows -fsanitize=address -fsanitize-address-poison-custom-array-cookie -### -- %s 2>&1 | FileCheck %s --check-prefix=CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE
+// CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE: -cc1{{.*}}-fsanitize-address-poison-custom-array-cookie
+
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=address -fno-sanitize-address-poison-custom-array-cookie %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE-OFF
+// RUN: %clang_cl --target=x86_64-windows -fsanitize=address -fno-sanitize-address-poison-custom-array-cookie -### -- %s 2>&1 | FileCheck %s --check-prefix=CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE-OFF
+// CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE-OFF-NOT: -cc1{{.*}}address-poison-custom-array-cookie
+
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=address -fno-sanitize-address-poison-custom-array-cookie -fsanitize-address-poison-custom-array-cookie %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE-BOTH
+// RUN: %clang_cl --target=x86_64-windows -fsanitize=address -fno-sanitize-address-poison-custom-array-cookie -fsanitize-address-poison-custom-array-cookie -### -- %s 2>&1 | FileCheck %s --check-prefix=CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE-BOTH
+// CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE-BOTH: -cc1{{.*}}-fsanitize-address-poison-custom-array-cookie
+
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=address -fsanitize-address-poison-custom-array-cookie -fno-sanitize-address-poison-custom-array-cookie %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE-BOTH-OFF
+// CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE-BOTH-OFF-NOT: -cc1{{.*}}address-poison-custom-array-cookie
+
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=address %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-ASAN-WITHOUT-POISON-CUSTOM-ARRAY-NEW-COOKIE
+// CHECK-ASAN-WITHOUT-POISON-CUSTOM-ARRAY-NEW-COOKIE-NOT: -cc1{{.*}}address-poison-custom-array-cookie
+
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address -fsanitize-address-globals-dead-stripping %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-ASAN-GLOBALS
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ASAN-GLOBALS
 // RUN: %clang_cl --target=x86_64-windows-msvc -fsanitize=address -fsanitize-address-globals-dead-stripping -### -- %s 2>&1 | FileCheck %s --check-prefix=CHECK-ASAN-GLOBALS
Index: test/CodeGen/address-sanitizer-and-array-cookie.cpp
===
--- test/CodeGen/address-sanitizer-and-array-cookie.cpp
+++ test/CodeGen/address-sanitizer-and-array-cookie.cpp
@@ -1,6 +1,6 @@
 // RUN: %clang_cc1 -triple x86_64-gnu-linux -emit-llvm -o - %s | FileCheck %s -check-prefix=PLAIN
 // RUN: %clang_cc1 -triple x86_64-gnu-linux -emit-llvm -o - -fsanitize=address %s | FileCheck %s -check-prefix=ASAN
-// RUN: %clang_cc1 -triple x86_64-gnu-linux -emit-llvm -o - -fsanitize=address -fsanitize-address-poison-class-member-array-new-cookie %s | FileCheck %s -check-prefix=ASAN-POISON-ALL-NEW-ARRAY
+// RUN: %clang_cc1 -triple x86_64-gnu-linux -emit-llvm -o - -fsanitize=address -fsanitize-address-poison-custom-array-cookie %s | FileCheck %s -check-prefix=ASAN-POISON-ALL-NEW-ARRAY
 
 typedef __typeof__(sizeof(0)) size_t;
 namespace std {
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -968,11 +968,11 @@
   Args.hasArg(OPT_fsanitize_cfi_icall_generalize_pointers);
   Opts.SanitizeStats = Args.hasArg(OPT_fsanitize_stats);
   if (Arg *A = Args.getLastArg(
-  OPT_fsanitize_address_poison_cl

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

2018-11-01 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments.



Comment at: lib/AST/Expr.cpp:1609
   case CK_AddressSpaceConversion:
-assert(getType()->isPointerType() || getType()->isBlockPointerType());
-assert(getSubExpr()->getType()->isPointerType() ||
-   getSubExpr()->getType()->isBlockPointerType());
-assert(getType()->getPointeeType().getAddressSpace() !=
-   getSubExpr()->getType()->getPointeeType().getAddressSpace());
-LLVM_FALLTHROUGH;
+assert(/*If pointer type then addr spaces for pointees must differ*/
+   (((getType()->isPointerType() &&

rjmccall wrote:
> Anastasia wrote:
> > I don't like this assert now. Would adding extra variable be cleaner here?
> Yeah, this assertion doesn't make any sense like this.  It should be checking 
> whether the cast is a gl-value and, if so, requiring the subexpression to 
> also be a gl-value and then asserting the difference between the type.  But 
> you can certainly do an address-space conversion on l-values that just happen 
> to be of pointer or block-pointer type.
No, if this is a gl-value cast, the assertion must ignore whether there's a 
pointee type, or it will be messed up on gl-values of pointer types.

That is, if I have a gl-value of type `char * __private`, I should be able to 
do an address-space promotion to get a gl-value of type `char * __generic`.  
It's okay that the pointers are into the same address space here — in fact, 
it's more than okay, it's necessary.


https://reviews.llvm.org/D53764



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


[PATCH] D53982: Output "rule" information in SARIF

2018-11-01 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman created this revision.
aaron.ballman added reviewers: george.karpenkov, dcoughlin, zaks.anna.

SARIF allows you to export descriptions about rules that are present in the 
SARIF log. This patch exposes the help text table generated into Checkers.inc 
as the rule's "full description" and exports all of the rules present in the 
analysis output.

This information is useful for analysis result viewers like CodeSonar.


https://reviews.llvm.org/D53982

Files:
  lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
  
test/Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif
  
test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
  test/Analysis/diagnostics/sarif-multi-diagnostic-test.c

Index: test/Analysis/diagnostics/sarif-multi-diagnostic-test.c
===
--- test/Analysis/diagnostics/sarif-multi-diagnostic-test.c
+++ test/Analysis/diagnostics/sarif-multi-diagnostic-test.c
@@ -0,0 +1,29 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.security.taint,debug.TaintTest %s -verify -analyzer-output=sarif -o - | diff -U1 -w -I ".*file:.*sarif-multi-diagnostic-test.c" -I '"version":' -I "2\.0\.0\-csd\.[0-9]*\.beta\." - %S/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
+#include "../Inputs/system-header-simulator.h"
+
+int atoi(const char *nptr);
+
+void f(void) {
+  char s[80];
+  scanf("%s", s);
+  int d = atoi(s); // expected-warning {{tainted}}
+}
+
+void g(void) {
+  void (*fp)(int);
+  fp(12); // expected-warning {{Called function pointer is an uninitialized pointer value}}
+}
+
+int h(int i) {
+  if (i == 0)
+return 1 / i; // expected-warning {{Division by zero}}
+  return 0;
+}
+
+int main(void) {
+  f();
+  g();
+  h(0);
+  return 0;
+}
+
Index: test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
===
--- test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
+++ test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
@@ -0,0 +1,301 @@
+{
+  "$schema": "http://json.schemastore.org/sarif-2.0.0-csd.2.beta.2018-10-10";,
+  "runs": [
+{
+  "files": {
+"file:sarif-multi-diagnostic-test.c": {
+  "fileLocation": {
+"uri": "file:sarif-multi-diagnostic-test.c"
+  },
+  "length": 761,
+  "mimeType": "text/plain",
+  "roles": [
+"resultFile"
+  ]
+}
+  },
+  "resources": {
+"rules": {
+  "core.CallAndMessage": {
+"fullDescription": {
+  "text": "Check for logical errors for function calls and Objective-C message expressions (e.g., uninitialized arguments, null function pointers)"
+},
+"name": {
+  "text": "core.CallAndMessage"
+}
+  },
+  "core.DivideZero": {
+"fullDescription": {
+  "text": "Check for division by zero"
+},
+"name": {
+  "text": "core.DivideZero"
+}
+  },
+  "debug.TaintTest": {
+"fullDescription": {
+  "text": "Mark tainted symbols as such."
+},
+"name": {
+  "text": "debug.TaintTest"
+}
+  }
+}
+  },
+  "results": [
+{
+  "codeFlows": [
+{
+  "threadFlows": [
+{
+  "locations": [
+{
+  "importance": "essential",
+  "location": {
+"message": {
+  "text": "Calling 'f'"
+},
+"physicalLocation": {
+  "fileLocation": {
+"uri": "file:sarif-multi-diagnostic-test.c"
+  },
+  "region": {
+"endColumn": 5,
+"endLine": 24,
+"startColumn": 3,
+"startLine": 24
+  }
+}
+  }
+},
+{
+  "importance": "essential",
+  "location": {
+"message": {
+  "text": "tainted"
+},
+"physicalLocation": {
+  "fileLocation": {
+"uri": "file:sarif-multi-diagnostic-test.c"
+  },
+  "region": {
+"endColumn": 17,
+"endLine": 9,
+"startColumn": 11,
+"startLine": 9
+

[PATCH] D52949: [Diagnostics] Implement -Wsizeof-pointer-div

2018-11-01 Thread Dávid Bolvanský via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL345847: [Diagnostics] Implement -Wsizeof-pointer-div  
(authored by xbolva00, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D52949?vs=172117&id=172148#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D52949

Files:
  cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
  cfe/trunk/lib/Sema/SemaExpr.cpp
  cfe/trunk/test/Sema/div-sizeof-ptr.cpp

Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3294,6 +3294,10 @@
   InGroup;
 def note_reference_is_return_value : Note<"%0 returns a reference">;
 
+def warn_division_sizeof_ptr : Warning<
+  "'%0' will return the size of the pointer, not the array itself">,
+  InGroup>;
+
 def note_function_warning_silence : Note<
 "prefix with the address-of operator to silence this warning">;
 def note_function_to_function_call : Note<
Index: cfe/trunk/test/Sema/div-sizeof-ptr.cpp
===
--- cfe/trunk/test/Sema/div-sizeof-ptr.cpp
+++ cfe/trunk/test/Sema/div-sizeof-ptr.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 %s -verify -Wsizeof-pointer-div -fsyntax-only
+
+template 
+int f(Ty (&Array)[N]) {
+  return sizeof(Array) / sizeof(Ty); // Should not warn
+}
+
+void test(int *p, int **q) {
+  int a1 = sizeof(p) / sizeof(*p);   // expected-warning {{'sizeof (p)' will return the size of the pointer, not the array itself}}
+  int a2 = sizeof p / sizeof *p; // expected-warning {{'sizeof p' will return the size of the pointer, not the array itself}}
+  int a3 = sizeof(*q) / sizeof(**q); // expected-warning {{'sizeof (*q)' will return the size of the pointer, not the array itself}}
+  int a4 = sizeof(p) / sizeof(int);  // expected-warning {{'sizeof (p)' will return the size of the pointer, not the array itself}}
+  int a5 = sizeof(p) / sizeof(p[0]); // expected-warning {{'sizeof (p)' will return the size of the pointer, not the array itself}}
+
+  // Should not warn
+  int b1 = sizeof(int *) / sizeof(int);
+  int b2 = sizeof(p) / sizeof(p);
+  int b3 = sizeof(*q) / sizeof(q);
+  int b4 = sizeof(p) / sizeof(char);
+
+  int arr[10];
+  int b5 = sizeof(arr) / sizeof(*arr);
+  int b6 = sizeof(arr) / sizeof(arr[0]);
+  int b7 = sizeof(arr) / sizeof(int);
+
+  int arr2[10][12];
+  int b8 = sizeof(arr2) / sizeof(*arr2);
+}
Index: cfe/trunk/lib/Sema/SemaExpr.cpp
===
--- cfe/trunk/lib/Sema/SemaExpr.cpp
+++ cfe/trunk/lib/Sema/SemaExpr.cpp
@@ -8726,6 +8726,32 @@
   << LHS.get()->getSourceRange() << RHS.get()->getSourceRange();
 }
 
+static void DiagnoseDivisionSizeofPointer(Sema &S, Expr *LHS, Expr *RHS,
+  SourceLocation Loc) {
+  const auto *LUE = dyn_cast(LHS);
+  const auto *RUE = dyn_cast(RHS);
+  if (!LUE || !RUE)
+return;
+  if (LUE->getKind() != UETT_SizeOf || LUE->isArgumentType() ||
+  RUE->getKind() != UETT_SizeOf)
+return;
+
+  QualType LHSTy = LUE->getArgumentExpr()->IgnoreParens()->getType();
+  QualType RHSTy;
+
+  if (RUE->isArgumentType())
+RHSTy = RUE->getArgumentType();
+  else
+RHSTy = RUE->getArgumentExpr()->IgnoreParens()->getType();
+
+  if (!LHSTy->isPointerType() || RHSTy->isPointerType())
+return;
+  if (LHSTy->getPointeeType() != RHSTy)
+return;
+
+  S.Diag(Loc, diag::warn_division_sizeof_ptr) << LHS << LHS->getSourceRange();
+}
+
 static void DiagnoseBadDivideOrRemainderValues(Sema& S, ExprResult &LHS,
ExprResult &RHS,
SourceLocation Loc, bool IsDiv) {
@@ -8756,8 +8782,10 @@
 
   if (compType.isNull() || !compType->isArithmeticType())
 return InvalidOperands(Loc, LHS, RHS);
-  if (IsDiv)
+  if (IsDiv) {
 DiagnoseBadDivideOrRemainderValues(*this, LHS, RHS, Loc, IsDiv);
+DiagnoseDivisionSizeofPointer(*this, LHS.get(), RHS.get(), Loc);
+  }
   return compType;
 }
 
@@ -16603,4 +16631,4 @@
 
   return new (Context)
   ObjCAvailabilityCheckExpr(Version, AtLoc, RParen, Context.BoolTy);
-}
+}
\ No newline at end of file
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r345847 - [Diagnostics] Implement -Wsizeof-pointer-div

2018-11-01 Thread David Bolvansky via cfe-commits
Author: xbolva00
Date: Thu Nov  1 09:26:10 2018
New Revision: 345847

URL: http://llvm.org/viewvc/llvm-project?rev=345847&view=rev
Log:
[Diagnostics] Implement -Wsizeof-pointer-div 

Summary:
void test(int *arr) {
int arr_len = sizeof(arr) / sizeof(*arr);  // warn, incorrect way to 
compute number of array elements
}

Enabled under -Wall (same behaviour as GCC)

Reviewers: rsmith, MTC, aaron.ballman

Reviewed By: aaron.ballman

Subscribers: MTC, thakis, jfb, cfe-commits

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

Added:
cfe/trunk/test/Sema/div-sizeof-ptr.cpp
Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaExpr.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=345847&r1=345846&r2=345847&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Nov  1 09:26:10 
2018
@@ -3294,6 +3294,10 @@ def warn_address_of_reference_null_compa
   InGroup;
 def note_reference_is_return_value : Note<"%0 returns a reference">;
 
+def warn_division_sizeof_ptr : Warning<
+  "'%0' will return the size of the pointer, not the array itself">,
+  InGroup>;
+
 def note_function_warning_silence : Note<
 "prefix with the address-of operator to silence this warning">;
 def note_function_to_function_call : Note<

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=345847&r1=345846&r2=345847&view=diff
==
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Thu Nov  1 09:26:10 2018
@@ -8726,6 +8726,32 @@ static void checkArithmeticNull(Sema &S,
   << LHS.get()->getSourceRange() << RHS.get()->getSourceRange();
 }
 
+static void DiagnoseDivisionSizeofPointer(Sema &S, Expr *LHS, Expr *RHS,
+  SourceLocation Loc) {
+  const auto *LUE = dyn_cast(LHS);
+  const auto *RUE = dyn_cast(RHS);
+  if (!LUE || !RUE)
+return;
+  if (LUE->getKind() != UETT_SizeOf || LUE->isArgumentType() ||
+  RUE->getKind() != UETT_SizeOf)
+return;
+
+  QualType LHSTy = LUE->getArgumentExpr()->IgnoreParens()->getType();
+  QualType RHSTy;
+
+  if (RUE->isArgumentType())
+RHSTy = RUE->getArgumentType();
+  else
+RHSTy = RUE->getArgumentExpr()->IgnoreParens()->getType();
+
+  if (!LHSTy->isPointerType() || RHSTy->isPointerType())
+return;
+  if (LHSTy->getPointeeType() != RHSTy)
+return;
+
+  S.Diag(Loc, diag::warn_division_sizeof_ptr) << LHS << LHS->getSourceRange();
+}
+
 static void DiagnoseBadDivideOrRemainderValues(Sema& S, ExprResult &LHS,
ExprResult &RHS,
SourceLocation Loc, bool IsDiv) 
{
@@ -8756,8 +8782,10 @@ QualType Sema::CheckMultiplyDivideOperan
 
   if (compType.isNull() || !compType->isArithmeticType())
 return InvalidOperands(Loc, LHS, RHS);
-  if (IsDiv)
+  if (IsDiv) {
 DiagnoseBadDivideOrRemainderValues(*this, LHS, RHS, Loc, IsDiv);
+DiagnoseDivisionSizeofPointer(*this, LHS.get(), RHS.get(), Loc);
+  }
   return compType;
 }
 
@@ -16603,4 +16631,4 @@ ExprResult Sema::ActOnObjCAvailabilityCh
 
   return new (Context)
   ObjCAvailabilityCheckExpr(Version, AtLoc, RParen, Context.BoolTy);
-}
+}
\ No newline at end of file

Added: cfe/trunk/test/Sema/div-sizeof-ptr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/div-sizeof-ptr.cpp?rev=345847&view=auto
==
--- cfe/trunk/test/Sema/div-sizeof-ptr.cpp (added)
+++ cfe/trunk/test/Sema/div-sizeof-ptr.cpp Thu Nov  1 09:26:10 2018
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 %s -verify -Wsizeof-pointer-div -fsyntax-only
+
+template 
+int f(Ty (&Array)[N]) {
+  return sizeof(Array) / sizeof(Ty); // Should not warn
+}
+
+void test(int *p, int **q) {
+  int a1 = sizeof(p) / sizeof(*p);   // expected-warning {{'sizeof (p)' will 
return the size of the pointer, not the array itself}}
+  int a2 = sizeof p / sizeof *p; // expected-warning {{'sizeof p' will 
return the size of the pointer, not the array itself}}
+  int a3 = sizeof(*q) / sizeof(**q); // expected-warning {{'sizeof (*q)' will 
return the size of the pointer, not the array itself}}
+  int a4 = sizeof(p) / sizeof(int);  // expected-warning {{'sizeof (p)' will 
return the size of the pointer, not the array itself}}
+  int a5 = sizeof(p) / sizeof(p[0]); // expected-warning {{'sizeof (p)' will 
return the size of the pointer, not the array itself}}
+
+  // Should not warn
+  int b1 = sizeof(int *) / sizeof(int);
+  int b2 = sizeof(p) / sizeof(p);
+  int b3 = sizeof(*q) / sizeof(q);
+  

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

2018-11-01 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

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

> In https://reviews.llvm.org/D53738#1283459, @rjmccall wrote:
>
> > Well, it could be passed around through most code as some sort of 
> > abstractly-represented intermediate "type" which could be either a QualType 
> > or a fixed-point semantics.
>
>
> Sounds to me like you're describing a new `Type` that can contain an 
> arbitrary fixed-point semantic :)


The fact that it doesn't exist in the modeled type system is important.

The arbitrary-type overflow intrinsics have this same problem, and we did not 
try to solve it by creating a new type class for arbitrary-precision integers 
and promoting the arguments.

> It still feels like this just introduces inconsistencies into the form of the 
> AST. If we do add this extra type object to the BO, won't people wonder why 
> we use ImplicitCastExpr for non-fixedpoint operations but use the special 
> `QualTypeOrFPSemantics BinaryOperator::ComputationType;` for fixedpoint 
> operations even though they both have nearly the same semantic meaning 
> (converting to a common type before doing the operation)?



1. You would have an inconsistency in either case, since e.g. numeric + 
otherwise always returns the same type as its operands, but this would not.
2. The question is easily answered by pointing at the language spec.  The 
language does not say that the operands are promoted to a common type; it says 
the result is determined numerically from the true numeric values of the 
operands.

>>> It might just be easier to store the full-precision info in BO directly. BO 
>>> might be too common to warrant the size increase, though. 
>>> FixedPointSemantics can probably be optimized to only take 32 bits.
>> 
>> What you can definitely do is store a bit in BO saying that there's extra 
>> storage for the intermediate "type".
> 
> Is this similar to how ExtQuals works? How would this be implemented?

Take a look at how `DeclRefExpr` stores its various optional components.

>>> As a side note, comparisons are still a bit up in the air. I don't think we 
>>> came to a conclusion on whether they should be done in full precision or 
>>> bitwise. The spec isn't clear.
>> 
>> ...bitwise?
> 
> The spec uses different wording for the arithmetic operations and comparison 
> operations, and it's not entirely obvious what it means. For the arithmetic 
> operators, it has the whole section on finding the full precision common 
> type, but for comparisons it says:
> 
>> When comparing fixed-point values with fixed-point values or integer values,
>>  the values are compared directly; the values of the operands are not 
>> converted before the
>>  comparison is made.
> 
> What 'directly' means in conjunction with 'the operands are not converted' is 
> not clear. It's reasonable to assume that it either means comparing 
> value-wise (so, the same as finding a common type that fits all values and 
> then comparing; though this would obviously require conversion), or perhaps 
> 'directly' means a bitwise (representation) comparison. The latter seems a 
> bit farfetched to me, though.

I think this is just like fixed-point arithmetic: you should do an exact 
numeric comparison, but there's no formal conversion because it would have to 
be a conversion to some concrete type, which could leave to (mandatory!) 
inexactness when there's no common type that expresses the full range of both 
operands.

Arguably this is all how integer arithmetic and comparisons should work, but 
that's not something they can fix in a TR.  (Floating-point doesn't need to 
work this way because the FP types subset each other.)


Repository:
  rC Clang

https://reviews.llvm.org/D53738



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


[PATCH] D53847: [C++2a] P0634r3: Down with typename!

2018-11-01 Thread Nicolas Lesser via Phabricator via cfe-commits
Rakete updated this revision to Diff 172147.
Rakete marked an inline comment as done.
Rakete added a comment.

Remove spaces to be consistent.


Repository:
  rC Clang

https://reviews.llvm.org/D53847

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Parse/Parser.h
  include/clang/Sema/Scope.h
  include/clang/Sema/Sema.h
  lib/Parse/ParseDecl.cpp
  lib/Parse/ParseDeclCXX.cpp
  lib/Parse/ParseExprCXX.cpp
  lib/Parse/ParseTentative.cpp
  lib/Parse/Parser.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaLookup.cpp
  lib/Sema/SemaTemplate.cpp
  test/CXX/drs/dr1xx.cpp
  test/CXX/drs/dr2xx.cpp
  test/CXX/drs/dr4xx.cpp
  test/CXX/drs/dr5xx.cpp
  test/CXX/temp/temp.res/p5.cpp
  test/CXX/temp/temp.res/temp.dep/temp.dep.type/p1.cpp
  test/FixIt/fixit.cpp
  test/SemaCXX/MicrosoftCompatibility.cpp
  test/SemaCXX/MicrosoftExtensions.cpp
  test/SemaCXX/MicrosoftSuper.cpp
  test/SemaCXX/unknown-type-name.cpp

Index: test/SemaCXX/unknown-type-name.cpp
===
--- test/SemaCXX/unknown-type-name.cpp
+++ test/SemaCXX/unknown-type-name.cpp
@@ -36,39 +36,39 @@
 
   static int n;
   static type m;
-  static int h(T::type, int); // expected-error{{missing 'typename'}}
-  static int h(T::type x, char); // expected-error{{missing 'typename'}}
+  static int h(T::type, int); // expected-warning{{implicit 'typename' is a C++2a extension}}
+  static int h(T::type x, char); // expected-warning{{implicit 'typename' is a C++2a extension}}
 };
 
 template
-A::type g(T t) { return t; } // expected-error{{missing 'typename'}}
+A::type g(T t) { return t; } // expected-warning{{implicit 'typename' is a C++2a extension}}
 
 template
-A::type A::f() { return type(); } // expected-error{{missing 'typename'}}
+A::type A::f() { return type(); } // expected-warning{{implicit 'typename' is a C++2a extension}}
 
 template
-void f(T::type) { } // expected-error{{missing 'typename'}}
+void f(T::type) { } // expected-warning{{implicit 'typename' is a C++2a extension}}
 
 template
-void g(T::type x) { } // expected-error{{missing 'typename'}}
+void g(T::type x) { } // expected-warning{{implicit 'typename' is a C++2a extension}}
 
 template
-void f(T::type, int) { } // expected-error{{missing 'typename'}}
+void f(T::type, int) { } // expected-warning{{implicit 'typename' is a C++2a extension}}
 
 template
-void f(T::type x, char) { } // expected-error{{missing 'typename'}}
+void f(T::type x, char) { } // expected-warning{{implicit 'typename' is a C++2a extension}}
 
 template
-void f(int, T::type) { } // expected-error{{missing 'typename'}}
+void f(int, T::type) { } // expected-warning{{implicit 'typename' is a C++2a extension}}
 
 template
-void f(char, T::type x) { } // expected-error{{missing 'typename'}}
+void f(char, T::type x) { } // expected-warning{{implicit 'typename' is a C++2a extension}}
 
 template
-void f(int, T::type, int) { } // expected-error{{missing 'typename'}}
+void f(int, T::type, int) { } // expected-warning{{implicit 'typename' is a C++2a extension}}
 
 template
-void f(int, T::type x, char) { } // expected-error{{missing 'typename'}}
+void f(int, T::type x, char) { } // expected-warning{{implicit 'typename' is a C++2a extension}}
 
 int *p;
 
@@ -86,11 +86,11 @@
 
 template int A::n(T::value); // ok
 template
-A::type // expected-error{{missing 'typename'}}
+A::type // expected-warning {{implicit 'typename' is a C++2a extension}}
 A::m(T::value, 0); // ok
 
-template int A::h(T::type, int) {} // expected-error{{missing 'typename'}}
-template int A::h(T::type x, char) {} // expected-error{{missing 'typename'}}
+template int A::h(T::type, int) {} // expected-warning{{implicit 'typename' is a C++2a extension}}
+template int A::h(T::type x, char) {} // expected-warning{{implicit 'typename' is a C++2a extension}}
 
 template int h(T::type, int); // expected-error{{missing 'typename'}}
 template int h(T::type x, char); // expected-error{{missing 'typename'}}
@@ -100,7 +100,7 @@
 // expected-warning@-2 {{variable templates are a C++14 extension}}
 #endif
 template int junk2(T::junk) throw(); // expected-error{{missing 'typename'}}
-template int junk3(T::junk) = delete; // expected-error{{missing 'typename'}}
+template int junk3(T::junk) = delete; // expected-warning{{implicit 'typename' is a C++2a extension}}
 #if __cplusplus <= 199711L
 //expected-warning@-2 {{deleted function definitions are a C++11 extension}}
 #endif
@@ -118,4 +118,5 @@
 // FIXME: We know which type specifier should have been specified here. Provide
 //a fix-it to add 'typename A::type'
 template
-A::g() { } // expected-error{{requires a type specifier}}
+A::g() { } // expected-error{{expected unqualified-id}}
+// expected-warning@-1{{implicit 'typename' is a C++2a extension}}
Index: test/SemaCXX/MicrosoftSuper.cpp
===
--- test/SemaCXX/MicrosoftSuper.cpp
+++ test/SemaCXX/MicrosoftSuper.cpp
@@ -108,8 +108,8 @@
   

[PATCH] D53847: [C++2a] P0634r3: Down with typename!

2018-11-01 Thread Nicolas Lesser via Phabricator via cfe-commits
Rakete marked 7 inline comments as done.
Rakete added inline comments.



Comment at: lib/Parse/Parser.cpp:1778
+/*IsClassTemplateDeductionContext*/true,
+/*AllowImplicitTypename*/false)) {
   SourceLocation BeginLoc = Tok.getLocation();

rsmith wrote:
> Don't we need to sometimes allow an implicit typename here for correct 
> disambiguation?
This is done elsewhere now. :)



Comment at: test/SemaCXX/unknown-type-name.cpp:50
 template
-void f(T::type) { } // expected-error{{missing 'typename'}}
+void f(T::type) { } // expected-warning {{implicit 'typename' is a C++2a 
extension}}
 

rsmith wrote:
> This is wrong.
> 
> ```
> template
> X f(T::type);
> ```
> 
> declares a variable template. This would be valid if the name `f` were a 
> //qualified-id//, and lookup for `f` found a function template, though.
> 
> (Same for the next 7 cases.)
I agree, but those are definitions, so the next 7 cases and this now are 
correct.



Comment at: test/SemaCXX/unknown-type-name.cpp:102-103
 #endif
-template int junk2(T::junk) throw(); // expected-error{{missing 
'typename'}}
-template int junk3(T::junk) = delete; // expected-error{{missing 
'typename'}}
+template int junk2(T::junk) throw(); // expected-warning 
{{implicit 'typename' is a C++2a extension}}
+template int junk3(T::junk) = delete; // expected-warning 
{{implicit 'typename' is a C++2a extension}}
 #if __cplusplus <= 199711L

rsmith wrote:
> These two are incorrect.
Only the first one is incorrect (i.e. doesn't have implicit typename). The 
second one does, because it's a function definition.


Repository:
  rC Clang

https://reviews.llvm.org/D53847



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


Re: r345833 - Logging: add CMake dependency so libAST can use OSLog analysis.

2018-11-01 Thread Tim Northover via cfe-commits
Oh bother, I vaguely remember that means I’ll have broken .so builds now?

Either way, I’ve reverted the sequence in r345846. I’ll work on putting the 
os_log machinery into AST and update the review.

Thanks for looking out!

Tim.

> On 1 Nov 2018, at 16:05, Benjamin Kramer  wrote:
> 
> This doesn't work. AST cannot depend on Analysis because Analysis already 
> depends on AST.
> 
> On Thu, Nov 1, 2018 at 3:24 PM Tim Northover via cfe-commits 
> mailto:cfe-commits@lists.llvm.org>> wrote:
> Author: tnorthover
> Date: Thu Nov  1 07:22:20 2018
> New Revision: 345833
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=345833&view=rev 
> 
> Log:
> Logging: add CMake dependency so libAST can use OSLog analysis.
> 
> Should fix bots on platforms with slightly different symbol resolution
> semantics.
> 
> Modified:
> cfe/trunk/lib/AST/CMakeLists.txt
> 
> Modified: cfe/trunk/lib/AST/CMakeLists.txt
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/CMakeLists.txt?rev=345833&r1=345832&r2=345833&view=diff
>  
> 
> ==
> --- cfe/trunk/lib/AST/CMakeLists.txt (original)
> +++ cfe/trunk/lib/AST/CMakeLists.txt Thu Nov  1 07:22:20 2018
> @@ -1,4 +1,5 @@
>  set(LLVM_LINK_COMPONENTS
> +  Analysis
>BinaryFormat
>Support
>)
> 
> 
> ___
> 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


r345846 - Revert "Logging: make os_log buffer size an integer constant expression.

2018-11-01 Thread Tim Northover via cfe-commits
Author: tnorthover
Date: Thu Nov  1 09:15:24 2018
New Revision: 345846

URL: http://llvm.org/viewvc/llvm-project?rev=345846&view=rev
Log:
Revert "Logging: make os_log buffer size an integer constant expression.

This also reverts a couple of follow-up commits trying to fix the
dependency issues. Latest revision added a cyclic dependency that can't
just be patched up in 5 minutes.

Modified:
cfe/trunk/lib/AST/CMakeLists.txt
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/test/CodeGen/builtins.c

Modified: cfe/trunk/lib/AST/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/CMakeLists.txt?rev=345846&r1=345845&r2=345846&view=diff
==
--- cfe/trunk/lib/AST/CMakeLists.txt (original)
+++ cfe/trunk/lib/AST/CMakeLists.txt Thu Nov  1 09:15:24 2018
@@ -72,7 +72,6 @@ add_clang_library(clangAST
   VTTBuilder.cpp
 
   LINK_LIBS
-  clangAnalysis
   clangBasic
   clangLex
   )

Modified: cfe/trunk/lib/AST/ExprConstant.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=345846&r1=345845&r2=345846&view=diff
==
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)
+++ cfe/trunk/lib/AST/ExprConstant.cpp Thu Nov  1 09:15:24 2018
@@ -33,7 +33,6 @@
 //
 
//===--===//
 
-#include "clang/Analysis/Analyses/OSLog.h"
 #include "clang/AST/APValue.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTDiagnostic.h"
@@ -8127,12 +8126,6 @@ bool IntExprEvaluator::VisitBuiltinCallE
 llvm_unreachable("unexpected EvalMode");
   }
 
-  case Builtin::BI__builtin_os_log_format_buffer_size: {
-analyze_os_log::OSLogBufferLayout Layout;
-analyze_os_log::computeOSLogBufferLayout(Info.Ctx, E, Layout);
-return Success(Layout.size().getQuantity(), E);
-  }
-
   case Builtin::BI__builtin_bswap16:
   case Builtin::BI__builtin_bswap32:
   case Builtin::BI__builtin_bswap64: {

Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=345846&r1=345845&r2=345846&view=diff
==
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Thu Nov  1 09:15:24 2018
@@ -3609,6 +3609,13 @@ RValue CodeGenFunction::EmitBuiltinExpr(
   case Builtin::BI__builtin_os_log_format:
 return emitBuiltinOSLogFormat(*E);
 
+  case Builtin::BI__builtin_os_log_format_buffer_size: {
+analyze_os_log::OSLogBufferLayout Layout;
+analyze_os_log::computeOSLogBufferLayout(CGM.getContext(), E, Layout);
+return RValue::get(ConstantInt::get(ConvertType(E->getType()),
+Layout.size().getQuantity()));
+  }
+
   case Builtin::BI__xray_customevent: {
 if (!ShouldXRayInstrumentFunction())
   return RValue::getIgnored();

Modified: cfe/trunk/test/CodeGen/builtins.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/builtins.c?rev=345846&r1=345845&r2=345846&view=diff
==
--- cfe/trunk/test/CodeGen/builtins.c (original)
+++ cfe/trunk/test/CodeGen/builtins.c Thu Nov  1 09:15:24 2018
@@ -729,28 +729,25 @@ void test_builtin_os_log_merge_helper1(v
 
 // CHECK-LABEL: define void @test_builtin_os_log_errno
 void test_builtin_os_log_errno() {
-  // CHECK-NOT: @stacksave
-  // CHECK: %[[BUF:.*]] = alloca [4 x i8], align 1
-  // CHECK: %[[DECAY:.*]] = getelementptr inbounds [4 x i8], [4 x i8]* 
%[[BUF]], i32 0, i32 0
-  // CHECK: call void @__os_log_helper_1_2_1_0_96(i8* %[[DECAY]])
-  // CHECK-NOT: @stackrestore
+  // CHECK: %[[VLA:.*]] = alloca i8, i64 4, align 16
+  // CHECK: call void @__os_log_helper_16_2_1_0_96(i8* %[[VLA]])
 
   char buf[__builtin_os_log_format_buffer_size("%m")];
   __builtin_os_log_format(buf, "%m");
 }
 
-// CHECK-LABEL: define linkonce_odr hidden void @__os_log_helper_1_2_1_0_96
+// CHECK-LABEL: define linkonce_odr hidden void @__os_log_helper_16_2_1_0_96
 // CHECK: (i8* %[[BUFFER:.*]])
 
 // CHECK: %[[BUFFER_ADDR:.*]] = alloca i8*, align 8
 // CHECK: store i8* %[[BUFFER]], i8** %[[BUFFER_ADDR]], align 8
 // CHECK: %[[BUF:.*]] = load i8*, i8** %[[BUFFER_ADDR]], align 8
 // CHECK: %[[SUMMARY:.*]] = getelementptr i8, i8* %[[BUF]], i64 0
-// CHECK: store i8 2, i8* %[[SUMMARY]], align 1
+// CHECK: store i8 2, i8* %[[SUMMARY]], align 16
 // CHECK: %[[NUMARGS:.*]] = getelementptr i8, i8* %[[BUF]], i64 1
 // CHECK: store i8 1, i8* %[[NUMARGS]], align 1
 // CHECK: %[[ARGDESCRIPTOR:.*]] = getelementptr i8, i8* %[[BUF]], i64 2
-// CHECK: store i8 96, i8* %[[ARGDESCRIPTOR]], align 1
+// CHECK: store i8 96, i8* %[[ARGDESCRIPTOR]], align 2
 // CHECK: %[[ARGSIZE:.*]] = getelementptr i8, i8* %[[BUF]], i64 3
 // CHECK: store i8 0, i8*

[PATCH] D53847: [C++2a] P0634r3: Down with typename!

2018-11-01 Thread Nicolas Lesser via Phabricator via cfe-commits
Rakete updated this revision to Diff 172146.
Rakete marked 6 inline comments as done.
Rakete added a comment.

Addressed review comments! :)


Repository:
  rC Clang

https://reviews.llvm.org/D53847

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Parse/Parser.h
  include/clang/Sema/Scope.h
  include/clang/Sema/Sema.h
  lib/Parse/ParseDecl.cpp
  lib/Parse/ParseDeclCXX.cpp
  lib/Parse/ParseExprCXX.cpp
  lib/Parse/ParseTentative.cpp
  lib/Parse/Parser.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaLookup.cpp
  lib/Sema/SemaTemplate.cpp
  test/CXX/drs/dr1xx.cpp
  test/CXX/drs/dr2xx.cpp
  test/CXX/drs/dr4xx.cpp
  test/CXX/drs/dr5xx.cpp
  test/CXX/temp/temp.res/p5.cpp
  test/CXX/temp/temp.res/temp.dep/temp.dep.type/p1.cpp
  test/FixIt/fixit.cpp
  test/SemaCXX/MicrosoftCompatibility.cpp
  test/SemaCXX/MicrosoftExtensions.cpp
  test/SemaCXX/MicrosoftSuper.cpp
  test/SemaCXX/unknown-type-name.cpp

Index: test/SemaCXX/unknown-type-name.cpp
===
--- test/SemaCXX/unknown-type-name.cpp
+++ test/SemaCXX/unknown-type-name.cpp
@@ -36,39 +36,39 @@
 
   static int n;
   static type m;
-  static int h(T::type, int); // expected-error{{missing 'typename'}}
-  static int h(T::type x, char); // expected-error{{missing 'typename'}}
+  static int h(T::type, int); // expected-warning {{implicit 'typename' is a C++2a extension}}
+  static int h(T::type x, char); // expected-warning {{implicit 'typename' is a C++2a extension}}
 };
 
 template
-A::type g(T t) { return t; } // expected-error{{missing 'typename'}}
+A::type g(T t) { return t; } // expected-warning {{implicit 'typename' is a C++2a extension}}
 
 template
-A::type A::f() { return type(); } // expected-error{{missing 'typename'}}
+A::type A::f() { return type(); } // expected-warning {{implicit 'typename' is a C++2a extension}}
 
 template
-void f(T::type) { } // expected-error{{missing 'typename'}}
+void f(T::type) { } // expected-warning {{implicit 'typename' is a C++2a extension}}
 
 template
-void g(T::type x) { } // expected-error{{missing 'typename'}}
+void g(T::type x) { } // expected-warning {{implicit 'typename' is a C++2a extension}}
 
 template
-void f(T::type, int) { } // expected-error{{missing 'typename'}}
+void f(T::type, int) { } // expected-warning {{implicit 'typename' is a C++2a extension}}
 
 template
-void f(T::type x, char) { } // expected-error{{missing 'typename'}}
+void f(T::type x, char) { } // expected-warning {{implicit 'typename' is a C++2a extension}}
 
 template
-void f(int, T::type) { } // expected-error{{missing 'typename'}}
+void f(int, T::type) { } // expected-warning {{implicit 'typename' is a C++2a extension}}
 
 template
-void f(char, T::type x) { } // expected-error{{missing 'typename'}}
+void f(char, T::type x) { } // expected-warning {{implicit 'typename' is a C++2a extension}}
 
 template
-void f(int, T::type, int) { } // expected-error{{missing 'typename'}}
+void f(int, T::type, int) { } // expected-warning {{implicit 'typename' is a C++2a extension}}
 
 template
-void f(int, T::type x, char) { } // expected-error{{missing 'typename'}}
+void f(int, T::type x, char) { } // expected-warning {{implicit 'typename' is a C++2a extension}}
 
 int *p;
 
@@ -86,26 +86,26 @@
 
 template int A::n(T::value); // ok
 template
-A::type // expected-error{{missing 'typename'}}
+A::type // expected-warning {{implicit 'typename' is a C++2a extension}}
 A::m(T::value, 0); // ok
 
-template int A::h(T::type, int) {} // expected-error{{missing 'typename'}}
-template int A::h(T::type x, char) {} // expected-error{{missing 'typename'}}
+template int A::h(T::type, int) {} // expected-warning {{implicit 'typename' is a C++2a extension}}
+template int A::h(T::type x, char) {} // expected-warning {{implicit 'typename' is a C++2a extension}}
 
-template int h(T::type, int); // expected-error{{missing 'typename'}}
-template int h(T::type x, char); // expected-error{{missing 'typename'}}
+template int h(T::type, int); // expected-error {{missing 'typename'}}
+template int h(T::type x, char); // expected-error {{missing 'typename'}}
 
 template int junk1(T::junk);
 #if __cplusplus <= 201103L
 // expected-warning@-2 {{variable templates are a C++14 extension}}
 #endif
-template int junk2(T::junk) throw(); // expected-error{{missing 'typename'}}
-template int junk3(T::junk) = delete; // expected-error{{missing 'typename'}}
+template int junk2(T::junk) throw(); // expected-error {{missing 'typename'}}
+template int junk3(T::junk) = delete; // expected-warning {{implicit 'typename' is a C++2a extension}}
 #if __cplusplus <= 199711L
 //expected-warning@-2 {{deleted function definitions are a C++11 extension}}
 #endif
 
-template int junk4(T::junk j); // expected-error{{missing 'typename'}}
+template int junk4(T::junk j); // expected-error {{missing 'typename'}}
 
 // FIXME: We can tell this was intended to be a function because it does not
 //have a dependent nested na

[PATCH] D53433: [clangd] auto-index stores symbols per-file instead of per-TU.

2018-11-01 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments.



Comment at: clangd/index/Background.cpp:235
+IndexedFileDigests[Path] = FilesToUpdate.lookup(Path);
+IndexedSymbols.update(Path,
+  make_unique(std::move(Syms).build()),

This call is already thread-safe, no need for it to be under lock.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53433



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


Re: r345833 - Logging: add CMake dependency so libAST can use OSLog analysis.

2018-11-01 Thread Benjamin Kramer via cfe-commits
This doesn't work. AST cannot depend on Analysis because Analysis already
depends on AST.

On Thu, Nov 1, 2018 at 3:24 PM Tim Northover via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: tnorthover
> Date: Thu Nov  1 07:22:20 2018
> New Revision: 345833
>
> URL: http://llvm.org/viewvc/llvm-project?rev=345833&view=rev
> Log:
> Logging: add CMake dependency so libAST can use OSLog analysis.
>
> Should fix bots on platforms with slightly different symbol resolution
> semantics.
>
> Modified:
> cfe/trunk/lib/AST/CMakeLists.txt
>
> Modified: cfe/trunk/lib/AST/CMakeLists.txt
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/CMakeLists.txt?rev=345833&r1=345832&r2=345833&view=diff
>
> ==
> --- cfe/trunk/lib/AST/CMakeLists.txt (original)
> +++ cfe/trunk/lib/AST/CMakeLists.txt Thu Nov  1 07:22:20 2018
> @@ -1,4 +1,5 @@
>  set(LLVM_LINK_COMPONENTS
> +  Analysis
>BinaryFormat
>Support
>)
>
>
> ___
> 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] D53956: Fix test assumption that Linux implies glibc.

2018-11-01 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added a comment.

I hate this chunk of code. :-(
`TEST_HAS_C11_FEATURES` basically means that we can use C11 library features 
like `aligned_alloc`, and `timespec` etc.


Repository:
  rCXX libc++

https://reviews.llvm.org/D53956



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


r345844 - [clang] Improve ctor initializer completions.

2018-11-01 Thread Kadir Cetinkaya via cfe-commits
Author: kadircet
Date: Thu Nov  1 08:54:18 2018
New Revision: 345844

URL: http://llvm.org/viewvc/llvm-project?rev=345844&view=rev
Log:
[clang] Improve ctor initializer completions.

Summary:
Instead of providing generic "args" for member and base class
initializers, tries to fetch relevant constructors and show their signatures.

Reviewers: ilya-biryukov

Reviewed By: ilya-biryukov

Subscribers: ZaMaZaN4iK, eraman, arphaman, cfe-commits

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

Modified:
cfe/trunk/lib/Sema/SemaCodeComplete.cpp
cfe/trunk/test/CodeCompletion/ctor-initializer.cpp
cfe/trunk/test/Index/complete-ctor-inits.cpp
cfe/trunk/test/Index/complete-cxx-inline-methods.cpp

Modified: cfe/trunk/lib/Sema/SemaCodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCodeComplete.cpp?rev=345844&r1=345843&r2=345844&view=diff
==
--- cfe/trunk/lib/Sema/SemaCodeComplete.cpp (original)
+++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp Thu Nov  1 08:54:18 2018
@@ -808,12 +808,20 @@ void ResultBuilder::AdjustResultPriority
   }
 }
 
+DeclContext::lookup_result getConstructors(ASTContext &Context,
+   const CXXRecordDecl *Record) {
+  QualType RecordTy = Context.getTypeDeclType(Record);
+  DeclarationName ConstructorName =
+  Context.DeclarationNames.getCXXConstructorName(
+  Context.getCanonicalType(RecordTy));
+  return Record->lookup(ConstructorName);
+}
+
 void ResultBuilder::MaybeAddConstructorResults(Result R) {
   if (!SemaRef.getLangOpts().CPlusPlus || !R.Declaration ||
   !CompletionContext.wantConstructorResults())
 return;
 
-  ASTContext &Context = SemaRef.Context;
   const NamedDecl *D = R.Declaration;
   const CXXRecordDecl *Record = nullptr;
   if (const ClassTemplateDecl *ClassTemplate = dyn_cast(D))
@@ -831,16 +839,8 @@ void ResultBuilder::MaybeAddConstructorR
   if (!Record)
 return;
 
-
-  QualType RecordTy = Context.getTypeDeclType(Record);
-  DeclarationName ConstructorName
-= Context.DeclarationNames.getCXXConstructorName(
-   Context.getCanonicalType(RecordTy));
-  DeclContext::lookup_result Ctors = Record->lookup(ConstructorName);
-  for (DeclContext::lookup_iterator I = Ctors.begin(),
-  E = Ctors.end();
-   I != E; ++I) {
-R.Declaration = *I;
+  for(auto Ctor : getConstructors(SemaRef.Context, Record)) {
+R.Declaration = Ctor;
 R.CursorKind = getCursorKindForDecl(R.Declaration);
 Results.push_back(R);
   }
@@ -5073,11 +5073,77 @@ void Sema::CodeCompleteConstructorInitia
   }
 
   // Add completions for base classes.
-  CodeCompletionBuilder Builder(Results.getAllocator(),
-Results.getCodeCompletionTUInfo());
   PrintingPolicy Policy = getCompletionPrintingPolicy(*this);
   bool SawLastInitializer = Initializers.empty();
   CXXRecordDecl *ClassDecl = Constructor->getParent();
+
+  auto GenerateCCS = [&](const NamedDecl *ND, const char *Name) {
+CodeCompletionBuilder Builder(Results.getAllocator(),
+  Results.getCodeCompletionTUInfo());
+Builder.AddTypedTextChunk(Name);
+Builder.AddChunk(CodeCompletionString::CK_LeftParen);
+if (auto Function = dyn_cast(ND))
+  AddFunctionParameterChunks(PP, Policy, Function, Builder);
+else if (auto FunTemplDecl = dyn_cast(ND))
+  AddFunctionParameterChunks(PP, Policy, FunTemplDecl->getTemplatedDecl(),
+ Builder);
+Builder.AddChunk(CodeCompletionString::CK_RightParen);
+return Builder.TakeString();
+  };
+  auto AddDefaultCtorInit = [&](const char *Name, const char *Type,
+const NamedDecl *ND) {
+CodeCompletionBuilder Builder(Results.getAllocator(),
+  Results.getCodeCompletionTUInfo());
+Builder.AddTypedTextChunk(Name);
+Builder.AddChunk(CodeCompletionString::CK_LeftParen);
+Builder.AddPlaceholderChunk(Type);
+Builder.AddChunk(CodeCompletionString::CK_RightParen);
+if (ND) {
+  auto CCR = CodeCompletionResult(
+  Builder.TakeString(), ND,
+  SawLastInitializer ? CCP_NextInitializer : CCP_MemberDeclaration);
+  if (isa(ND))
+CCR.CursorKind = CXCursor_MemberRef;
+  return Results.AddResult(CCR);
+}
+return Results.AddResult(CodeCompletionResult(
+Builder.TakeString(),
+SawLastInitializer ? CCP_NextInitializer : CCP_MemberDeclaration));
+  };
+  auto AddCtorsWithName = [&](const CXXRecordDecl *RD, unsigned int Priority,
+  const char *Name, const FieldDecl *FD) {
+if (!RD)
+  return AddDefaultCtorInit(Name,
+FD ? Results.getAllocator().CopyString(
+ FD->getType().getAsString(Policy))
+

[PATCH] D53654: [clang] Improve ctor initializer completions.

2018-11-01 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC345844: [clang] Improve ctor initializer completions. 
(authored by kadircet, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D53654?vs=171926&id=172140#toc

Repository:
  rC Clang

https://reviews.llvm.org/D53654

Files:
  lib/Sema/SemaCodeComplete.cpp
  test/CodeCompletion/ctor-initializer.cpp
  test/Index/complete-ctor-inits.cpp
  test/Index/complete-cxx-inline-methods.cpp

Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -808,12 +808,20 @@
   }
 }
 
+DeclContext::lookup_result getConstructors(ASTContext &Context,
+   const CXXRecordDecl *Record) {
+  QualType RecordTy = Context.getTypeDeclType(Record);
+  DeclarationName ConstructorName =
+  Context.DeclarationNames.getCXXConstructorName(
+  Context.getCanonicalType(RecordTy));
+  return Record->lookup(ConstructorName);
+}
+
 void ResultBuilder::MaybeAddConstructorResults(Result R) {
   if (!SemaRef.getLangOpts().CPlusPlus || !R.Declaration ||
   !CompletionContext.wantConstructorResults())
 return;
 
-  ASTContext &Context = SemaRef.Context;
   const NamedDecl *D = R.Declaration;
   const CXXRecordDecl *Record = nullptr;
   if (const ClassTemplateDecl *ClassTemplate = dyn_cast(D))
@@ -831,16 +839,8 @@
   if (!Record)
 return;
 
-
-  QualType RecordTy = Context.getTypeDeclType(Record);
-  DeclarationName ConstructorName
-= Context.DeclarationNames.getCXXConstructorName(
-   Context.getCanonicalType(RecordTy));
-  DeclContext::lookup_result Ctors = Record->lookup(ConstructorName);
-  for (DeclContext::lookup_iterator I = Ctors.begin(),
-  E = Ctors.end();
-   I != E; ++I) {
-R.Declaration = *I;
+  for(auto Ctor : getConstructors(SemaRef.Context, Record)) {
+R.Declaration = Ctor;
 R.CursorKind = getCursorKindForDecl(R.Declaration);
 Results.push_back(R);
   }
@@ -5073,11 +5073,77 @@
   }
 
   // Add completions for base classes.
-  CodeCompletionBuilder Builder(Results.getAllocator(),
-Results.getCodeCompletionTUInfo());
   PrintingPolicy Policy = getCompletionPrintingPolicy(*this);
   bool SawLastInitializer = Initializers.empty();
   CXXRecordDecl *ClassDecl = Constructor->getParent();
+
+  auto GenerateCCS = [&](const NamedDecl *ND, const char *Name) {
+CodeCompletionBuilder Builder(Results.getAllocator(),
+  Results.getCodeCompletionTUInfo());
+Builder.AddTypedTextChunk(Name);
+Builder.AddChunk(CodeCompletionString::CK_LeftParen);
+if (auto Function = dyn_cast(ND))
+  AddFunctionParameterChunks(PP, Policy, Function, Builder);
+else if (auto FunTemplDecl = dyn_cast(ND))
+  AddFunctionParameterChunks(PP, Policy, FunTemplDecl->getTemplatedDecl(),
+ Builder);
+Builder.AddChunk(CodeCompletionString::CK_RightParen);
+return Builder.TakeString();
+  };
+  auto AddDefaultCtorInit = [&](const char *Name, const char *Type,
+const NamedDecl *ND) {
+CodeCompletionBuilder Builder(Results.getAllocator(),
+  Results.getCodeCompletionTUInfo());
+Builder.AddTypedTextChunk(Name);
+Builder.AddChunk(CodeCompletionString::CK_LeftParen);
+Builder.AddPlaceholderChunk(Type);
+Builder.AddChunk(CodeCompletionString::CK_RightParen);
+if (ND) {
+  auto CCR = CodeCompletionResult(
+  Builder.TakeString(), ND,
+  SawLastInitializer ? CCP_NextInitializer : CCP_MemberDeclaration);
+  if (isa(ND))
+CCR.CursorKind = CXCursor_MemberRef;
+  return Results.AddResult(CCR);
+}
+return Results.AddResult(CodeCompletionResult(
+Builder.TakeString(),
+SawLastInitializer ? CCP_NextInitializer : CCP_MemberDeclaration));
+  };
+  auto AddCtorsWithName = [&](const CXXRecordDecl *RD, unsigned int Priority,
+  const char *Name, const FieldDecl *FD) {
+if (!RD)
+  return AddDefaultCtorInit(Name,
+FD ? Results.getAllocator().CopyString(
+ FD->getType().getAsString(Policy))
+   : Name,
+FD);
+auto Ctors = getConstructors(Context, RD);
+if (Ctors.begin() == Ctors.end())
+  return AddDefaultCtorInit(Name, Name, RD);
+for (const auto Ctor : Ctors) {
+  auto CCR = CodeCompletionResult(GenerateCCS(Ctor, Name), RD, Priority);
+  CCR.CursorKind = getCursorKindForDecl(Ctor);
+  Results.AddResult(CCR);
+}
+  };
+  auto AddBase = [&](const CXXBaseSpecifier &Base) {
+const char *BaseName =
+Results.getAllocator().CopyString(

[PATCH] D53700: Support _Clang as a scoped attribute identifier

2018-11-01 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

Ping


https://reviews.llvm.org/D53700



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


[PATCH] D52421: [Sema] Diagnose parameter names that shadow inherited field names

2018-11-01 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

@rsmith -- any thoughts on this before I commit?


https://reviews.llvm.org/D52421



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


[PATCH] D53974: [clang-tidy] new checker: bugprone-too-small-loop-variable

2018-11-01 Thread Tamás Zolnai via Phabricator via cfe-commits
ztamas added a comment.

In https://reviews.llvm.org/D53974#1283833, @sberg wrote:

> > I run the new checker on LibreOffice project. I found ~25 false positives, 
> > which seems small enough to me. This false positives can be supressed 
> > easily.
>
> Do you have a link to such a false positive and how it got suppressed in the 
> LibreOffice code base?  (If those are included in the referenced 
> https://cgit.freedesktop.org/libreoffice/core/commit/?id=26ccd00bc96c585b7065af0dcce246b5bfaae5e1,
>  I failed to spot them.)


I uploaded here the output of the new checker on LibreOffice after my 
referenced commit:
https://gist.github.com/tzolnai/9c2c78323c098e2e09d63c1a1384274b

Search for bugprone-too-small-loop-variable in the log. Not all are false 
positives.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53974



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


[PATCH] D53514: os_log: make buffer size an integer constant expression.

2018-11-01 Thread Tim Northover via Phabricator via cfe-commits
t.p.northover closed this revision.
t.p.northover added a comment.

Thanks Eli. I committed it as r345828, and then had to fixup some linker 
dependencies on other platforms, which took me a couple of tries (r345833 and 
r345835).


Repository:
  rC Clang

https://reviews.llvm.org/D53514



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


[PATCH] D53979: [analyzer][CTU] Correctly signal in the function index generation tool if there was an error

2018-11-01 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun updated this revision to Diff 172134.
xazax.hun added a reviewer: a.sidorin.
xazax.hun added a comment.

- Remove yet another dependency from the tool that is no longer used.


https://reviews.llvm.org/D53979

Files:
  tools/clang-func-mapping/CMakeLists.txt
  tools/clang-func-mapping/ClangFnMapGen.cpp

Index: tools/clang-func-mapping/ClangFnMapGen.cpp
===
--- tools/clang-func-mapping/ClangFnMapGen.cpp
+++ tools/clang-func-mapping/ClangFnMapGen.cpp
@@ -14,23 +14,16 @@
 
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/ASTContext.h"
-#include "clang/AST/GlobalDecl.h"
-#include "clang/AST/Mangle.h"
-#include "clang/AST/StmtVisitor.h"
 #include "clang/Basic/SourceManager.h"
-#include "clang/Basic/TargetInfo.h"
 #include "clang/CrossTU/CrossTranslationUnit.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendActions.h"
-#include "clang/Index/USRGeneration.h"
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/Support/CommandLine.h"
-#include "llvm/Support/Path.h"
 #include "llvm/Support/Signals.h"
 #include 
 #include 
-#include 
 
 using namespace llvm;
 using namespace clang;
@@ -41,21 +34,22 @@
 
 class MapFunctionNamesConsumer : public ASTConsumer {
 public:
-  MapFunctionNamesConsumer(ASTContext &Context) : Ctx(Context) {}
+  MapFunctionNamesConsumer(ASTContext &Context)
+  : SM(Context.getSourceManager()) {}
 
   ~MapFunctionNamesConsumer() {
 // Flush results to standard output.
 llvm::outs() << createCrossTUIndexString(Index);
   }
 
-  virtual void HandleTranslationUnit(ASTContext &Ctx) {
+  void HandleTranslationUnit(ASTContext &Ctx) override {
 handleDecl(Ctx.getTranslationUnitDecl());
   }
 
 private:
   void handleDecl(const Decl *D);
 
-  ASTContext &Ctx;
+  SourceManager &SM;
   llvm::StringMap Index;
   std::string CurrentFileName;
 };
@@ -67,8 +61,6 @@
   if (const auto *FD = dyn_cast(D)) {
 if (FD->isThisDeclarationADefinition()) {
   if (const Stmt *Body = FD->getBody()) {
-std::string LookupName = CrossTranslationUnitContext::getLookupName(FD);
-const SourceManager &SM = Ctx.getSourceManager();
 if (CurrentFileName.empty()) {
   CurrentFileName =
   SM.getFileEntryForID(SM.getMainFileID())->tryGetRealPathName();
@@ -80,8 +72,11 @@
 case ExternalLinkage:
 case VisibleNoLinkage:
 case UniqueExternalLinkage:
-  if (SM.isInMainFile(Body->getBeginLoc()))
+  if (SM.isInMainFile(Body->getBeginLoc())) {
+std::string LookupName =
+CrossTranslationUnitContext::getLookupName(FD);
 Index[LookupName] = CurrentFileName;
+  }
 default:
   break;
 }
@@ -98,9 +93,7 @@
 protected:
   std::unique_ptr CreateASTConsumer(CompilerInstance &CI,
  llvm::StringRef) {
-std::unique_ptr PFC(
-new MapFunctionNamesConsumer(CI.getASTContext()));
-return PFC;
+return llvm::make_unique(CI.getASTContext());
   }
 };
 
@@ -119,6 +112,6 @@
 
   ClangTool Tool(OptionsParser.getCompilations(),
  OptionsParser.getSourcePathList());
-  Tool.run(newFrontendActionFactory().get());
-  return 0;
+  
+  return Tool.run(newFrontendActionFactory().get());
 }
Index: tools/clang-func-mapping/CMakeLists.txt
===
--- tools/clang-func-mapping/CMakeLists.txt
+++ tools/clang-func-mapping/CMakeLists.txt
@@ -1,8 +1,6 @@
 set(LLVM_LINK_COMPONENTS
   ${LLVM_TARGETS_TO_BUILD}
-  asmparser
   support
-  mc
   )
 
 add_clang_executable(clang-func-mapping
@@ -15,7 +13,6 @@
   clangBasic
   clangCrossTU
   clangFrontend
-  clangIndex
   clangTooling
   )
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D53979: [analyzer][CTU] Correctly signal in the function index generation tool if there was an error

2018-11-01 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun created this revision.
xazax.hun added reviewers: NoQ, george.karpenkov, Szelethus, martong.
Herald added subscribers: dkrupp, donat.nagy, arphaman, mikhail.ramalho, 
a.sidorin, rnkovacs, szepet, baloghadamsoftware, whisperity, mgorny.

Do not always return 0 to the OS. This will make it easier for orchestration 
tools like CodeChecker to detect errors.
Also small NFC cleanups along the way.


Repository:
  rC Clang

https://reviews.llvm.org/D53979

Files:
  tools/clang-func-mapping/CMakeLists.txt
  tools/clang-func-mapping/ClangFnMapGen.cpp

Index: tools/clang-func-mapping/ClangFnMapGen.cpp
===
--- tools/clang-func-mapping/ClangFnMapGen.cpp
+++ tools/clang-func-mapping/ClangFnMapGen.cpp
@@ -14,23 +14,17 @@
 
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/ASTContext.h"
-#include "clang/AST/GlobalDecl.h"
-#include "clang/AST/Mangle.h"
-#include "clang/AST/StmtVisitor.h"
 #include "clang/Basic/SourceManager.h"
-#include "clang/Basic/TargetInfo.h"
 #include "clang/CrossTU/CrossTranslationUnit.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Index/USRGeneration.h"
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/Support/CommandLine.h"
-#include "llvm/Support/Path.h"
 #include "llvm/Support/Signals.h"
 #include 
 #include 
-#include 
 
 using namespace llvm;
 using namespace clang;
@@ -41,21 +35,22 @@
 
 class MapFunctionNamesConsumer : public ASTConsumer {
 public:
-  MapFunctionNamesConsumer(ASTContext &Context) : Ctx(Context) {}
+  MapFunctionNamesConsumer(ASTContext &Context)
+  : SM(Context.getSourceManager()) {}
 
   ~MapFunctionNamesConsumer() {
 // Flush results to standard output.
 llvm::outs() << createCrossTUIndexString(Index);
   }
 
-  virtual void HandleTranslationUnit(ASTContext &Ctx) {
+  void HandleTranslationUnit(ASTContext &Ctx) override {
 handleDecl(Ctx.getTranslationUnitDecl());
   }
 
 private:
   void handleDecl(const Decl *D);
 
-  ASTContext &Ctx;
+  SourceManager &SM;
   llvm::StringMap Index;
   std::string CurrentFileName;
 };
@@ -67,8 +62,6 @@
   if (const auto *FD = dyn_cast(D)) {
 if (FD->isThisDeclarationADefinition()) {
   if (const Stmt *Body = FD->getBody()) {
-std::string LookupName = CrossTranslationUnitContext::getLookupName(FD);
-const SourceManager &SM = Ctx.getSourceManager();
 if (CurrentFileName.empty()) {
   CurrentFileName =
   SM.getFileEntryForID(SM.getMainFileID())->tryGetRealPathName();
@@ -80,8 +73,11 @@
 case ExternalLinkage:
 case VisibleNoLinkage:
 case UniqueExternalLinkage:
-  if (SM.isInMainFile(Body->getBeginLoc()))
+  if (SM.isInMainFile(Body->getBeginLoc())) {
+std::string LookupName =
+CrossTranslationUnitContext::getLookupName(FD);
 Index[LookupName] = CurrentFileName;
+  }
 default:
   break;
 }
@@ -98,9 +94,7 @@
 protected:
   std::unique_ptr CreateASTConsumer(CompilerInstance &CI,
  llvm::StringRef) {
-std::unique_ptr PFC(
-new MapFunctionNamesConsumer(CI.getASTContext()));
-return PFC;
+return llvm::make_unique(CI.getASTContext());
   }
 };
 
@@ -119,6 +113,6 @@
 
   ClangTool Tool(OptionsParser.getCompilations(),
  OptionsParser.getSourcePathList());
-  Tool.run(newFrontendActionFactory().get());
-  return 0;
+  
+  return Tool.run(newFrontendActionFactory().get());
 }
Index: tools/clang-func-mapping/CMakeLists.txt
===
--- tools/clang-func-mapping/CMakeLists.txt
+++ tools/clang-func-mapping/CMakeLists.txt
@@ -1,8 +1,6 @@
 set(LLVM_LINK_COMPONENTS
   ${LLVM_TARGETS_TO_BUILD}
-  asmparser
   support
-  mc
   )
 
 add_clang_executable(clang-func-mapping
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D53949: [clang][CodeGen] Implicit Conversion Sanitizer: discover the world of CompoundAssign operators

2018-11-01 Thread John Regehr via Phabricator via cfe-commits
regehr added a comment.

Regarding ++ and --, I think for now it's fine to just document that these 
aren't instrumented.


Repository:
  rC Clang

https://reviews.llvm.org/D53949



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


[PATCH] D53974: [clang-tidy] new checker: bugprone-too-small-loop-variable

2018-11-01 Thread Tamás Zolnai via Phabricator via cfe-commits
ztamas added a comment.

In https://reviews.llvm.org/D53974#1283759, @ZaMaZaN4iK wrote:

> Hmm, i thought Clang has some warning for this, but I was wrong... Did you 
> think to implement this check as Clang warning?


It did not come in my mind to do that. It might be good idea, I guess. This 
version of the check however might find too much false positives for a clang 
warning.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53974



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


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

2018-11-01 Thread Bevin Hansson via Phabricator via cfe-commits
ebevhan added a comment.

In https://reviews.llvm.org/D53738#1283459, @rjmccall wrote:

> Well, it could be passed around through most code as some sort of 
> abstractly-represented intermediate "type" which could be either a QualType 
> or a fixed-point semantics.


Sounds to me like you're describing a new `Type` that can contain an arbitrary 
fixed-point semantic :)

It still feels like this just introduces inconsistencies into the form of the 
AST. If we do add this extra type object to the BO, won't people wonder why we 
use ImplicitCastExpr for non-fixedpoint operations but use the special 
`QualTypeOrFPSemantics BinaryOperator::ComputationType;` for fixedpoint 
operations even though they both have nearly the same semantic meaning 
(converting to a common type before doing the operation)?

(The difference being that using the `ComputationType` requires you to cast 
back to the result type afterwards.)

> 
> 
>> It might just be easier to store the full-precision info in BO directly. BO 
>> might be too common to warrant the size increase, though. 
>> FixedPointSemantics can probably be optimized to only take 32 bits.
> 
> What you can definitely do is store a bit in BO saying that there's extra 
> storage for the intermediate "type".

Is this similar to how ExtQuals works? How would this be implemented?

>> As a side note, comparisons are still a bit up in the air. I don't think we 
>> came to a conclusion on whether they should be done in full precision or 
>> bitwise. The spec isn't clear.
> 
> ...bitwise?

The spec uses different wording for the arithmetic operations and comparison 
operations, and it's not entirely obvious what it means. For the arithmetic 
operators, it has the whole section on finding the full precision common type, 
but for comparisons it says:

> When comparing fixed-point values with fixed-point values or integer values,
>  the values are compared directly; the values of the operands are not 
> converted before the
>  comparison is made.

What 'directly' means in conjunction with 'the operands are not converted' is 
not clear. It's reasonable to assume that it either means comparing value-wise 
(so, the same as finding a common type that fits all values and then comparing; 
though this would obviously require conversion), or perhaps 'directly' means a 
bitwise (representation) comparison. The latter seems a bit farfetched to me, 
though.


Repository:
  rC Clang

https://reviews.llvm.org/D53738



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


[PATCH] D50318: Support Swift in platform availability attribute

2018-11-01 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM aside from a few small nits.




Comment at: lib/Parse/ParseDecl.cpp:1005
+if (Keyword == Ident_deprecated && Platform->Ident &&
+Platform->Ident->getName() == "swift") {
+  // For swift, we deprecate for all versions.

You can use `Platform->Ident->isStr("swift")` here instead.



Comment at: lib/Parse/ParseDecl.cpp:1007
+  // For swift, we deprecate for all versions.
+  if (!Changes[Deprecated].KeywordLoc.isInvalid()) {
+Diag(KeywordLoc, diag::err_availability_redundant)

Might as well switch the logic around to using `isValid()` instead.



Comment at: lib/Sema/SemaDeclAttr.cpp:2368
 
+  if (II->getName() == "swift") {
+if (Introduced.isValid() || Obsoleted.isValid() ||

`II->isStr()` here as well.


Repository:
  rC Clang

https://reviews.llvm.org/D50318



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


r345839 - Multiversioning- Ensure all MV functions are emitted.

2018-11-01 Thread Erich Keane via cfe-commits
Author: erichkeane
Date: Thu Nov  1 08:11:43 2018
New Revision: 345839

URL: http://llvm.org/viewvc/llvm-project?rev=345839&view=rev
Log:
Multiversioning- Ensure all MV functions are emitted.

Multiverson function versions are always used (by the resolver), so ensure that
they are always emitted.

Change-Id: I5d2e0841fddf0d18918b3fb92ae76814add7ee96

Modified:
cfe/trunk/lib/AST/ASTContext.cpp
cfe/trunk/test/CodeGen/attr-target-mv.c
cfe/trunk/test/CodeGenCXX/attr-cpuspecific.cpp
cfe/trunk/test/CodeGenCXX/attr-target-mv-member-funcs.cpp

Modified: cfe/trunk/lib/AST/ASTContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=345839&r1=345838&r2=345839&view=diff
==
--- cfe/trunk/lib/AST/ASTContext.cpp (original)
+++ cfe/trunk/lib/AST/ASTContext.cpp Thu Nov  1 08:11:43 2018
@@ -9773,6 +9773,10 @@ bool ASTContext::DeclMustBeEmitted(const
 return true;
 
   if (const auto *FD = dyn_cast(D)) {
+// Multiversioned functions always have to be emitted, because they are 
used
+// by the resolver.
+if (FD->isMultiVersion())
+  return true;
 // Forward declarations aren't required.
 if (!FD->doesThisDeclarationHaveABody())
   return FD->doesDeclarationForceExternallyVisibleDefinition();

Modified: cfe/trunk/test/CodeGen/attr-target-mv.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/attr-target-mv.c?rev=345839&r1=345838&r2=345839&view=diff
==
--- cfe/trunk/test/CodeGen/attr-target-mv.c (original)
+++ cfe/trunk/test/CodeGen/attr-target-mv.c Thu Nov  1 08:11:43 2018
@@ -72,6 +72,16 @@ void bar4() {
 // WINDOWS: call i32 @foo.sse4.2
 // WINDOWS: call i32 @foo
 
+// LINUX: define linkonce i32 @foo_inline.arch_ivybridge()
+// LINUX: ret i32 1
+// LINUX: define linkonce i32 @foo_inline()
+// LINUX: ret i32 2
+
+// WINDOWS: define linkonce_odr dso_local i32 @foo_inline.arch_ivybridge()
+// WINDOWS: ret i32 1
+// WINDOWS: define linkonce_odr dso_local i32 @foo_inline()
+// WINDOWS: ret i32 2
+
 // LINUX: define i32 @bar2()
 // LINUX: call i32 @foo_inline.ifunc()
 
@@ -106,6 +116,22 @@ void bar4() {
 // WINDOWS: call void @foo_decls.sse4.2
 // Windows: call void @foo_decls
 
+// LINUX: define linkonce void @foo_decls()
+// LINUX: define linkonce void @foo_decls.sse4.2()
+
+// WINDOWS: define linkonce_odr dso_local void @foo_decls()
+// WINDOWS: define linkonce_odr dso_local void @foo_decls.sse4.2()
+
+// LINUX: define linkonce void @foo_multi(i32 %{{[^,]+}}, double %{{[^\)]+}})
+// LINUX: define linkonce void @foo_multi.avx_sse4.2(i32 %{{[^,]+}}, double 
%{{[^\)]+}})
+// LINUX: define linkonce void @foo_multi.fma4_sse4.2(i32 %{{[^,]+}}, double 
%{{[^\)]+}})
+// LINUX: define linkonce void @foo_multi.arch_ivybridge_fma4_sse4.2(i32 
%{{[^,]+}}, double %{{[^\)]+}})
+
+// WINDOWS: define linkonce_odr dso_local void @foo_multi(i32 %{{[^,]+}}, 
double %{{[^\)]+}})
+// WINDOWS: define linkonce_odr dso_local void @foo_multi.avx_sse4.2(i32 
%{{[^,]+}}, double %{{[^\)]+}})
+// WINDOWS: define linkonce_odr dso_local void @foo_multi.fma4_sse4.2(i32 
%{{[^,]+}}, double %{{[^\)]+}})
+// WINDOWS: define linkonce_odr dso_local void 
@foo_multi.arch_ivybridge_fma4_sse4.2(i32 %{{[^,]+}}, double %{{[^\)]+}})
+
 // LINUX: define void @bar4()
 // LINUX: call void @foo_multi.ifunc(i32 1, double 5.{{[0+e]*}})
 
@@ -156,26 +182,3 @@ void bar4() {
 
 // WINDOWS: declare dso_local i32 @foo_inline.arch_sandybridge()
 
-// LINUX: define linkonce i32 @foo_inline.arch_ivybridge()
-// LINUX: ret i32 1
-// LINUX: define linkonce i32 @foo_inline()
-// LINUX: ret i32 2
-
-// WINDOWS: define linkonce_odr dso_local i32 @foo_inline.arch_ivybridge()
-// WINDOWS: ret i32 1
-// WINDOWS: define linkonce_odr dso_local i32 @foo_inline()
-// WINDOWS: ret i32 2
-
-// LINUX: define linkonce void @foo_decls()
-// LINUX: define linkonce void @foo_decls.sse4.2()
-
-// WINDOWS: define linkonce_odr dso_local void @foo_decls()
-// WINDOWS: define linkonce_odr dso_local void @foo_decls.sse4.2()
-
-// LINUX: define linkonce void @foo_multi.avx_sse4.2(i32 %{{[^,]+}}, double 
%{{[^\)]+}})
-// LINUX: define linkonce void @foo_multi.fma4_sse4.2(i32 %{{[^,]+}}, double 
%{{[^\)]+}})
-// LINUX: define linkonce void @foo_multi.arch_ivybridge_fma4_sse4.2(i32 
%{{[^,]+}}, double %{{[^\)]+}})
-
-// WINDOWS: define linkonce_odr dso_local void @foo_multi.avx_sse4.2(i32 
%{{[^,]+}}, double %{{[^\)]+}})
-// WINDOWS: define linkonce_odr dso_local void @foo_multi.fma4_sse4.2(i32 
%{{[^,]+}}, double %{{[^\)]+}})
-// WINDOWS: define linkonce_odr dso_local void 
@foo_multi.arch_ivybridge_fma4_sse4.2(i32 %{{[^,]+}}, double %{{[^\)]+}})

Modified: cfe/trunk/test/CodeGenCXX/attr-cpuspecific.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/attr-cpuspecific.cpp?rev=345839&r1=345838&r2=345839&view=diff

r345838 - CPU-Dispatch- Fix type of a member function, prevent deferrals

2018-11-01 Thread Erich Keane via cfe-commits
Author: erichkeane
Date: Thu Nov  1 08:11:41 2018
New Revision: 345838

URL: http://llvm.org/viewvc/llvm-project?rev=345838&view=rev
Log:
CPU-Dispatch- Fix type of a member function, prevent deferrals

The member type creation for a cpu-dispatch function was not correctly
including the 'this' parameter, so ensure that the type is properly
determined. Also, disable defer in the cases of emitting the functoins,
as it can end up resulting in the wrong version being emitted.

Change-Id: I0b8fc5e0b0d1ae1a9d98fd54f35f27f6e5d5d083

Added:
cfe/trunk/test/CodeGenCXX/attr-cpuspecific.cpp   (with props)
Modified:
cfe/trunk/lib/CodeGen/CodeGenModule.cpp

Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=345838&r1=345837&r2=345838&view=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Thu Nov  1 08:11:41 2018
@@ -2538,7 +2538,13 @@ void CodeGenModule::emitCPUDispatchDefin
   assert(FD && "Not a FunctionDecl?");
   const auto *DD = FD->getAttr();
   assert(DD && "Not a cpu_dispatch Function?");
-  llvm::Type *DeclTy = getTypes().ConvertTypeForMem(FD->getType());
+  QualType CanonTy = Context.getCanonicalType(FD->getType());
+  llvm::Type *DeclTy = getTypes().ConvertFunctionType(CanonTy, FD);
+
+  if (const auto *CXXFD = dyn_cast(FD)) {
+const CGFunctionInfo &FInfo = 
getTypes().arrangeCXXMethodDeclaration(CXXFD);
+DeclTy = getTypes().GetFunctionType(FInfo);
+  }
 
   StringRef ResolverName = getMangledName(GD);
 
@@ -2564,7 +2570,7 @@ void CodeGenModule::emitCPUDispatchDefin
 std::string MangledName = getMangledNameImpl(*this, GD, FD, true) +
   getCPUSpecificMangling(*this, II->getName());
 llvm::Constant *Func = GetOrCreateLLVMFunction(
-MangledName, DeclTy, GD, /*ForVTable=*/false, /*DontDefer=*/false,
+MangledName, DeclTy, GD, /*ForVTable=*/false, /*DontDefer=*/true,
 /*IsThunk=*/false, llvm::AttributeList(), ForDefinition);
 llvm::SmallVector Features;
 Target.getCPUSpecificCPUDispatchFeatures(II->getName(), Features);

Added: cfe/trunk/test/CodeGenCXX/attr-cpuspecific.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/attr-cpuspecific.cpp?rev=345838&view=auto
==
--- cfe/trunk/test/CodeGenCXX/attr-cpuspecific.cpp (added)
+++ cfe/trunk/test/CodeGenCXX/attr-cpuspecific.cpp Thu Nov  1 08:11:41 2018
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -o - %s | FileCheck %s 
--check-prefixes=CHECK,LINUX
+// RUN: %clang_cc1 -triple x86_64-windows-pc -fms-compatibility -emit-llvm -o 
- %s | FileCheck %s --check-prefixes=CHECK,WINDOWS
+
+struct S {
+  __attribute__((cpu_specific(atom)))
+  void Func(){}
+  __attribute__((cpu_dispatch(ivybridge,atom)))
+  void Func(){}
+};
+
+void foo() {
+  S s;
+  s.Func();
+}
+
+// LINUX: define void (%struct.S*)* @_ZN1S4FuncEv.resolver
+// LINUX: ret void (%struct.S*)* @_ZN1S4FuncEv.S
+// LINUX: ret void (%struct.S*)* @_ZN1S4FuncEv.O
+
+// WINDOWS: define dso_local void @"?Func@S@@QEAAXXZ"(%struct.S*)
+// WINDOWS: musttail call void @"?Func@S@@QEAAXXZ.S"(%struct.S* %0)
+// WINDOWS: musttail call void @"?Func@S@@QEAAXXZ.O"(%struct.S* %0)
+

Propchange: cfe/trunk/test/CodeGenCXX/attr-cpuspecific.cpp
--
svn:eol-style = native

Propchange: cfe/trunk/test/CodeGenCXX/attr-cpuspecific.cpp
--
svn:keywords = "Author Date Id Rev URL"

Propchange: cfe/trunk/test/CodeGenCXX/attr-cpuspecific.cpp
--
svn:mime-type = text/plain


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


[PATCH] D50318: Support Swift in platform availability attribute

2018-11-01 Thread Jeff Muizelaar via Phabricator via cfe-commits
jrmuizel added a comment.

Review ping


Repository:
  rC Clang

https://reviews.llvm.org/D50318



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


[PATCH] D53141: [OpenMP][libomptarget] Add runtime function for pushing coalesced global records

2018-11-01 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev accepted this revision.
ABataev added a comment.

LG


Repository:
  rOMP OpenMP

https://reviews.llvm.org/D53141



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


[PATCH] D53974: [clang-tidy] new checker: bugprone-too-small-loop-variable

2018-11-01 Thread Tamás Zolnai via Phabricator via cfe-commits
ztamas added a comment.

In https://reviews.llvm.org/D53974#1283833, @sberg wrote:

> > I run the new checker on LibreOffice project. I found ~25 false positives, 
> > which seems small enough to me. This false positives can be supressed 
> > easily.
>
> Do you have a link to such a false positive and how it got suppressed in the 
> LibreOffice code base?  (If those are included in the referenced 
> https://cgit.freedesktop.org/libreoffice/core/commit/?id=26ccd00bc96c585b7065af0dcce246b5bfaae5e1,
>  I failed to spot them.)


I did not supress anything yet, since this checker is still work in progress. 
The final version might avoid those false postives.
One example in basic/source/runtime/dllmgr-x86.cxx
This line:
for (sal_uInt16 i = 1; i < (arguments == 0 ? 0 : arguments->Count()); ++i)

arguments->Count() returns a sal_uInt16 too, which is unsigned short, however 
"0" is an int literal so the "<>?<>:<>" expression will have an int type.
To supress that you can cast the literal:

for (sal_uInt16 i = 1; i < (arguments == 0 ? (sal_uInt16)0 : 
arguments->Count()); ++i)

Or cast the whole expression:

for (sal_uInt16 i = 1; i < (sal_uInt16)(arguments == 0 ? 0 : 
arguments->Count()); ++i)

Or use int for the loop variable:

for (int i = 1; i < (arguments == 0 ? 0 : arguments->Count()); ++i)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53974



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


[PATCH] D53974: [clang-tidy] new checker: bugprone-too-small-loop-variable

2018-11-01 Thread Stephan Bergmann via Phabricator via cfe-commits
sberg added a comment.

> I run the new checker on LibreOffice project. I found ~25 false positives, 
> which seems small enough to me. This false positives can be supressed easily.

Do you have a link to such a false positive and how it got suppressed in the 
LibreOffice code base?  (If those are included in the referenced 
https://cgit.freedesktop.org/libreoffice/core/commit/?id=26ccd00bc96c585b7065af0dcce246b5bfaae5e1,
 I failed to spot them.)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53974



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


r345835 - Logging: put link against libclangAnalysis rather than libLLVMAnalysis for os_log

2018-11-01 Thread Tim Northover via cfe-commits
Author: tnorthover
Date: Thu Nov  1 07:43:35 2018
New Revision: 345835

URL: http://llvm.org/viewvc/llvm-project?rev=345835&view=rev
Log:
Logging: put link against libclangAnalysis rather than libLLVMAnalysis for 
os_log

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

Modified: cfe/trunk/lib/AST/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/CMakeLists.txt?rev=345835&r1=345834&r2=345835&view=diff
==
--- cfe/trunk/lib/AST/CMakeLists.txt (original)
+++ cfe/trunk/lib/AST/CMakeLists.txt Thu Nov  1 07:43:35 2018
@@ -1,5 +1,4 @@
 set(LLVM_LINK_COMPONENTS
-  Analysis
   BinaryFormat
   Support
   )
@@ -73,6 +72,7 @@ add_clang_library(clangAST
   VTTBuilder.cpp
 
   LINK_LIBS
+  clangAnalysis
   clangBasic
   clangLex
   )


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


[PATCH] D52949: [Diagnostics] Implement -Wsizeof-pointer-div

2018-11-01 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM with a minor formatting nit.




Comment at: lib/Sema/SemaExpr.cpp:8729
 
+static void DiagnoseDivisionSizeofPointer(Sema &S, Expr *LHS, Expr *RHS,
+  SourceLocation Loc) {

xbolva00 wrote:
> aaron.ballman wrote:
> > `const Expr *`
> if const, i have build issues with "<< LHS"
That sounds like a bug with the diagnostic builder, but it can be addressed 
separately.



Comment at: test/Sema/div-sizeof-ptr.cpp:1
+// RUN: %clang_cc1 %s -verify -Wsizeof-pointer-div -fsyntax-only
+

Please run this file through clang-format as the formatting appears to be off.



Comment at: test/Sema/div-sizeof-ptr.cpp:27
+int arr2[10][12];
+int b8 = sizeof(arr2) / sizeof(*arr2); 
+}

GCC does not warn here, but I question whether we should. This evaluates to 
`10`, but is that what the user expects it to evaluate to, as opposed to `120`?

Regardless, I think it's reasonable to not warn on this right now (it would 
need different diagnostic text).


https://reviews.llvm.org/D52949



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


[PATCH] D53023: Prototype OpenCL BIFs using Tablegen

2018-11-01 Thread Joey Gouly via Phabricator via cfe-commits
joey updated this revision to Diff 172122.
joey added a comment.

I re-worked where the builtins are actually inserted, now it's in a similar 
place to the "normal" clang builtins.

I addressed the issue of putting the return type also into the args type (could 
rename this to signature/proto table to make it more obvious).

I'm planning to write an RFC to get buy-in from the rest of the community 
before doing too much else to the patch.


https://reviews.llvm.org/D53023

Files:
  include/clang/Basic/CMakeLists.txt
  include/clang/Basic/OpenCLBuiltins.td
  lib/Sema/SemaLookup.cpp
  test/SemaOpenCL/builtin-new.cl
  utils/TableGen/CMakeLists.txt
  utils/TableGen/ClangOpenCLBuiltinEmitter.cpp
  utils/TableGen/TableGen.cpp
  utils/TableGen/TableGenBackends.h

Index: utils/TableGen/TableGenBackends.h
===
--- utils/TableGen/TableGenBackends.h
+++ utils/TableGen/TableGenBackends.h
@@ -78,6 +78,7 @@
 void EmitTestPragmaAttributeSupportedAttributes(llvm::RecordKeeper &Records,
 llvm::raw_ostream &OS);
 
+void EmitClangOpenCLBuiltins(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
 } // end namespace clang
 
 #endif
Index: utils/TableGen/TableGen.cpp
===
--- utils/TableGen/TableGen.cpp
+++ utils/TableGen/TableGen.cpp
@@ -61,7 +61,8 @@
   GenDiagDocs,
   GenOptDocs,
   GenDataCollectors,
-  GenTestPragmaAttributeSupportedAttributes
+  GenTestPragmaAttributeSupportedAttributes,
+  GenClangOpenCLBuiltins,
 };
 
 namespace {
@@ -161,7 +162,9 @@
 clEnumValN(GenTestPragmaAttributeSupportedAttributes,
"gen-clang-test-pragma-attribute-supported-attributes",
"Generate a list of attributes supported by #pragma clang "
-   "attribute for testing purposes")));
+   "attribute for testing purposes"),
+clEnumValN(GenClangOpenCLBuiltins, "gen-clang-opencl-builtins",
+   "Generate OpenCL builtin handlers")));
 
 cl::opt
 ClangComponent("clang-component",
@@ -288,6 +291,9 @@
   case GenTestPragmaAttributeSupportedAttributes:
 EmitTestPragmaAttributeSupportedAttributes(Records, OS);
 break;
+  case GenClangOpenCLBuiltins:
+EmitClangOpenCLBuiltins(Records, OS);
+break;
   }
 
   return false;
Index: utils/TableGen/ClangOpenCLBuiltinEmitter.cpp
===
--- /dev/null
+++ utils/TableGen/ClangOpenCLBuiltinEmitter.cpp
@@ -0,0 +1,189 @@
+//===- ClangOpenCLBuiltinEmitter.cpp - Generate Clang OpenCL Builtin handling
+//=-*- C++ -*--=//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// This tablegen backend emits Clang OpenCL Builtin checking code.
+//
+//===--===//
+
+#include "llvm/ADT/MapVector.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringSet.h"
+#include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/raw_ostream.h"
+#include "llvm/TableGen/Error.h"
+#include "llvm/TableGen/Record.h"
+#include "llvm/TableGen/StringMatcher.h"
+#include "llvm/TableGen/TableGenBackend.h"
+
+#include 
+
+using namespace llvm;
+
+namespace {
+class BuiltinNameEmitter {
+public:
+  BuiltinNameEmitter(RecordKeeper &Records, raw_ostream &OS)
+  : Records(Records), OS(OS) {}
+
+  void Emit();
+
+private:
+  RecordKeeper &Records;
+  raw_ostream &OS;
+
+  void EmitDeclarations();
+  void EmitTable();
+  void GetOverloads();
+
+  MapVector>>
+  OverloadInfo;
+  std::vector, unsigned>> ArgTypesSet;
+};
+} // namespace
+
+void BuiltinNameEmitter::GetOverloads() {
+  unsigned CumulativeArgIndex = 0;
+  std::vector Builtins = Records.getAllDerivedDefinitions("Builtin");
+  for (const auto *B : Builtins) {
+StringRef BName = B->getValueAsString("name");
+
+if (OverloadInfo.find(BName) == OverloadInfo.end()) {
+  OverloadInfo.insert(std::make_pair(
+  BName, std::vector>{}));
+}
+
+auto Args = B->getValueAsListOfDefs("args");
+auto it =
+std::find_if(ArgTypesSet.begin(), ArgTypesSet.end(),
+ [&](const std::pair, unsigned> &a) {
+   return a.first == Args;
+ });
+unsigned ArgIndex;
+if (it == ArgTypesSet.end()) {
+  ArgTypesSet.push_back(std::make_pair(Args, CumulativeArgIndex));
+  ArgIndex = CumulativeArgIndex;
+  CumulativeArgIndex += Args.size();
+} else {
+  ArgIndex = it->second;
+}
+OverloadInfo[BName].push_back(std::make_pair(B, ArgIndex));
+  }
+}
+
+void B

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-11-01 Thread Hans Wennborg via Phabricator via cfe-commits
hans added inline comments.



Comment at: clang/include/clang/Driver/CLCompatOptions.td:336
   HelpText<"Disable precompiled headers, overrides /Yc and /Yu">;
+def _SLASH_Zc_dllexportInlines : CLFlag<"Zc:dllexportInlines">;
+def _SLASH_Zc_dllexportInlines_ : CLFlag<"Zc:dllexportInlines-">;

Just one more small thing I remembered: please add a test in 
tests/Driver/cl-options.c to make sure we translate this to the cc1 flag 
correctly.


https://reviews.llvm.org/D51340



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


[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

2018-11-01 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment.

maybe @aaron.ballman wants to take a look too


https://reviews.llvm.org/D52835



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


[PATCH] D53974: [clang-tidy] new checker: bugprone-too-small-loop-variable

2018-11-01 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments.



Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:26
+
+
+/// \brief The matcher for loops with suspicious integer loop variable.

Please remove unnecessary empty line.



Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:41
+void TooSmallLoopVariableCheck::registerMatchers(MatchFinder *Finder) {
+
+  const StatementMatcher LoopVarMatcher =

Please remove unnecessary empty line.



Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:85
+
+
+/// \brief Returns the positive part of the integer width for an integer type

Please remove unnecessary empty line.



Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:88
+unsigned calcPositiveBits(const ASTContext &Context, const QualType 
&IntExprType) {
+
+  assert(IntExprType->isIntegerType());

Please remove unnecessary empty line.



Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:96
+
+
+/// \brief Calculate the condition expression's positive bits, but ignore 
constant like values to reduce false positives

Please remove unnecessary empty line.



Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:99
+unsigned calcCondExprPositiveBits(const ASTContext &Context, const Expr 
*CondExpr, const QualType &CondExprType) {
+
+  unsigned CondExprPosBits = 0;

Please remove unnecessary empty line.



Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:105
+  // We are interested in variable values' positive bits
+  if(const auto *BinOperator = dyn_cast(CondExpr)) {
+const Expr *RHSE = BinOperator->getRHS()->IgnoreParenImpCasts();

Please run Clang-format.



Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:108
+const Expr *LHSE = BinOperator->getLHS()->IgnoreParenImpCasts();
+
+

Please remove unnecessary empty line.



Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:114
+
+
+const QualType RHSEType = RHSE->getType();

Please remove unnecessary empty line.



Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:121
+
+
+if(RHSEType->isEnumeralType() || RHSEType.isConstQualified() ||

Please remove unnecessary empty line.



Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:123
+if(RHSEType->isEnumeralType() || RHSEType.isConstQualified() ||
+   RHSE->getBeginLoc().isMacroID() || isa(RHSE)) {
+

Braces are not needed.



Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:126
+  CondExprPosBits = calcPositiveBits(Context, LHSEType);
+
+}

Please remove unnecessary empty line.



Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:129
+else if (LHSEType->isEnumeralType() || LHSEType.isConstQualified() ||
+ LHSE->getBeginLoc().isMacroID() || isa(LHSE)) {
+

Braces are not needed.



Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:130
+ LHSE->getBeginLoc().isMacroID() || isa(LHSE)) {
+
+  CondExprPosBits = calcPositiveBits(Context, RHSEType);

Please remove unnecessary empty line.



Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:132
+  CondExprPosBits = calcPositiveBits(Context, RHSEType);
+
+} else {

Please remove unnecessary empty line.



Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:133
+
+} else {
+

Braces are not needed.



Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:134
+} else {
+
+  CondExprPosBits = std::max(calcPositiveBits(Context, LHSEType),

Please remove unnecessary empty line.



Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:138
+}
+  } else {
+

Braces are not needed.



Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:139
+  } else {
+
+CondExprPosBits = calcPositiveBits(Context, CondExprType);

Please remove unnecessary empty line.



Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:141
+CondExprPosBits = calcPositiveBits(Context, CondExprType);
+
+  }

Please remove unnecessary empty line.



Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:149
+void TooSmallLoopVariableCheck::check(const MatchFinder::MatchResult &Result) {
+
+  const auto *LoopVar = Result.Nodes.getNodeAs(loopVarName);

Please remove unnecessary empty line.



Comment at: clang-tidy/bugprone/TooSmallLoopVariableC

[PATCH] D33672: [analyzer] INT50-CPP. Do not cast to an out-of-range enumeration checker

2018-11-01 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In https://reviews.llvm.org/D33672#1283778, @ZaMaZaN4iK wrote:

> Which other changes and/or approvals are required for merging into trunk?


There's one unresolved comment from earlier and a few very minor nits that I 
found, but I think this is ready to land otherwise. Do you need someone to 
commit on your behalf? If so, then please rebase off ToT and update the patch 
and someone here can commit for you.




Comment at: lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp:84-86
+  new BuiltinBug(this, "enum cast out of range",
+ "the value provided to the cast expression is not in "
+ "the valid range of values for the enum"));

NoQ wrote:
> > diagnostics should not be full sentences or grammatically correct, so drop 
> > the capitalization and full stop
> 
> No, in fact, Static Analyzer diagnostics are traditionally capitalized, 
> unlike other warnings, so i'm afraid this will need to be changed back >.< 
> Still no fullstop though.
This comment is still unresolved.



Comment at: lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp:96
+  // Get the value of the expression to cast.
+  const auto ValueToCastOptional =
+  C.getSVal(CE->getSubExpr()).getAs();

`const auto *`



Comment at: test/Analysis/enum-cast-out-of-range.cpp:38
+struct S {
+unscoped_unspecified_t E : 5;
+};

Formatting is off here.


https://reviews.llvm.org/D33672



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


r345833 - Logging: add CMake dependency so libAST can use OSLog analysis.

2018-11-01 Thread Tim Northover via cfe-commits
Author: tnorthover
Date: Thu Nov  1 07:22:20 2018
New Revision: 345833

URL: http://llvm.org/viewvc/llvm-project?rev=345833&view=rev
Log:
Logging: add CMake dependency so libAST can use OSLog analysis.

Should fix bots on platforms with slightly different symbol resolution
semantics.

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

Modified: cfe/trunk/lib/AST/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/CMakeLists.txt?rev=345833&r1=345832&r2=345833&view=diff
==
--- cfe/trunk/lib/AST/CMakeLists.txt (original)
+++ cfe/trunk/lib/AST/CMakeLists.txt Thu Nov  1 07:22:20 2018
@@ -1,4 +1,5 @@
 set(LLVM_LINK_COMPONENTS
+  Analysis
   BinaryFormat
   Support
   )


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


[PATCH] D33672: [analyzer] INT50-CPP. Do not cast to an out-of-range enumeration checker

2018-11-01 Thread Alexander Zaitsev via Phabricator via cfe-commits
ZaMaZaN4iK added a comment.

Which other changes and/or approvals are required for merging into trunk?


https://reviews.llvm.org/D33672



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


[PATCH] D53974: [clang-tidy] new checker: bugprone-too-small-loop-variable

2018-11-01 Thread Alexander Zaitsev via Phabricator via cfe-commits
ZaMaZaN4iK added a comment.

Hmm, i thought Clang has some warning for this, but I was wrong... Did you 
think to implement this check as Clang warning?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53974



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


r345828 - Logging: make os_log buffer size an integer constant expression.

2018-11-01 Thread Tim Northover via cfe-commits
Author: tnorthover
Date: Thu Nov  1 06:49:54 2018
New Revision: 345828

URL: http://llvm.org/viewvc/llvm-project?rev=345828&view=rev
Log:
Logging: make os_log buffer size an integer constant expression.

The size of an os_log buffer is known at any stage of compilation, so making it
a constant expression means that the common idiom of declaring a buffer for it
won't result in a VLA. That allows the compiler to skip saving and restoring
the stack pointer around such buffers.

Modified:
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/test/CodeGen/builtins.c

Modified: cfe/trunk/lib/AST/ExprConstant.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=345828&r1=345827&r2=345828&view=diff
==
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)
+++ cfe/trunk/lib/AST/ExprConstant.cpp Thu Nov  1 06:49:54 2018
@@ -33,6 +33,7 @@
 //
 
//===--===//
 
+#include "clang/Analysis/Analyses/OSLog.h"
 #include "clang/AST/APValue.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTDiagnostic.h"
@@ -8126,6 +8127,12 @@ bool IntExprEvaluator::VisitBuiltinCallE
 llvm_unreachable("unexpected EvalMode");
   }
 
+  case Builtin::BI__builtin_os_log_format_buffer_size: {
+analyze_os_log::OSLogBufferLayout Layout;
+analyze_os_log::computeOSLogBufferLayout(Info.Ctx, E, Layout);
+return Success(Layout.size().getQuantity(), E);
+  }
+
   case Builtin::BI__builtin_bswap16:
   case Builtin::BI__builtin_bswap32:
   case Builtin::BI__builtin_bswap64: {

Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=345828&r1=345827&r2=345828&view=diff
==
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Thu Nov  1 06:49:54 2018
@@ -3609,13 +3609,6 @@ RValue CodeGenFunction::EmitBuiltinExpr(
   case Builtin::BI__builtin_os_log_format:
 return emitBuiltinOSLogFormat(*E);
 
-  case Builtin::BI__builtin_os_log_format_buffer_size: {
-analyze_os_log::OSLogBufferLayout Layout;
-analyze_os_log::computeOSLogBufferLayout(CGM.getContext(), E, Layout);
-return RValue::get(ConstantInt::get(ConvertType(E->getType()),
-Layout.size().getQuantity()));
-  }
-
   case Builtin::BI__xray_customevent: {
 if (!ShouldXRayInstrumentFunction())
   return RValue::getIgnored();

Modified: cfe/trunk/test/CodeGen/builtins.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/builtins.c?rev=345828&r1=345827&r2=345828&view=diff
==
--- cfe/trunk/test/CodeGen/builtins.c (original)
+++ cfe/trunk/test/CodeGen/builtins.c Thu Nov  1 06:49:54 2018
@@ -729,25 +729,28 @@ void test_builtin_os_log_merge_helper1(v
 
 // CHECK-LABEL: define void @test_builtin_os_log_errno
 void test_builtin_os_log_errno() {
-  // CHECK: %[[VLA:.*]] = alloca i8, i64 4, align 16
-  // CHECK: call void @__os_log_helper_16_2_1_0_96(i8* %[[VLA]])
+  // CHECK-NOT: @stacksave
+  // CHECK: %[[BUF:.*]] = alloca [4 x i8], align 1
+  // CHECK: %[[DECAY:.*]] = getelementptr inbounds [4 x i8], [4 x i8]* 
%[[BUF]], i32 0, i32 0
+  // CHECK: call void @__os_log_helper_1_2_1_0_96(i8* %[[DECAY]])
+  // CHECK-NOT: @stackrestore
 
   char buf[__builtin_os_log_format_buffer_size("%m")];
   __builtin_os_log_format(buf, "%m");
 }
 
-// CHECK-LABEL: define linkonce_odr hidden void @__os_log_helper_16_2_1_0_96
+// CHECK-LABEL: define linkonce_odr hidden void @__os_log_helper_1_2_1_0_96
 // CHECK: (i8* %[[BUFFER:.*]])
 
 // CHECK: %[[BUFFER_ADDR:.*]] = alloca i8*, align 8
 // CHECK: store i8* %[[BUFFER]], i8** %[[BUFFER_ADDR]], align 8
 // CHECK: %[[BUF:.*]] = load i8*, i8** %[[BUFFER_ADDR]], align 8
 // CHECK: %[[SUMMARY:.*]] = getelementptr i8, i8* %[[BUF]], i64 0
-// CHECK: store i8 2, i8* %[[SUMMARY]], align 16
+// CHECK: store i8 2, i8* %[[SUMMARY]], align 1
 // CHECK: %[[NUMARGS:.*]] = getelementptr i8, i8* %[[BUF]], i64 1
 // CHECK: store i8 1, i8* %[[NUMARGS]], align 1
 // CHECK: %[[ARGDESCRIPTOR:.*]] = getelementptr i8, i8* %[[BUF]], i64 2
-// CHECK: store i8 96, i8* %[[ARGDESCRIPTOR]], align 2
+// CHECK: store i8 96, i8* %[[ARGDESCRIPTOR]], align 1
 // CHECK: %[[ARGSIZE:.*]] = getelementptr i8, i8* %[[BUF]], i64 3
 // CHECK: store i8 0, i8* %[[ARGSIZE]], align 1
 // CHECK-NEXT: ret void


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


[PATCH] D52949: [Diagnostics] Implement -Wsizeof-pointer-div

2018-11-01 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment.

- Addressed review comments.

We match the GCC behaviour here, except the case:
int b1 = sizeof(int *) / sizeof(int);

But I think it is ok now.




Comment at: lib/Sema/SemaExpr.cpp:8729
 
+static void DiagnoseDivisionSizeofPointer(Sema &S, Expr *LHS, Expr *RHS,
+  SourceLocation Loc) {

aaron.ballman wrote:
> `const Expr *`
if const, i have build issues with "<< LHS"


https://reviews.llvm.org/D52949



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


[PATCH] D52949: [Diagnostics] Implement -Wsizeof-pointer-div

2018-11-01 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 updated this revision to Diff 172117.
xbolva00 marked 5 inline comments as done.

https://reviews.llvm.org/D52949

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaExpr.cpp
  test/Sema/div-sizeof-ptr.cpp

Index: test/Sema/div-sizeof-ptr.cpp
===
--- test/Sema/div-sizeof-ptr.cpp
+++ test/Sema/div-sizeof-ptr.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 %s -verify -Wsizeof-pointer-div -fsyntax-only
+
+template 
+int f(Ty (&Array)[N]) {
+  return sizeof(Array) / sizeof(Ty); // Should not warn
+}
+
+void test(int *p, int **q) {
+int a1 = sizeof(p) / sizeof(*p);  // expected-warning {{'sizeof (p)' will return the size of the pointer, not the array itself}}
+int a2 = sizeof p / sizeof *p;// expected-warning {{'sizeof p' will return the size of the pointer, not the array itself}}
+int a3 = sizeof(*q) / sizeof(**q);// expected-warning {{'sizeof (*q)' will return the size of the pointer, not the array itself}}
+int a4 = sizeof(p) / sizeof(int); // expected-warning {{'sizeof (p)' will return the size of the pointer, not the array itself}}
+int a5 = sizeof(p) / sizeof(p[0]);// expected-warning {{'sizeof (p)' will return the size of the pointer, not the array itself}}
+
+// Should not warn
+int b1 = sizeof(int *) / sizeof(int);
+int b2 = sizeof(p) / sizeof(p);
+int b3 = sizeof(*q) / sizeof(q);
+int b4 = sizeof(p) / sizeof(char);
+
+int arr[10];
+int b5 = sizeof(arr) / sizeof(*arr);
+int b6 = sizeof(arr) / sizeof(arr[0]);
+int b7 = sizeof(arr) / sizeof(int);
+
+int arr2[10][12];
+int b8 = sizeof(arr2) / sizeof(*arr2); 
+}
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -8726,6 +8726,32 @@
   << LHS.get()->getSourceRange() << RHS.get()->getSourceRange();
 }
 
+static void DiagnoseDivisionSizeofPointer(Sema &S, Expr *LHS, Expr *RHS,
+  SourceLocation Loc) {
+  const auto *LUE = dyn_cast(LHS);
+  const auto *RUE = dyn_cast(RHS);
+  if (!LUE || !RUE)
+return;
+  if (LUE->getKind() != UETT_SizeOf || LUE->isArgumentType() ||
+  RUE->getKind() != UETT_SizeOf)
+return;
+
+  QualType LHSTy = LUE->getArgumentExpr()->IgnoreParens()->getType();
+  QualType RHSTy;
+
+  if (RUE->isArgumentType())
+RHSTy = RUE->getArgumentType();
+  else
+RHSTy = RUE->getArgumentExpr()->IgnoreParens()->getType();
+
+  if (!LHSTy->isPointerType() || RHSTy->isPointerType())
+return;
+  if (LHSTy->getPointeeType() != RHSTy)
+return;
+
+  S.Diag(Loc, diag::warn_division_sizeof_ptr) << LHS << LHS->getSourceRange();
+}
+
 static void DiagnoseBadDivideOrRemainderValues(Sema& S, ExprResult &LHS,
ExprResult &RHS,
SourceLocation Loc, bool IsDiv) {
@@ -8756,8 +8782,10 @@
 
   if (compType.isNull() || !compType->isArithmeticType())
 return InvalidOperands(Loc, LHS, RHS);
-  if (IsDiv)
+  if (IsDiv) {
 DiagnoseBadDivideOrRemainderValues(*this, LHS, RHS, Loc, IsDiv);
+DiagnoseDivisionSizeofPointer(*this, LHS.get(), RHS.get(), Loc);
+  }
   return compType;
 }
 
@@ -16599,4 +16627,4 @@
 
   return new (Context)
   ObjCAvailabilityCheckExpr(Version, AtLoc, RParen, Context.BoolTy);
-}
+}
\ No newline at end of file
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -3294,6 +3294,10 @@
   InGroup;
 def note_reference_is_return_value : Note<"%0 returns a reference">;
 
+def warn_division_sizeof_ptr : Warning<
+  "'%0' will return the size of the pointer, not the array itself">,
+  InGroup>;
+
 def note_function_warning_silence : Note<
 "prefix with the address-of operator to silence this warning">;
 def note_function_to_function_call : Note<
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D53949: [clang][CodeGen] Implicit Conversion Sanitizer: discover the world of CompoundAssign operators

2018-11-01 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 172115.
lebedev.ri edited the summary of this revision.
lebedev.ri added a subscriber: mclow.lists.
lebedev.ri added a comment.

Added test coverage.
Please review :)


Repository:
  rC Clang

https://reviews.llvm.org/D53949

Files:
  lib/CodeGen/CGExprScalar.cpp
  test/CodeGen/catch-implicit-integer-sign-changes-CompoundAssignOperator.c
  test/CodeGen/catch-implicit-integer-truncations-CompoundAssignOperator.c

Index: test/CodeGen/catch-implicit-integer-truncations-CompoundAssignOperator.c
===
--- /dev/null
+++ test/CodeGen/catch-implicit-integer-truncations-CompoundAssignOperator.c
@@ -0,0 +1,737 @@
+// RUN: %clang_cc1 -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s --check-prefix=CHECK
+
+// RUN: %clang_cc1 -fsanitize=implicit-unsigned-integer-truncation,implicit-signed-integer-truncation -fno-sanitize-recover=implicit-unsigned-integer-truncation,implicit-signed-integer-truncation -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s -implicit-check-not="call void @__ubsan_handle_implicit_conversion" --check-prefixes=CHECK,CHECK-SANITIZE,CHECK-SANITIZE-ANYRECOVER,CHECK-SANITIZE-NORECOVER,CHECK-SANITIZE-UNREACHABLE
+// RUN: %clang_cc1 -fsanitize=implicit-unsigned-integer-truncation,implicit-signed-integer-truncation -fsanitize-recover=implicit-unsigned-integer-truncation,implicit-signed-integer-truncation -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s -implicit-check-not="call void @__ubsan_handle_implicit_conversion" --check-prefixes=CHECK,CHECK-SANITIZE,CHECK-SANITIZE-ANYRECOVER,CHECK-SANITIZE-RECOVER
+// RUN: %clang_cc1 -fsanitize=implicit-unsigned-integer-truncation,implicit-signed-integer-truncation -fsanitize-trap=implicit-unsigned-integer-truncation,implicit-signed-integer-truncation -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s -implicit-check-not="call void @__ubsan_handle_implicit_conversion" --check-prefixes=CHECK,CHECK-SANITIZE,CHECK-SANITIZE-TRAP,CHECK-SANITIZE-UNREACHABLE
+
+// RUN: %clang_cc1 -fsanitize=implicit-signed-integer-truncation -fno-sanitize-recover=implicit-signed-integer-truncation -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s -implicit-check-not="call void @__ubsan_handle_implicit_conversion" --check-prefixes=CHECK,CHECK-SANITIZE,CHECK-SANITIZE-ANYRECOVER,CHECK-SANITIZE-NORECOVER,CHECK-SANITIZE-UNREACHABLE
+// RUN: %clang_cc1 -fsanitize=implicit-signed-integer-truncation -fsanitize-recover=implicit-signed-integer-truncation -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s -implicit-check-not="call void @__ubsan_handle_implicit_conversion" --check-prefixes=CHECK,CHECK-SANITIZE,CHECK-SANITIZE-ANYRECOVER,CHECK-SANITIZE-RECOVER
+// RUN: %clang_cc1 -fsanitize=implicit-signed-integer-truncation -fsanitize-trap=implicit-signed-integer-truncation -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s -implicit-check-not="call void @__ubsan_handle_implicit_conversion" --check-prefixes=CHECK,CHECK-SANITIZE,CHECK-SANITIZE-TRAP,CHECK-SANITIZE-UNREACHABLE
+
+// RUN: %clang_cc1 -fsanitize=implicit-signed-integer-truncation,implicit-integer-sign-change -fno-sanitize-recover=implicit-signed-integer-truncation,implicit-integer-sign-change -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s -implicit-check-not="call void @__ubsan_handle_implicit_conversion" --check-prefixes=CHECK,CHECK-SANITIZE,CHECK-SANITIZE-ANYRECOVER,CHECK-SANITIZE-NORECOVER,CHECK-SANITIZE-UNREACHABLE
+// RUN: %clang_cc1 -fsanitize=implicit-signed-integer-truncation,implicit-integer-sign-change -fsanitize-recover=implicit-signed-integer-truncation,implicit-integer-sign-change -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s -implicit-check-not="call void @__ubsan_handle_implicit_conversion" --check-prefixes=CHECK,CHECK-SANITIZE,CHECK-SANITIZE-ANYRECOVER,CHECK-SANITIZE-RECOVER
+// RUN: %clang_cc1 -fsanitize=implicit-signed-integer-truncation,implicit-integer-sign-change -fsanitize-trap=implicit-signed-integer-truncation,implicit-integer-sign-change -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s -implicit-check-not="call void @__ubsan_handle_implicit_conversion" --check-prefixes=CHECK,CHECK-SANITIZE,CHECK-SANITIZE-TRAP,CHECK-SANITIZE-UNREACHABLE
+
+// CHECK-SANITIZE-ANYRECOVER: @[[INT:.*]] = {{.*}} c"'int'\00" }
+// CHECK-SANITIZE-ANYRECOVER: @[[UNSIGNED_CHAR:.*]] = {{.*}} c"'unsigned char'\00" }
+// CHECK-SANITIZE-ANYRECOVER: @[[LINE_100_SIGNED_TRUNCATION:.*]] = {{.*}}, i32 100, i32 10 }, {{.*}}* @[[INT]], {{.*}}* @[[UNSIGNED_CHAR]], i8 2 }
+// CHECK-SANITIZE-ANYRECOVER: @[[SIGNED_CHAR:.*]] = {{.*}} c"'signed char'\00" }
+// CHECK-SANITIZE-ANYRECOVER: @[[LINE_200_SIGNED_TRUNCATION:.*]] = {{.*}}, i32 200, i32 10 }, {{.*}}* @[[INT]], {{.*}}* @[[SIGNED_CHAR]], i8 2 }
+// CHECK-SANITIZE-ANYRECOVER: @[[LINE_300_SIGNED_TRUNCATION:.*]] = {{.*}}, i32 300, i32 10 }, {{.*}}* @[[INT]], {{.*}}* @[[UNSIGNED_CHAR]], i8 2 }
+// CHECK-SANITIZE

[PATCH] D53974: [clang-tidy] new checker: bugprone-too-small-loop-variable

2018-11-01 Thread Tamás Zolnai via Phabricator via cfe-commits
ztamas added a comment.

Just a note. I run the new checker on LibreOffice project. I found ~25 false 
positives, which seems small enough to me. This false positives can be 
supressed easily.
Since the LibreOffice project has a similar check as a compiler plugin the code 
was already cleaned up, however I still found ~30 uncaught issue with the new 
checker:
https://cgit.freedesktop.org/libreoffice/core/commit/?id=26ccd00bc96c585b7065af0dcce246b5bfaae5e1


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53974



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


[PATCH] D53974: [clang-tidy] new checker: bugprone-too-small-loop-variable

2018-11-01 Thread Tamás Zolnai via Phabricator via cfe-commits
ztamas updated this revision to Diff 172113.
ztamas added a comment.

Add a description of the checker with examples to the documentation


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53974

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
  clang-tidy/bugprone/TooSmallLoopVariableCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-too-small-loop-variable.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-too-small-loop-variable.cpp

Index: test/clang-tidy/bugprone-too-small-loop-variable.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-too-small-loop-variable.cpp
@@ -0,0 +1,160 @@
+// RUN: %check_clang_tidy %s bugprone-too-small-loop-variable %t
+
+long size() { return 294967296l; }
+
+
+void voidBadForLoop() {
+  for (int i = 0; i < size(); ++i) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: loop variable has a narrower type ('int') than the type ('long') of termination condition [bugprone-too-small-loop-variable]
+}
+
+
+void voidBadForLoop2() {
+  for (int i = 0; i < size() + 10; ++i) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: loop variable has a narrower type ('int') than the type ('long') of termination condition [bugprone-too-small-loop-variable]
+}
+
+
+void voidBadForLoop3() {
+  for (int i = 0; i <= size() - 1; ++i) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: loop variable has a narrower type ('int') than the type ('long') of termination condition [bugprone-too-small-loop-variable]
+}
+
+
+void voidBadForLoop4() {
+  for (int i = 0; size() > i; ++i) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: loop variable has a narrower type ('int') than the type ('long') of termination condition [bugprone-too-small-loop-variable]
+}
+
+
+void voidBadForLoop5() {
+  for (int i = 0; size() - 1 >= i; ++i) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: loop variable has a narrower type ('int') than the type ('long') of termination condition [bugprone-too-small-loop-variable]
+}
+
+
+void voidBadForLoop6() {
+  int i = 0;
+  for (; i < size(); ++i) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: loop variable has a narrower type ('int') than the type ('long') of termination condition [bugprone-too-small-loop-variable]
+}
+
+
+void voidForLoopUnsignedCond() {
+  unsigned size = 3147483647;
+  for (int i = 0; i < size; ++i) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: loop variable has a narrower type ('int') than the type ('unsigned int') of termination condition [bugprone-too-small-loop-variable]
+}
+
+
+// False positive: because of the integer literal, loop condition has int type
+void voidForLoopFalsePositive() {
+  short size = 3;
+  bool cond = false;
+  for (short i = 0; i < (cond ? 0 : size); ++i) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: loop variable has a narrower type ('short') than the type ('int') of termination condition [bugprone-too-small-loop-variable]
+}
+
+
+// Simple use case when both expressions have the same type
+void voidGoodForLoop() {
+  for (long i = 0; i < size(); ++i) {}  // no warning
+}
+
+
+// Second simple use case when both expressions have the same type
+void voidGoodForLoop2() {
+  short loopCond = 10;
+  for (short i = 0; i < loopCond; ++i) {}  // no warning
+}
+
+
+// Because of the integer literal, the loop condition is int, but we suppress the warning here
+void voidForLoopShortPlusLiteral() {
+  short size = 3;
+  for (short i = 0; i <= (size - 1); ++i) {} // no warning
+}
+
+
+// Additions of two short variable is converted to int, but we suppress this to avoid false positives
+void voidForLoopShortPlusShort() {
+  short size = 256;
+  short increment = 14;
+  for (short i = 0; i < size + increment; ++i) {} // no warning
+}
+
+// Different integer types, but in this case the loop variable is the bigger type
+void voidForLoopReverseCond() {
+  short start = 256;
+  short end = 14;
+  for (int i = start; i >= end; --i) {} // no warning
+}
+
+
+// TODO: handle while loop
+void voidBadWhileLoop() {
+  short i = 0;
+  while (i < size()) {++i;} // missing warning
+}
+
+
+// TODO: handle do-while loop
+void voidBadDoWhileLoop() {
+  short i = 0;
+  do {++i;} while (i < size()); // missing warning
+}
+
+
+// We do not catch this, but other conversion related checks can catch it anyway
+void voidReverseForLoop() {
+  for (short i = size() - 1; i >= 0; --i) {}  // no warning
+}
+
+
+// TODO: handle those cases when the loop condition contains a floating point expression
+void voidDoubleForCond() {
+  double condVar = size();
+  for (short i = 0; i < condVar; ++i) {} // missing warning
+}
+
+
+// TODO: handle complex loop conditions
+void voidComplexForCond() {
+  bool additionalCond = true;
+  for (int i = 0; i < size() && additionalCond; ++i) {} // missing warning
+}
+
+
+// Macro in loop condition
+#define SIZE 125
+#define 

[PATCH] D53433: [clangd] auto-index stores symbols per-file instead of per-TU.

2018-11-01 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments.



Comment at: clangd/index/Background.cpp:194
-  // FIXME: partition the symbols by file rather than TU, to avoid duplication.
-  IndexedSymbols.update(AbsolutePath,
-llvm::make_unique(std::move(Symbols)),

kadircet wrote:
> sammccall wrote:
> > um, this (before your patch) doesn't look even slightly threadsafe. Sorry I 
> > didn't catch it in code review.
> > 
> > Now I'm not sure whether it makes sense for you to fix it or not. It seems 
> > fine to use two independent locks here, to me - we don't need actual 
> > consistency.
> Ah, sorry for missing on my side as well. But IIUC, the only unsafe part is 
> access to `FileHash` since `FileSymbols::update` is already thread-safe ?
> 
> If need be I can send out a patch to fix those issues.
Oops, you're right - I forgot `FileSymbols` was threadsafe.

Then I think FileHash itself is indeed the only issue and this patch 
(DigestsMu) fixes it already. Carry on!


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53433



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


[PATCH] D53417: [Clang][Sema][PowerPC] Choose a better candidate in overload function call if there is a compatible vector conversion instead of ambiguous call error

2018-11-01 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Sema/SemaOverload.cpp:3913
+for (auto Type : Types) {
+  if (S.Context.getCanonicalType(Type)->getTypeClass() != Type::Vector)
+return false;

wuzish wrote:
> hubert.reinterpretcast wrote:
> > hubert.reinterpretcast wrote:
> > > wuzish wrote:
> > > > hubert.reinterpretcast wrote:
> > > > > wuzish wrote:
> > > > > > hubert.reinterpretcast wrote:
> > > > > > > wuzish wrote:
> > > > > > > > hubert.reinterpretcast wrote:
> > > > > > > > > Considering that this is a local lambda called in one place, 
> > > > > > > > > are we expecting cases where the canonical type is not 
> > > > > > > > > something on which we can call getVectorKind()? If not, then 
> > > > > > > > > we do not need this `if`.
> > > > > > > > Well, that's for ExtVectorType. I encounter some testcase 
> > > > > > > > failure because of that. So I just narrow the condition to only 
> > > > > > > > handle Type::Vector.
> > > > > > > > 
> > > > > > > > As the following comment said, so I treat it separately.
> > > > > > > > 
> > > > > > > > /// ExtVectorType - Extended vector type. This type is created 
> > > > > > > > using
> > > > > > > > /// __attribute__((ext_vector_type(n)), where "n" is the number 
> > > > > > > > of elements.
> > > > > > > > /// Unlike vector_size, ext_vector_type is only allowed on 
> > > > > > > > typedef's. This
> > > > > > > > /// class enables syntactic extensions, like Vector Components 
> > > > > > > > for accessing
> > > > > > > > /// points (as .xyzw), colors (as .rgba), and textures (modeled 
> > > > > > > > after OpenGL
> > > > > > > > /// Shading Language).
> > > > > > > An ExtVectorType is a VectorType, so what sort of failures are 
> > > > > > > you hitting?
> > > > > > Yes. But the TypeClass of ExtVectorType is ExtVector.
> > > > > > 
> > > > > > some test points check the error report for ambiguous call because 
> > > > > > of too many implicit cast choices from ext_vector_type to vector 
> > > > > > type. Such as blow.
> > > > > > 
> > > > > > 
> > > > > > ```
> > > > > > typedef char char16 __attribute__ ((__vector_size__ (16)));
> > > > > > typedef long long longlong16 __attribute__ ((__vector_size__ (16)));
> > > > > > typedef char char16_e __attribute__ ((__ext_vector_type__ (16)));
> > > > > > typedef long long longlong16_e __attribute__ ((__ext_vector_type__ 
> > > > > > (2)));
> > > > > > 
> > > > > > 
> > > > > > void f1_test(char16 c16, longlong16 ll16, char16_e c16e, 
> > > > > > longlong16_e ll16e) {
> > > > > >   int &ir1 = f1(c16);
> > > > > >   float &fr1 = f1(ll16);
> > > > > >   f1(c16e); // expected-error{{call to 'f1' is ambiguous}}
> > > > > >   f1(ll16e); // expected-error{{call to 'f1' is ambiguous}}
> > > > > > }
> > > > > > ```
> > > > > > 
> > > > > > If we are gonna widen the condition, we can make a follow-up patch. 
> > > > > > Or we need include this condition and do this together in this 
> > > > > > patch?
> > > > > The widening that has occurred is in allowing the scope of the change 
> > > > > to encompass cases where AltiVec vectors are not sufficiently 
> > > > > involved. The change in behaviour makes sense, and perhaps the 
> > > > > community may want to pursue it; however, the mandate to make that 
> > > > > level of change has not been given.
> > > > > 
> > > > > I do not believe that requiring that the TypeClass be VectorType is 
> > > > > the correct narrowing of the scope. Instead, we may want to consider 
> > > > > requiring that for each `SCS` in { `SCS1`, `SCS2` }, there is one 
> > > > > AltiVec type and one generic vector type in { `SCS.getFromType()`, 
> > > > > `SCS.getToType(2)` }.
> > > > > 
> > > > The key point is that ExtVector is a kind of typeclass, **and** its 
> > > > vector kind is generic vector type.
> > > > 
> > > > So you think we should encompass ext_vector cases as a part of the 
> > > > scope of this patch? And modify the above cases' expected behavior 
> > > > (remove the **expected-error**)?
> > > I gave a concrete suggested narrowing of the scope that does not exclude 
> > > ExtVectorType or other derived types of VectorType. It also does not 
> > > change the behaviour of the `f1_test` case above. We could pursue 
> > > additional discussion over that case (separable from the current patch) 
> > > on the mailing list.
> > > 
> > > I believe my suggestion does do something about this case:
> > > ```
> > > typedef unsigned int GccType __attribute__((__ext_vector_type__(16)));
> > > typedef __vector unsigned int AltiVecType;
> > > 
> > > typedef float GccOtherType __attribute__((__vector_size__(16)));
> > > 
> > > void f(GccType);
> > > void f(GccOtherType);
> > > 
> > > void g(AltiVecType v) { f(v); }
> > > ```
> > > 
> > > I think that addressing the latter case is within the realm of things 
> > > that we should consider for this patch. Either way, we should ensure that 
> > > tests for AltiVec/__ext_vector_ty

[PATCH] D53974: [clang-tidy] new checker: bugprone-too-small-loop-variable

2018-11-01 Thread Tamás Zolnai via Phabricator via cfe-commits
ztamas created this revision.
Herald added subscribers: cfe-commits, xazax.hun, mgorny.

The new checker searches for those for loops which has a loop variable
with a "too small" type which means this type can't represent all values
which are part of the iteration range.
For example:

int main() {

  long size = 30;
  for( short int i = 0; i < size; ++i) {}

}

The short type leads to infinite loop here because it can't store all
values in the [0..size] interval. In a real use case, size means a
container's size which depends on the user input. Which means for small
amount of objects the algorithm works, but with a larger user input the
software will freeze.

The idea of the checker comes from the LibreOffice project, where the
same check was implemented as a clang compiler plugin, called
LoopVarTooSmall (LLVM licensed).
The idea is the same behind this check, but the code is different because
of the different framework.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53974

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
  clang-tidy/bugprone/TooSmallLoopVariableCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-too-small-loop-variable.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-too-small-loop-variable.cpp

Index: test/clang-tidy/bugprone-too-small-loop-variable.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-too-small-loop-variable.cpp
@@ -0,0 +1,160 @@
+// RUN: %check_clang_tidy %s bugprone-too-small-loop-variable %t
+
+long size() { return 294967296l; }
+
+
+void voidBadForLoop() {
+  for (int i = 0; i < size(); ++i) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: loop variable has a narrower type ('int') than the type ('long') of termination condition [bugprone-too-small-loop-variable]
+}
+
+
+void voidBadForLoop2() {
+  for (int i = 0; i < size() + 10; ++i) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: loop variable has a narrower type ('int') than the type ('long') of termination condition [bugprone-too-small-loop-variable]
+}
+
+
+void voidBadForLoop3() {
+  for (int i = 0; i <= size() - 1; ++i) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: loop variable has a narrower type ('int') than the type ('long') of termination condition [bugprone-too-small-loop-variable]
+}
+
+
+void voidBadForLoop4() {
+  for (int i = 0; size() > i; ++i) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: loop variable has a narrower type ('int') than the type ('long') of termination condition [bugprone-too-small-loop-variable]
+}
+
+
+void voidBadForLoop5() {
+  for (int i = 0; size() - 1 >= i; ++i) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: loop variable has a narrower type ('int') than the type ('long') of termination condition [bugprone-too-small-loop-variable]
+}
+
+
+void voidBadForLoop6() {
+  int i = 0;
+  for (; i < size(); ++i) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: loop variable has a narrower type ('int') than the type ('long') of termination condition [bugprone-too-small-loop-variable]
+}
+
+
+void voidForLoopUnsignedCond() {
+  unsigned size = 3147483647;
+  for (int i = 0; i < size; ++i) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: loop variable has a narrower type ('int') than the type ('unsigned int') of termination condition [bugprone-too-small-loop-variable]
+}
+
+
+// False positive: because of the integer literal, loop condition has int type
+void voidForLoopFalsePositive() {
+  short size = 3;
+  bool cond = false;
+  for (short i = 0; i < (cond ? 0 : size); ++i) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: loop variable has a narrower type ('short') than the type ('int') of termination condition [bugprone-too-small-loop-variable]
+}
+
+
+// Simple use case when both expressions have the same type
+void voidGoodForLoop() {
+  for (long i = 0; i < size(); ++i) {}  // no warning
+}
+
+
+// Second simple use case when both expressions have the same type
+void voidGoodForLoop2() {
+  short loopCond = 10;
+  for (short i = 0; i < loopCond; ++i) {}  // no warning
+}
+
+
+// Because of the integer literal, the loop condition is int, but we suppress the warning here
+void voidForLoopShortPlusLiteral() {
+  short size = 3;
+  for (short i = 0; i <= (size - 1); ++i) {} // no warning
+}
+
+
+// Additions of two short variable is converted to int, but we suppress this to avoid false positives
+void voidForLoopShortPlusShort() {
+  short size = 256;
+  short increment = 14;
+  for (short i = 0; i < size + increment; ++i) {} // no warning
+}
+
+// Different integer types, but in this case the loop variable is the bigger type
+void voidForLoopReverseCond() {
+  short start = 256;
+  short end = 14;
+  for (int i = start; i >= end; --i) {} // no warning
+}
+
+
+// TODO: handle while loop
+void voidBadWhileLoop() {
+  short i = 0;
+  while (i < size())

[PATCH] D52949: [Diagnostics] Implement -Wsizeof-pointer-div

2018-11-01 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: include/clang/Basic/DiagnosticGroups.td:147
 def : DiagGroup<"div-by-zero", [DivZero]>;
+def DivSizeofPtr : DiagGroup<"sizeof-pointer-div">;
 

Do you really need the explicit diagnostic group? Given that there's only one 
diagnostic here, I would lower this into the diag (see below).



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:3299
+  "'%0' will return the size of the pointer, not the array itself">,
+  InGroup, DefaultIgnore;
+

`InGroup>`



Comment at: lib/Sema/SemaExpr.cpp:8729
 
+static void DiagnoseDivisionSizeofPointer(Sema &S, Expr *LHS, Expr *RHS,
+  SourceLocation Loc) {

`const Expr *`



Comment at: lib/Sema/SemaExpr.cpp:8731-8733
+  UnaryExprOrTypeTraitExpr *LUE =
+  dyn_cast_or_null(LHS);
+  UnaryExprOrTypeTraitExpr *RUE =

Can use `const auto *` here as the type is spelled out in the initialization.

Under what circumstances would you expect to get a null `LHS` or `RHS`? I 
suspect this should be using `dyn_cast` instead.



Comment at: lib/Sema/SemaExpr.cpp:8746-8750
+  if (RUE->isArgumentType()) {
+RHSTy = RUE->getArgumentType();
+  } else {
+RHSTy = RUE->getArgumentExpr()->IgnoreParens()->getType();
+  }

Elide braces.



Comment at: test/Sema/div-sizeof-ptr.c:9
+
+int e = sizeof(int *) / sizeof(int);
+int f = sizeof(p) / sizeof(p);

xbolva00 wrote:
> GCC warns also in this case, but it is weird...
I think it's reasonable to not diagnose here, but I don't have strong feelings.



Comment at: test/Sema/div-sizeof-ptr.c:1
+// RUN: %clang_cc1 %s -verify -Wsizeof-pointer-div -fsyntax-only
+

This is missing tests for the positive behavior -- can you add some tests 
involving arrays rather than pointers? Here are some other fun test cases as 
well:
```
int a[10][12];
sizeof(a) / sizeof(*a); // Should this warn?
```
```
template 
int f(Ty (&Array)[N]) {
  return sizeof(Array) / sizeof(Ty); // Shouldn't warn
}
```


https://reviews.llvm.org/D52949



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


r345826 - CPU-Dispatch-- Fix conflict between 'generic' and 'pentium'

2018-11-01 Thread Erich Keane via cfe-commits
Author: erichkeane
Date: Thu Nov  1 05:50:37 2018
New Revision: 345826

URL: http://llvm.org/viewvc/llvm-project?rev=345826&view=rev
Log:
CPU-Dispatch-- Fix conflict between 'generic' and 'pentium'

When a dispatch function was being emitted that had both a generic and a
pentium configuration listed, we would assert.  This is because neither
configuration has any 'features' associated with it so they were both
considered the 'default' version.  'pentium' lacks any features because
we implement it in terms of __builtin_cpu_supports (instead of Intel
proprietary checks), which is unable to decern between the two.

The fix for this is to omit the 'generic' version from the dispatcher if
both are present. This permits existing code to compile, and still will
choose the 'best' version available (since 'pentium' is technically
better than 'generic').

Change-Id: I4b69f3e0344e74cbdbb04497845d5895dd05fda0

Modified:
cfe/trunk/lib/CodeGen/CodeGenModule.cpp
cfe/trunk/test/CodeGen/attr-cpuspecific.c

Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=345826&r1=345825&r2=345826&view=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Thu Nov  1 05:50:37 2018
@@ -2583,6 +2583,22 @@ void CodeGenModule::emitCPUDispatchDefin
 return CodeGenFunction::GetX86CpuSupportsMask(LHS.Conditions.Features) 
>
CodeGenFunction::GetX86CpuSupportsMask(RHS.Conditions.Features);
   });
+
+  // If the list contains multiple 'default' versions, such as when it contains
+  // 'pentium' and 'generic', don't emit the call to the generic one (since we
+  // always run on at least a 'pentium'). We do this by deleting the 'least
+  // advanced' (read, lowest mangling letter).
+  while (Options.size() > 1 &&
+ CodeGenFunction::GetX86CpuSupportsMask(
+ (Options.end() - 2)->Conditions.Features) == 0) {
+StringRef LHSName = (Options.end() - 2)->Function->getName();
+StringRef RHSName = (Options.end() - 1)->Function->getName();
+if (LHSName.compare(RHSName) < 0)
+  Options.erase(Options.end() - 2);
+else
+  Options.erase(Options.end() - 1);
+  }
+
   CodeGenFunction CGF(*this);
   CGF.EmitMultiVersionResolver(ResolverFunc, Options);
 }

Modified: cfe/trunk/test/CodeGen/attr-cpuspecific.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/attr-cpuspecific.c?rev=345826&r1=345825&r2=345826&view=diff
==
--- cfe/trunk/test/CodeGen/attr-cpuspecific.c (original)
+++ cfe/trunk/test/CodeGen/attr-cpuspecific.c Thu Nov  1 05:50:37 2018
@@ -205,6 +205,24 @@ int HasParamsAndReturn(int i, double d);
 // WINDOWS-NEXT: ret i32 %[[RET]]
 // WINDOWS-NOT: call void @llvm.trap
 
+ATTR(cpu_dispatch(atom, generic, pentium))
+int GenericAndPentium(int i, double d);
+// LINUX: define i32 (i32, double)* @GenericAndPentium.resolver()
+// LINUX: call void @__cpu_indicator_init
+// LINUX: ret i32 (i32, double)* @GenericAndPentium.O
+// LINUX: ret i32 (i32, double)* @GenericAndPentium.B
+// LINUX-NOT: ret i32 (i32, double)* @GenericAndPentium.A
+// LINUX-NOT: call void @llvm.trap
+
+// WINDOWS: define dso_local i32 @GenericAndPentium(i32, double)
+// WINDOWS: call void @__cpu_indicator_init
+// WINDOWS: %[[RET:.+]] = musttail call i32 @GenericAndPentium.O(i32 %0, 
double %1)
+// WINDOWS-NEXT: ret i32 %[[RET]]
+// WINDOWS: %[[RET:.+]] = musttail call i32 @GenericAndPentium.B(i32 %0, 
double %1)
+// WINDOWS-NEXT: ret i32 %[[RET]]
+// WINDOWS-NOT: call i32 @GenericAndPentium.A
+// WINDOWS-NOT: call void @llvm.trap
+
 // CHECK: attributes #[[S]] = 
{{.*}}"target-features"="+avx,+cmov,+f16c,+mmx,+popcnt,+sse,+sse2,+sse3,+sse4.1,+sse4.2,+ssse3,+x87,+xsave"
 // CHECK: attributes #[[K]] = 
{{.*}}"target-features"="+adx,+avx,+avx2,+avx512cd,+avx512er,+avx512f,+avx512pf,+bmi,+cmov,+f16c,+fma,+lzcnt,+mmx,+movbe,+popcnt,+sse,+sse2,+sse3,+sse4.1,+sse4.2,+ssse3,+x87,+xsave"
 // CHECK: attributes #[[O]] = 
{{.*}}"target-features"="+cmov,+mmx,+movbe,+sse,+sse2,+sse3,+ssse3,+x87"


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


[PATCH] D53871: [OpenCL] Allow clk_event_t comparisons

2018-11-01 Thread Sven van Haastregt via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC345825: Allow clk_event_t comparisons (authored by svenvh, 
committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D53871?vs=171711&id=172110#toc

Repository:
  rC Clang

https://reviews.llvm.org/D53871

Files:
  lib/Sema/SemaExpr.cpp
  test/SemaOpenCL/clk_event_t.cl
  test/SemaOpenCL/invalid-clk-events-cl2.0.cl


Index: test/SemaOpenCL/clk_event_t.cl
===
--- test/SemaOpenCL/clk_event_t.cl
+++ test/SemaOpenCL/clk_event_t.cl
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL2.0
+
+// Taken from opencl-c.h
+#define CLK_NULL_EVENT (__builtin_astype(((void*)(__SIZE_MAX__)), clk_event_t))
+
+global clk_event_t ce; // expected-error {{the '__global clk_event_t' type 
cannot be used to declare a program scope variable}}
+
+int clk_event_tests() {
+  event_t e;
+  clk_event_t ce1;
+  clk_event_t ce2;
+
+  if (e == ce1) { // expected-error {{invalid operands to binary expression 
('event_t' and 'clk_event_t')}}
+return 9;
+  }
+
+  if (ce1 != ce2) {
+return 1;
+  }
+  else if (ce1 == CLK_NULL_EVENT || ce2 != CLK_NULL_EVENT) {
+return 0;
+  }
+
+  return 2;
+}
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -10497,6 +10497,10 @@
   }
 
   if (getLangOpts().OpenCLVersion >= 200) {
+if (LHSType->isClkEventT() && RHSType->isClkEventT()) {
+  return computeResultTy();
+}
+
 if (LHSType->isQueueT() && RHSType->isQueueT()) {
   return computeResultTy();
 }
Index: test/SemaOpenCL/invalid-clk-events-cl2.0.cl
===
--- test/SemaOpenCL/invalid-clk-events-cl2.0.cl
+++ test/SemaOpenCL/invalid-clk-events-cl2.0.cl
@@ -1,3 +0,0 @@
-// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL2.0
-
-global clk_event_t ce; // expected-error {{the '__global clk_event_t' type 
cannot be used to declare a program scope variable}}


Index: test/SemaOpenCL/clk_event_t.cl
===
--- test/SemaOpenCL/clk_event_t.cl
+++ test/SemaOpenCL/clk_event_t.cl
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL2.0
+
+// Taken from opencl-c.h
+#define CLK_NULL_EVENT (__builtin_astype(((void*)(__SIZE_MAX__)), clk_event_t))
+
+global clk_event_t ce; // expected-error {{the '__global clk_event_t' type cannot be used to declare a program scope variable}}
+
+int clk_event_tests() {
+  event_t e;
+  clk_event_t ce1;
+  clk_event_t ce2;
+
+  if (e == ce1) { // expected-error {{invalid operands to binary expression ('event_t' and 'clk_event_t')}}
+return 9;
+  }
+
+  if (ce1 != ce2) {
+return 1;
+  }
+  else if (ce1 == CLK_NULL_EVENT || ce2 != CLK_NULL_EVENT) {
+return 0;
+  }
+
+  return 2;
+}
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -10497,6 +10497,10 @@
   }
 
   if (getLangOpts().OpenCLVersion >= 200) {
+if (LHSType->isClkEventT() && RHSType->isClkEventT()) {
+  return computeResultTy();
+}
+
 if (LHSType->isQueueT() && RHSType->isQueueT()) {
   return computeResultTy();
 }
Index: test/SemaOpenCL/invalid-clk-events-cl2.0.cl
===
--- test/SemaOpenCL/invalid-clk-events-cl2.0.cl
+++ test/SemaOpenCL/invalid-clk-events-cl2.0.cl
@@ -1,3 +0,0 @@
-// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL2.0
-
-global clk_event_t ce; // expected-error {{the '__global clk_event_t' type cannot be used to declare a program scope variable}}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r345825 - Allow clk_event_t comparisons

2018-11-01 Thread Sven van Haastregt via cfe-commits
Author: svenvh
Date: Thu Nov  1 05:43:00 2018
New Revision: 345825

URL: http://llvm.org/viewvc/llvm-project?rev=345825&view=rev
Log:
Allow clk_event_t comparisons

Also rename `invalid-clk-events-cl2.0.cl` to `clk_event_t.cl` and
repurpose it to include both positive and negative clk_event_t tests.

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

Added:
cfe/trunk/test/SemaOpenCL/clk_event_t.cl
Removed:
cfe/trunk/test/SemaOpenCL/invalid-clk-events-cl2.0.cl
Modified:
cfe/trunk/lib/Sema/SemaExpr.cpp

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=345825&r1=345824&r2=345825&view=diff
==
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Thu Nov  1 05:43:00 2018
@@ -10497,6 +10497,10 @@ QualType Sema::CheckCompareOperands(Expr
   }
 
   if (getLangOpts().OpenCLVersion >= 200) {
+if (LHSType->isClkEventT() && RHSType->isClkEventT()) {
+  return computeResultTy();
+}
+
 if (LHSType->isQueueT() && RHSType->isQueueT()) {
   return computeResultTy();
 }

Added: cfe/trunk/test/SemaOpenCL/clk_event_t.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaOpenCL/clk_event_t.cl?rev=345825&view=auto
==
--- cfe/trunk/test/SemaOpenCL/clk_event_t.cl (added)
+++ cfe/trunk/test/SemaOpenCL/clk_event_t.cl Thu Nov  1 05:43:00 2018
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL2.0
+
+// Taken from opencl-c.h
+#define CLK_NULL_EVENT (__builtin_astype(((void*)(__SIZE_MAX__)), clk_event_t))
+
+global clk_event_t ce; // expected-error {{the '__global clk_event_t' type 
cannot be used to declare a program scope variable}}
+
+int clk_event_tests() {
+  event_t e;
+  clk_event_t ce1;
+  clk_event_t ce2;
+
+  if (e == ce1) { // expected-error {{invalid operands to binary expression 
('event_t' and 'clk_event_t')}}
+return 9;
+  }
+
+  if (ce1 != ce2) {
+return 1;
+  }
+  else if (ce1 == CLK_NULL_EVENT || ce2 != CLK_NULL_EVENT) {
+return 0;
+  }
+
+  return 2;
+}

Removed: cfe/trunk/test/SemaOpenCL/invalid-clk-events-cl2.0.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaOpenCL/invalid-clk-events-cl2.0.cl?rev=345824&view=auto
==
--- cfe/trunk/test/SemaOpenCL/invalid-clk-events-cl2.0.cl (original)
+++ cfe/trunk/test/SemaOpenCL/invalid-clk-events-cl2.0.cl (removed)
@@ -1,3 +0,0 @@
-// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL2.0
-
-global clk_event_t ce; // expected-error {{the '__global clk_event_t' type 
cannot be used to declare a program scope variable}}


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


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

2018-11-01 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia updated this revision to Diff 172109.
Anastasia added a comment.

Addressed comments from John.


https://reviews.llvm.org/D53764

Files:
  include/clang/Sema/Sema.h
  lib/AST/Expr.cpp
  lib/CodeGen/CGExpr.cpp
  lib/Sema/DeclSpec.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaExprCXX.cpp
  lib/Sema/SemaInit.cpp
  lib/Sema/SemaType.cpp
  test/CodeGenOpenCLCXX/address-space-deduction.cl

Index: test/CodeGenOpenCLCXX/address-space-deduction.cl
===
--- /dev/null
+++ test/CodeGenOpenCLCXX/address-space-deduction.cl
@@ -0,0 +1,42 @@
+// RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=c++ -O0 -emit-llvm -o - | FileCheck %s -check-prefixes=COMMON,PTR
+// RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=c++ -O0 -emit-llvm -o - -DREF | FileCheck %s -check-prefixes=COMMON,REF
+
+#ifdef REF
+#define PTR &
+#define ADR(x) x
+#else
+#define PTR *
+#define ADR(x) &x
+#endif
+
+//COMMON: @glob = addrspace(1) global i32
+int glob;
+//PTR: @glob_p = addrspace(1) global i32 addrspace(4)* addrspacecast (i32 addrspace(1)* @glob to i32 addrspace(4)*)
+//REF: @glob_p = addrspace(1) global i32 addrspace(4)* null
+int PTR glob_p = ADR(glob);
+
+//COMMON: @_ZZ3fooi{{P|R}}U3AS4iE6loc_st = internal addrspace(1) global i32
+//PTR: @_ZZ3fooiPU3AS4iE8loc_st_p = internal addrspace(1) global i32 addrspace(4)* addrspacecast (i32 addrspace(1)* @_ZZ3fooiPU3AS4iE6loc_st to i32 addrspace(4)*)
+//REF: @_ZZ3fooiRU3AS4iE8loc_st_p = internal addrspace(1) global i32 addrspace(4)* null
+//COMMON: @loc_ext_p = external addrspace(1) {{global|constant}} i32 addrspace(4)*
+//COMMON: @loc_ext = external addrspace(1) global i32
+
+//REF: store i32 addrspace(4)* addrspacecast (i32 addrspace(1)* @glob to i32 addrspace(4)*), i32 addrspace(4)* addrspace(1)* @glob_p
+
+//COMMON: define spir_func i32 @_Z3fooi{{P|R}}U3AS4i(i32 %par, i32 addrspace(4)*{{.*}} %par_p)
+int foo(int par, int PTR par_p){
+//COMMON: %loc = alloca i32
+  int loc;
+//COMMON: %loc_p = alloca i32 addrspace(4)*
+//COMMON: [[GAS:%[0-9]+]] = addrspacecast i32* %loc to i32 addrspace(4)*
+//COMMON: store i32 addrspace(4)* [[GAS]], i32 addrspace(4)** %loc_p
+  int PTR loc_p = ADR(loc);
+
+// CHECK directives for the following code are located above.
+  static int loc_st;
+  static int PTR loc_st_p = ADR(loc_st);
+  extern int loc_ext;
+  extern int PTR loc_ext_p;
+  (void)loc_ext_p;
+  return loc_ext;
+}
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -7177,7 +7177,8 @@
   bool IsPointee =
   ChunkIndex > 0 &&
   (D.getTypeObject(ChunkIndex - 1).Kind == DeclaratorChunk::Pointer ||
-   D.getTypeObject(ChunkIndex - 1).Kind == DeclaratorChunk::BlockPointer);
+   D.getTypeObject(ChunkIndex - 1).Kind == DeclaratorChunk::BlockPointer ||
+   D.getTypeObject(ChunkIndex - 1).Kind == DeclaratorChunk::Reference);
   bool IsFuncReturnType =
   ChunkIndex > 0 &&
   D.getTypeObject(ChunkIndex - 1).Kind == DeclaratorChunk::Function;
Index: lib/Sema/SemaInit.cpp
===
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -7209,12 +7209,20 @@
   return CreateMaterializeTemporaryExpr(E->getType(), E, false);
 }
 
-ExprResult
-InitializationSequence::Perform(Sema &S,
-const InitializedEntity &Entity,
-const InitializationKind &Kind,
-MultiExprArg Args,
-QualType *ResultType) {
+ExprResult Sema::PerformQualificationConversion(Expr *E, QualType Ty,
+ExprValueKind VK,
+CheckedConversionKind CCK) {
+  CastKind CK = (Ty.getAddressSpace() != E->getType().getAddressSpace())
+? CK_AddressSpaceConversion
+: CK_NoOp;
+  return ImpCastExprToType(E, Ty, CK, VK, /*BasePath=*/nullptr, CCK);
+}
+
+ExprResult InitializationSequence::Perform(Sema &S,
+   const InitializedEntity &Entity,
+   const InitializationKind &Kind,
+   MultiExprArg Args,
+   QualType *ResultType) {
   if (Failed()) {
 Diagnose(S, Entity, Kind, Args);
 return ExprError();
@@ -7603,12 +7611,11 @@
 case SK_QualificationConversionRValue: {
   // Perform a qualification conversion; these can never go wrong.
   ExprValueKind VK =
-  Step->Kind == SK_QualificationConversionLValue ?
-  VK_LValue :
-  (Step->Kind == SK_QualificationConversionXValue ?
-   VK_XValue :
-   VK_RValue);
-  CurInit = S.ImpCastExprToType(CurInit.get(), Step->Type, CK_NoOp, VK);
+  Step->Kind == SK_Qualific

r345823 - Update to the 10-10 SARIF spec.

2018-11-01 Thread Aaron Ballman via cfe-commits
Author: aaronballman
Date: Thu Nov  1 04:52:07 2018
New Revision: 345823

URL: http://llvm.org/viewvc/llvm-project?rev=345823&view=rev
Log:
Update to the 10-10 SARIF spec.

This removes the Step property (which can be calculated by consumers 
trivially), and updates the schema and version numbers accordingly.

Modified:
cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp

cfe/trunk/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif
cfe/trunk/test/Analysis/diagnostics/sarif-diagnostics-taint-test.c

Modified: cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp?rev=345823&r1=345822&r2=345823&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp Thu Nov  1 04:52:07 
2018
@@ -154,10 +154,9 @@ static StringRef importanceToStr(Importa
   llvm_unreachable("Fully covered switch is not so fully covered");
 }
 
-static json::Object createThreadFlowLocation(int Step, json::Object &&Location,
+static json::Object createThreadFlowLocation(json::Object &&Location,
  Importance I) {
-  return json::Object{{"step", Step},
-  {"location", std::move(Location)},
+  return json::Object{{"location", std::move(Location)},
   {"importance", importanceToStr(I)}};
 }
 
@@ -192,12 +191,10 @@ static Importance calculateImportance(co
 static json::Object createThreadFlow(const PathPieces &Pieces,
  json::Object &Files) {
   const SourceManager &SMgr = Pieces.front()->getLocation().getManager();
-  int Step = 1;
   json::Array Locations;
   for (const auto &Piece : Pieces) {
 const PathDiagnosticLocation &P = Piece->getLocation();
 Locations.push_back(createThreadFlowLocation(
-Step++,
 createLocation(createPhysicalLocation(P.asRange(),
   *P.asLocation().getFileEntry(),
   SMgr, Files),
@@ -261,8 +258,10 @@ void SarifDiagnostics::FlushDiagnosticsI
 llvm::errs() << "warning: could not create file: " << EC.message() << '\n';
 return;
   }
-  json::Object Sarif{{"$schema", "http://json.schemastore.org/sarif-2.0.0"},
- {"version", "2.0.0-beta.2018-09-26"},
- {"runs", json::Array{createRun(Diags)}}};
+  json::Object Sarif{
+  {"$schema",
+   "http://json.schemastore.org/sarif-2.0.0-csd.2.beta.2018-10-10"},
+  {"version", "2.0.0-csd.2.beta.2018-10-10"},
+  {"runs", json::Array{createRun(Diags)}}};
   OS << llvm::formatv("{0:2}", json::Value(std::move(Sarif)));
 }

Modified: 
cfe/trunk/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif?rev=345823&r1=345822&r2=345823&view=diff
==
--- 
cfe/trunk/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif
 (original)
+++ 
cfe/trunk/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif
 Thu Nov  1 04:52:07 2018
@@ -1,5 +1,5 @@
 {
-  "$schema": "http://json.schemastore.org/sarif-2.0.0";,
+  "$schema": "http://json.schemastore.org/sarif-2.0.0-csd.2.beta.2018-10-10";,
   "runs": [
 {
   "files": {
@@ -7,7 +7,7 @@
   "fileLocation": {
 "uri": "file:sarif-diagnostics-taint-test.c"
   },
-  "length": 497,
+  "length": 510,
   "mimeType": "text/plain",
   "roles": [
 "resultFile"
@@ -38,8 +38,7 @@
 "startLine": 13
   }
 }
-  },
-  "step": 1
+  }
 },
 {
   "importance": "essential",
@@ -58,8 +57,7 @@
 "startLine": 9
   }
 }
-  },
-  "step": 2
+  }
 }
   ]
 }
@@ -95,5 +93,5 @@
   }
 }
   ],
-  "version": "2.0.0-beta.2018-09-26"
+  "version": "2.0.0-csd.2.beta.2018-10-10"
 }

Modified: cfe/trunk/test/Analysis/diagnostics/sarif-diagnostics-taint-test.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/diagnostics/sarif-diagnostics-taint-test.c?rev=345823&r1=345822&r2=345823&view=diff
==
--- cfe/trunk/test/Analysis/diagnostics/sarif-diagnostics-taint-test.c 

[PATCH] D52296: [Clang] - Add -gsingle-file-split-dwarf option.

2018-11-01 Thread George Rimar via Phabricator via cfe-commits
grimar added a comment.

Nice :) 
So seems the last unresolved question left is the naming of the new option?
If we do not want to go with `-gsingle-file-split-dwarf` then what it should be?

What about `-fdebug-fission` as an alias for `-gsplit-dwarf`.
And `-fsingle-file-debug-fission` for the new option?

Or, may be:

`-fdebug-fission` for the new option and then
`-fdebug-fission -fdebug-split-dwo` would work together as `-gsplit-dwarf`?


https://reviews.llvm.org/D52296



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


[PATCH] D34329: [clang-diff] Initial implementation.

2018-11-01 Thread Sylvestre Ledru via Phabricator via cfe-commits
sylvestre.ledru added a comment.

@johannes Someone asked for that in Debian: 
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=907269


Repository:
  rL LLVM

https://reviews.llvm.org/D34329



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


[PATCH] D53433: [clangd] auto-index stores symbols per-file instead of per-TU.

2018-11-01 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments.



Comment at: clangd/index/Background.cpp:194
-  // FIXME: partition the symbols by file rather than TU, to avoid duplication.
-  IndexedSymbols.update(AbsolutePath,
-llvm::make_unique(std::move(Symbols)),

sammccall wrote:
> um, this (before your patch) doesn't look even slightly threadsafe. Sorry I 
> didn't catch it in code review.
> 
> Now I'm not sure whether it makes sense for you to fix it or not. It seems 
> fine to use two independent locks here, to me - we don't need actual 
> consistency.
Ah, sorry for missing on my side as well. But IIUC, the only unsafe part is 
access to `FileHash` since `FileSymbols::update` is already thread-safe ?

If need be I can send out a patch to fix those issues.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53433



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


[PATCH] D52949: [Diagnostics] Implement -Wsizeof-pointer-div

2018-11-01 Thread Henry Wong via Phabricator via cfe-commits
MTC added a comment.

Sorry for the long delay for this patch! The implementation is fine for me. 
However, I'm the newbie on clang diagnostics and have no further insight into 
this checker. @aaron.ballman may have more valuable insights into this checker.




Comment at: test/Sema/div-sizeof-ptr.c:13
+int b3 = sizeof(*q) / sizeof(q);
+int b4 = sizeof(p) / sizeof(char);
+}

My bad! I didn't read the code carefully.


https://reviews.llvm.org/D52949



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


[PATCH] D53697: [ASTImporter][Structural Eq] Check for isBeingDefined

2018-11-01 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment.
Herald added a reviewer: shafik.

Shafik,

Sorry for the inconvenience.
Seems like the libcxx tests of lldb are automatically skipped on Linux, so it 
slipped through my test execution.
Also, I did not receive any email from this build server about the build 
failure. Nevertheless, I'll monitor the buildbots with any next ASTImporter 
commit.
I think the fix for the assertion is easy to have, but it seems very difficult 
to test without having libcxx/MacOs at hand. Is there a way to execute libcxx 
tests from Linux?


Repository:
  rC Clang

https://reviews.llvm.org/D53697



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


[PATCH] D53871: [OpenCL] Allow clk_event_t comparisons

2018-11-01 Thread Marco Antognini via Phabricator via cfe-commits
mantognini accepted this revision.
mantognini added a comment.
This revision is now accepted and ready to land.

LGTM, but please add a comment in the test file.




Comment at: test/SemaOpenCL/clk_event_t.cl:3
 
+#define CLK_NULL_EVENT (__builtin_astype(((void*)(__SIZE_MAX__)), clk_event_t))
+

It would be nice to have a comment saying this macro was extracted from the 
`opencl-c.h` header file.


Repository:
  rC Clang

https://reviews.llvm.org/D53871



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


[PATCH] D52998: [benchmark] Disable exceptions in Microsoft STL

2018-11-01 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev accepted this revision.
kbobyrev added a comment.
This revision is now accepted and ready to land.

In https://reviews.llvm.org/D52998#1258962, @eandrews wrote:

> Yes. I understand. At the moment, exception handling flags are generated 
> based on `BENCHMARK_ENABLE_EXCEPTIONS`  in `utils/benchmark/CMakeLists.txt` . 
>  However, `_HAS_EXCEPTIONS` is not defined based on this (code below). The 
> warnings are a result of this mismatch.
>
>   if (NOT BENCHMARK_ENABLE_EXCEPTIONS)
>   add_cxx_compiler_flag(-EHs-)
>   add_cxx_compiler_flag(-EHa-)
> endif()
>
>
> I think it makes most sense to add definition for `_HAS_EXCEPTIONS=0 `here as 
> opposed to modifying `llvm/CMakeLists.txt`.  Please correct me if I'm wrong. 
> I'm not too familiar with CMake. @kbobyrev Please let me know what you think 
> as well since you had suggested `llvm/CMakeLists.txt`.


As discussed in the mailing list, I think it also makes sense to set this

Sorry for the delay; yes, this looks good to me!


https://reviews.llvm.org/D52998



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


[PATCH] D53936: [clang-tidy] More clearly separate public, check-facing APIs from internal ones.

2018-11-01 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments.



Comment at: clang-tidy/ClangTidy.h:11
+//
+// It should remain as stable as possible, as many out-of-tree checks exist.
+//===--===//

steveire wrote:
> alexfh wrote:
> > sammccall wrote:
> > > steveire wrote:
> > > > Clang C++ code does not have any stability requirements. That's quite 
> > > > well-established.
> > > > 
> > > > This comment adds a new requirement of stability for this C++ API. You 
> > > > should probably put a RFC on cfe-dev about it. This header is no more 
> > > > public or stable than any other Clang header.
> > > > 
> > > > 
> > > > Clang C++ code does not have any stability requirements. That's quite 
> > > > well-established.
> > > 
> > > I was aiming for "stability is a design goal", not "this is a stable API 
> > > and you may not break it".
> > > Happy to take a shot at rewording, maybe you have suggestions?
> > > 
> > > > This comment adds a new requirement of stability for this C++ API. You 
> > > > should probably put a RFC on cfe-dev about it.
> > > 
> > > Stability here has always been an aim; I'm trying to document the current 
> > > state of the world.
> > > 
> > > e.g. in D15528 @alexfh wrote:
> > > > It's one of the goals of clang-tidy to provide an easy way to maintain 
> > > > out-of-tree checks.
> > > 
> > > > This header is no more public or stable than any other Clang header.
> > > I wish that were true - my current yakshave is reducing the need for 
> > > `registerPPCallbacks` to make clang-tidy more flexible for use in a 
> > > library. Removing it entirely would be great but having talked to some 
> > > clang-tidy maintainers, out-of-tree checks are at least a factor here.
> > I tend to agree with Stephen re: lack of stability guarantees of Clang 
> > APIs. Clang-tidy API on its own is not nearly enough to create a useful 
> > check, thus any out-of-tree check will use a significant amount of Clang 
> > APIs. Though the project aims at providing an easy way to maintain 
> > out-of-tree checks, there's little benefit in clang-tidy APIs being more 
> > stable than the API surface of Clang needed for an average out-of-tree 
> > check. Maintainers of any clang-based out-of-tree code should be ready to 
> > update their code with every change of the clang revision they use. 
> > Clang-tidy checks are not an exception. Putting this documentation here may 
> > send a very wrong message and serve badly both to the clang-tidy developers 
> > and to the folks who decide to depend on it.
> > 
> > A softer version of this (something along the lines of "A large number of 
> > out-of-tree checks depend on this API, so be careful when introducing 
> > potentially breaking changes.") may work here, but the same note will be 
> > needed on the whole AST, ASTMatchers, and Lex libraries in Clang (and many 
> > other ones, I guess). 
> I have a proposal coming up which involves a breaking change to AST_MATCHER* 
> macros (many of which are out of tree), so I'm really just trying to make 
> sure no backward compatibility concern opposes that.
> 
> I still see no point in the comment, if you don't also put the same comment 
> in ASTMatchers.h, each header in AST/ etc. The files have the same 
> out-of-tree use, and no one of them deserves a comment more than the others.
> 
> In fact, I had the idea just last week that I want to write a documentation 
> page outlining compatibility guarantees in various user-facing parts of 
> llvm/clang. It would be a single page containing the two sections about 
> compatibility here: 
> https://llvm.org/docs/DeveloperPolicy.html#ir-backwards-compatibility, C++ 
> API stability, whether clang-query interpreter commands are stable, whether 
> driver command lines and plumbing command lines are stable etc. AFAIK, that 
> does not exist yet.
> 
> Regarding C++ API compatibility something along the lines here 
> (http://clang-developers.42468.n3.nabble.com/getLocStart-versus-getStartLoc-tp4061010p4061292.html)
>  and here 
> (http://clang-developers.42468.n3.nabble.com/API-Removal-Notice-4-weeks-getStartLoc-getLocStart-getLocEnd-td4061566.html)
>  . A note like yours would belong on such a page too. I don't think it 
> belongs in this header.
> I wish that were true - my current yakshave is reducing the need for 
> registerPPCallbacks to make clang-tidy more flexible for use in a library. 
> Removing it entirely would be great but having talked to some clang-tidy 
> maintainers, out-of-tree checks are at least a factor here.

What do you mean by that, exactly?
Having PP callbacks in the clang-tidy checks is essential for many checks.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53936



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


[PATCH] D53936: [clang-tidy] More clearly separate public, check-facing APIs from internal ones.

2018-11-01 Thread Stephen Kelly via Phabricator via cfe-commits
steveire added inline comments.



Comment at: clang-tidy/ClangTidy.h:11
+//
+// It should remain as stable as possible, as many out-of-tree checks exist.
+//===--===//

alexfh wrote:
> sammccall wrote:
> > steveire wrote:
> > > Clang C++ code does not have any stability requirements. That's quite 
> > > well-established.
> > > 
> > > This comment adds a new requirement of stability for this C++ API. You 
> > > should probably put a RFC on cfe-dev about it. This header is no more 
> > > public or stable than any other Clang header.
> > > 
> > > 
> > > Clang C++ code does not have any stability requirements. That's quite 
> > > well-established.
> > 
> > I was aiming for "stability is a design goal", not "this is a stable API 
> > and you may not break it".
> > Happy to take a shot at rewording, maybe you have suggestions?
> > 
> > > This comment adds a new requirement of stability for this C++ API. You 
> > > should probably put a RFC on cfe-dev about it.
> > 
> > Stability here has always been an aim; I'm trying to document the current 
> > state of the world.
> > 
> > e.g. in D15528 @alexfh wrote:
> > > It's one of the goals of clang-tidy to provide an easy way to maintain 
> > > out-of-tree checks.
> > 
> > > This header is no more public or stable than any other Clang header.
> > I wish that were true - my current yakshave is reducing the need for 
> > `registerPPCallbacks` to make clang-tidy more flexible for use in a 
> > library. Removing it entirely would be great but having talked to some 
> > clang-tidy maintainers, out-of-tree checks are at least a factor here.
> I tend to agree with Stephen re: lack of stability guarantees of Clang APIs. 
> Clang-tidy API on its own is not nearly enough to create a useful check, thus 
> any out-of-tree check will use a significant amount of Clang APIs. Though the 
> project aims at providing an easy way to maintain out-of-tree checks, there's 
> little benefit in clang-tidy APIs being more stable than the API surface of 
> Clang needed for an average out-of-tree check. Maintainers of any clang-based 
> out-of-tree code should be ready to update their code with every change of 
> the clang revision they use. Clang-tidy checks are not an exception. Putting 
> this documentation here may send a very wrong message and serve badly both to 
> the clang-tidy developers and to the folks who decide to depend on it.
> 
> A softer version of this (something along the lines of "A large number of 
> out-of-tree checks depend on this API, so be careful when introducing 
> potentially breaking changes.") may work here, but the same note will be 
> needed on the whole AST, ASTMatchers, and Lex libraries in Clang (and many 
> other ones, I guess). 
I have a proposal coming up which involves a breaking change to AST_MATCHER* 
macros (many of which are out of tree), so I'm really just trying to make sure 
no backward compatibility concern opposes that.

I still see no point in the comment, if you don't also put the same comment in 
ASTMatchers.h, each header in AST/ etc. The files have the same out-of-tree 
use, and no one of them deserves a comment more than the others.

In fact, I had the idea just last week that I want to write a documentation 
page outlining compatibility guarantees in various user-facing parts of 
llvm/clang. It would be a single page containing the two sections about 
compatibility here: 
https://llvm.org/docs/DeveloperPolicy.html#ir-backwards-compatibility, C++ API 
stability, whether clang-query interpreter commands are stable, whether driver 
command lines and plumbing command lines are stable etc. AFAIK, that does not 
exist yet.

Regarding C++ API compatibility something along the lines here 
(http://clang-developers.42468.n3.nabble.com/getLocStart-versus-getStartLoc-tp4061010p4061292.html)
 and here 
(http://clang-developers.42468.n3.nabble.com/API-Removal-Notice-4-weeks-getStartLoc-getLocStart-getLocEnd-td4061566.html)
 . A note like yours would belong on such a page too. I don't think it belongs 
in this header.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53936



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


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

2018-11-01 Thread Nikolai Kosjar via Phabricator via cfe-commits
nik added a comment.

I've only the minimal example at hand right know - I'm waiting for feedback 
about the real world case.


Repository:
  rC Clang

https://reviews.llvm.org/D53866



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


Re: r345660 - [clang][ubsan] Implicit Conversion Sanitizer - integer sign change - clang part

2018-11-01 Thread Roman Lebedev via cfe-commits
On Thu, Nov 1, 2018 at 9:39 AM Roman Lebedev  wrote:
>
> On Thu, Nov 1, 2018 at 3:03 AM Friedman, Eli  wrote:
> >
> > On 10/30/2018 2:58 PM, Roman Lebedev via cfe-commits wrote:
> > > Author: lebedevri
> > > Date: Tue Oct 30 14:58:56 2018
> > > New Revision: 345660
> > >
> > > URL: http://llvm.org/viewvc/llvm-project?rev=345660&view=rev
> > > Log:
> > > [clang][ubsan] Implicit Conversion Sanitizer - integer sign change - 
> > > clang part
> > >
> > > This is the second half of Implicit Integer Conversion Sanitizer.
> > > It completes the first half, and finally makes the sanitizer
> > > fully functional! Only the bitfield handling is missing.
> >
> > This is causing failures on the polly-aosp buildbot
> > (http://lab.llvm.org:8011/builders/aosp-O3-polly-before-vectorizer-unprofitable/builds/690/steps/build-aosp/logs/stdio
> > ).  I haven't tried to reduce the issue yet; let me know if you need help.
> Thanks!
> I'm able to cause a similar crash, will handle it from here.
> It *should* be trivial/fast to triage, so maybe the revert can be
> avoided. Or not.
Fixed in r345816.

> > -Eli
Roman.

> > --
> > Employee of Qualcomm Innovation Center, Inc.
> > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux 
> > Foundation Collaborative Project
> >
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r345816 - [clang][CodeGen] ImplicitIntegerSignChangeSanitizer: actually ignore NOP casts.

2018-11-01 Thread Roman Lebedev via cfe-commits
Author: lebedevri
Date: Thu Nov  1 01:56:51 2018
New Revision: 345816

URL: http://llvm.org/viewvc/llvm-project?rev=345816&view=rev
Log:
[clang][CodeGen] ImplicitIntegerSignChangeSanitizer: actually ignore NOP casts.

I fully expected for that to be handled by the canonical type check,
but it clearly wasn't. Sadly, somehow it hide until now.

Reported by Eli Friedman.

Modified:
cfe/trunk/lib/CodeGen/CGExprScalar.cpp
cfe/trunk/test/CodeGen/catch-implicit-integer-sign-changes-true-negatives.c

Modified: cfe/trunk/lib/CodeGen/CGExprScalar.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprScalar.cpp?rev=345816&r1=345815&r2=345816&view=diff
==
--- cfe/trunk/lib/CodeGen/CGExprScalar.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExprScalar.cpp Thu Nov  1 01:56:51 2018
@@ -1127,10 +1127,9 @@ void ScalarExprEmitter::EmitIntegerSignC
   // Now, we do not need to emit the check in *all* of the cases.
   // We can avoid emitting it in some obvious cases where it would have been
   // dropped by the opt passes (instcombine) always anyways.
-  // If it's a cast between the same type, just differently-sugared. no check.
-  QualType CanonSrcType = CGF.getContext().getCanonicalType(SrcType);
-  QualType CanonDstType = CGF.getContext().getCanonicalType(DstType);
-  if (CanonSrcType == CanonDstType)
+  // If it's a cast between effectively the same type, no check.
+  // NOTE: this is *not* equivalent to checking the canonical types.
+  if (SrcSigned == DstSigned && SrcBits == DstBits)
 return;
   // At least one of the values needs to have signed type.
   // If both are unsigned, then obviously, neither of them can be negative.

Modified: 
cfe/trunk/test/CodeGen/catch-implicit-integer-sign-changes-true-negatives.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/catch-implicit-integer-sign-changes-true-negatives.c?rev=345816&r1=345815&r2=345816&view=diff
==
--- cfe/trunk/test/CodeGen/catch-implicit-integer-sign-changes-true-negatives.c 
(original)
+++ cfe/trunk/test/CodeGen/catch-implicit-integer-sign-changes-true-negatives.c 
Thu Nov  1 01:56:51 2018
@@ -138,3 +138,15 @@ uint32_t unsigned_int_to_uint32(unsigned
 uint32_t uint32_to_uint32(uint32_t src) {
   return src;
 }
+
+// "Transparent" Enum.
+// == 
//
+
+enum a { b = ~2147483647 };
+enum a c();
+void d(int);
+void e();
+void e() {
+  enum a f = c();
+  d(f);
+}


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


[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-11-01 Thread Hans Wennborg via Phabricator via cfe-commits
hans added a comment.

Thank you Takuto! I think the functionality looks great now: it's not too 
complicated :-)

I only have comments about the test.




Comment at: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp:3
+// RUN: -fno-dllexport-inlines -emit-llvm -O0 -o - |\
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=NOINLINE %s
+

Instead of "DEFAULT", I think "CHECK" is the default prefix to use when using 
the same prefix for multiple invocations.

Also, instead of INLINE and NOINLINE (which sounds like it's about inlining), 
I'd suggest perhaps "EXPORTINLINES" and "NOEXPORTINLINES" or something like 
that.



Comment at: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp:6
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc   \
+// RUN: -emit-llvm -O0 -o - |   \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=INLINE %s

Instead of using -O0 here and above, consider using -O1 -disable-llvm-passes so 
we get available_externally functions.



Comment at: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp:9
+
+// Function
+

This change doesn't have any effect on non-member functions, so I don't think 
we need these tests.



Comment at: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp:42
+
+// check for local static variables
+// NOINLINE-DAG: 
@"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA"
 = weak_odr dso_local dllexport global i32 0, comdat, align 4

Can you move the checks down to where the variable/function is defined? That 
makes it easier to read the test I think.



Comment at: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp:49
+
+class __declspec(dllexport) NoTemplateExportedClass {
+ public:

I think this test case should be first in the file, because it's really showing 
the core of this feature.

I would suggest just calling "ExportedClass" to make it simpler (if it was a 
template, then we'd put template in the name). Also, I suggest making it a 
struct instead so you don't have to write "public:".



Comment at: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp:51
+ public:
+  // DEFAULT-NOT: NoTemplateExportedClass@NoTemplateExportedClass@@
+  NoTemplateExportedClass() = default;

This check is only using part of the mangled name..
But I don't think the patch does anything special for this kind of function, so 
I'd just skip this test.



Comment at: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp:58
+
+  int f();
+

This doesn't seem to be used for anything?



Comment at: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp:73
+  // DEFAULT-NOT: InlineOutclassDefFuncWihtoutDefinition
+  __forceinline void InlineOutclassDefFuncWihtoutDefinition();
+

Why __foceinline? I think it should be just "inline". This goes for all use of 
__forceinline in this file.



Comment at: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp:76
+  // DEFAULT-NOT: InlineOutclassDefFunc@NoTemplateExportedClass@@
+  __forceinline void InlineOutclassDefFunc();
+

Hmm, I would have thought we should export this function.



Comment at: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp:102
+  void InclassDefFunc() {}
+  void OutclassDefFunc();
+

I'm not sure that OutclassDefFunc() adds much value to the test. Also 
templateValue below. I think they can be removed.



Comment at: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp:122
+template class TemplateExportedClass;
+
+

We should also have a test with implicit instantiation, and then the inline 
function should not be exported when using /Zc:dllexportInlines-.



Comment at: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp:132
+  }
+  void OutclassDefFunc();
+};

Again, I think we can drop the out-of-line function since this change should 
not affect those.



Comment at: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp:144
+// DEFAULT-DAG: define weak_odr dso_local void 
@"?OutclassDefFunc@?$TemplateNoExportedClass@VB22QEAAXXZ"
+template class TemplateNoExportedClass;
+

This case is not interesting, since it has nothing to do with dllexport/import, 
I think it can be removed.



Comment at: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp:148
+// DEFAULT-NOT: ?OutclassDefFunc@?$TemplateNoExportedClass@VC33@
+extern template class TemplateNoExportedClass;
+

Ditto.



Comment at: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp:165
+class __

[PATCH] D53970: [CMake][Fuchsia] Don't restrict Linux runtimes to UNIX

2018-11-01 Thread Petr Hosek via Phabricator via cfe-commits
phosek created this revision.
phosek added a reviewer: mcgrathr.
Herald added subscribers: cfe-commits, mgorny.

This allows building Linux runtimes on any platform if the correct
sysroot is provided via CMake option.


Repository:
  rC Clang

https://reviews.llvm.org/D53970

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


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -31,40 +31,40 @@
 if(APPLE)
   list(APPEND BUILTIN_TARGETS "default")
   list(APPEND RUNTIME_TARGETS "default")
-elseif(UNIX)
-  foreach(target i386;x86_64;armhf;aarch64)
-if(LINUX_${target}_SYSROOT)
-  # Set the per-target builtins options.
-  list(APPEND BUILTIN_TARGETS "${target}-linux-gnu")
-  set(BUILTINS_${target}-linux-gnu_CMAKE_SYSTEM_NAME Linux CACHE STRING "")
-  set(BUILTINS_${target}-linux-gnu_CMAKE_BUILD_TYPE Release CACHE STRING 
"")
-  set(BUILTINS_${target}-linux-gnu_CMAKE_SYSROOT 
${LINUX_${target}_SYSROOT} CACHE STRING "")
-
-  # Set the per-target runtimes options.
-  list(APPEND RUNTIME_TARGETS "${target}-linux-gnu")
-  set(RUNTIMES_${target}-linux-gnu_CMAKE_SYSTEM_NAME Linux CACHE STRING "")
-  set(RUNTIMES_${target}-linux-gnu_CMAKE_BUILD_TYPE Release CACHE STRING 
"")
-  set(RUNTIMES_${target}-linux-gnu_CMAKE_SYSROOT 
${LINUX_${target}_SYSROOT} CACHE STRING "")
-  set(RUNTIMES_${target}-linux-gnu_LLVM_ENABLE_ASSERTIONS ON CACHE BOOL "")
-  set(RUNTIMES_${target}-linux-gnu_LIBUNWIND_ENABLE_SHARED OFF CACHE BOOL 
"")
-  set(RUNTIMES_${target}-linux-gnu_LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL 
"")
-  set(RUNTIMES_${target}-linux-gnu_LIBUNWIND_INSTALL_LIBRARY OFF CACHE 
BOOL "")
-  set(RUNTIMES_${target}-linux-gnu_LIBCXXABI_USE_COMPILER_RT ON CACHE BOOL 
"")
-  set(RUNTIMES_${target}-linux-gnu_LIBCXXABI_ENABLE_SHARED OFF CACHE BOOL 
"")
-  set(RUNTIMES_${target}-linux-gnu_LIBCXXABI_USE_LLVM_UNWINDER ON CACHE 
BOOL "")
-  set(RUNTIMES_${target}-linux-gnu_LIBCXXABI_ENABLE_STATIC_UNWINDER ON 
CACHE BOOL "")
-  set(RUNTIMES_${target}-linux-gnu_LIBCXXABI_INSTALL_LIBRARY OFF CACHE 
BOOL "")
-  set(RUNTIMES_${target}-linux-gnu_LIBCXX_USE_COMPILER_RT ON CACHE BOOL "")
-  set(RUNTIMES_${target}-linux-gnu_LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
-  set(RUNTIMES_${target}-linux-gnu_LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON 
CACHE BOOL "")
-  set(RUNTIMES_${target}-linux-gnu_LIBCXX_ABI_VERSION 2 CACHE STRING "")
-  set(RUNTIMES_${target}-linux-gnu_SANITIZER_CXX_ABI "libc++" CACHE STRING 
"")
-  set(RUNTIMES_${target}-linux-gnu_SANITIZER_CXX_ABI_INTREE ON CACHE BOOL 
"")
-  set(RUNTIMES_${target}-linux-gnu_COMPILER_RT_USE_BUILTINS_LIBRARY ON 
CACHE BOOL "")
-endif()
-  endforeach()
 endif()
 
+foreach(target i386;x86_64;armhf;aarch64)
+  if(LINUX_${target}_SYSROOT)
+# Set the per-target builtins options.
+list(APPEND BUILTIN_TARGETS "${target}-linux-gnu")
+set(BUILTINS_${target}-linux-gnu_CMAKE_SYSTEM_NAME Linux CACHE STRING "")
+set(BUILTINS_${target}-linux-gnu_CMAKE_BUILD_TYPE Release CACHE STRING "")
+set(BUILTINS_${target}-linux-gnu_CMAKE_SYSROOT ${LINUX_${target}_SYSROOT} 
CACHE STRING "")
+
+# Set the per-target runtimes options.
+list(APPEND RUNTIME_TARGETS "${target}-linux-gnu")
+set(RUNTIMES_${target}-linux-gnu_CMAKE_SYSTEM_NAME Linux CACHE STRING "")
+set(RUNTIMES_${target}-linux-gnu_CMAKE_BUILD_TYPE Release CACHE STRING "")
+set(RUNTIMES_${target}-linux-gnu_CMAKE_SYSROOT ${LINUX_${target}_SYSROOT} 
CACHE STRING "")
+set(RUNTIMES_${target}-linux-gnu_LLVM_ENABLE_ASSERTIONS ON CACHE BOOL "")
+set(RUNTIMES_${target}-linux-gnu_LIBUNWIND_ENABLE_SHARED OFF CACHE BOOL "")
+set(RUNTIMES_${target}-linux-gnu_LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL 
"")
+set(RUNTIMES_${target}-linux-gnu_LIBUNWIND_INSTALL_LIBRARY OFF CACHE BOOL 
"")
+set(RUNTIMES_${target}-linux-gnu_LIBCXXABI_USE_COMPILER_RT ON CACHE BOOL 
"")
+set(RUNTIMES_${target}-linux-gnu_LIBCXXABI_ENABLE_SHARED OFF CACHE BOOL "")
+set(RUNTIMES_${target}-linux-gnu_LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL 
"")
+set(RUNTIMES_${target}-linux-gnu_LIBCXXABI_ENABLE_STATIC_UNWINDER ON CACHE 
BOOL "")
+set(RUNTIMES_${target}-linux-gnu_LIBCXXABI_INSTALL_LIBRARY OFF CACHE BOOL 
"")
+set(RUNTIMES_${target}-linux-gnu_LIBCXX_USE_COMPILER_RT ON CACHE BOOL "")
+set(RUNTIMES_${target}-linux-gnu_LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
+set(RUNTIMES_${target}-linux-gnu_LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE 
BOOL "")
+set(RUNTIMES_${target}-linux-gnu_LIBCXX_ABI_VERSION 2 CACHE STRING "")
+set(RUNTIMES_${target}-linux-gnu_SANITIZER_CXX_ABI "libc++" CACHE STRING 
"")
+set(RUNTIMES_${target}-linux-gnu_SANITIZER_CXX_ABI_INTREE ON CACHE BOOL "")
+set(RUNTIMES_${target}-linux-gnu_COMPILER_RT_USE_BUILTINS_LIBRARY ON CACHE 
BOOL "")
+  endif

<    1   2