[PATCH] D44994: [clang-format] Ensure wrapped ObjC selectors with 1 arg obey IndentWrappedFunctionNames

[Jacek Olesiak via Phabricator via cfe-commits]

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

Well spotted.


Repository:
  rC Clang

https://reviews.llvm.org/D44994



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

[PATCH] D44826: Add -Wunused-using, a warning that finds unused using declarations.

[Carlos Alberto Enciso via Phabricator via cfe-commits]

CarlosAlbertoEnciso added a comment.

@dblaikie,

Thanks for your feedback and questions. I will address them.

Currently I am looking with more detail the tracking of the 'used' and 
'referenced' bits.


Repository:
  rC Clang

https://reviews.llvm.org/D44826



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

[PATCH] D44559: [Sema] Wrong width of result of mul operation

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 added a comment.

In https://reviews.llvm.org/D44559#1049472, @rjmccall wrote:

> You are welcome to start a thread on cfe-dev to gather support for changing 
> the design of -Wconversion to always warn about the potential for overflow in 
> sub-int multiplication.


Maybe the stupid question: what if we introduce another special option 
-Wnoicywarn?


https://reviews.llvm.org/D44559



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

[PATCH] D44996: [clang-format] Ensure ObjC selectors with 0 args are annotated correctly

[Jacek Olesiak via Phabricator via cfe-commits]

jolesiak added inline comments.



Comment at: unittests/Format/FormatTestObjC.cpp:527
+  // Make sure selectors with 0, 1, or more arguments are not indented
+  // when IndentWrappedFunctionNames is false.
+  verifyFormat("- (a)\n"

I know that `Style.IndentWrappedFunctionNames` is false by default, but how 
about adding 
```
Style.IndentWrappedFunctionNames = false;
```
before these `verifyFormat` calls? I feel like that makes it more readable and 
makes test independent on this `Style` constant.


Repository:
  rC Clang

https://reviews.llvm.org/D44996



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

[PATCH] D44765: PR36643 Make clang-format support more generic TMarcos

[Teodor Petrov via Phabricator via cfe-commits]

obfuscated updated this revision to Diff 140191.
obfuscated marked an inline comment as done.
obfuscated added a comment.

Fixed tests. Fixed description.


https://reviews.llvm.org/D44765

Files:
  include/clang/Format/Format.h
  lib/Format/ContinuationIndenter.cpp
  lib/Format/Format.cpp
  lib/Format/FormatToken.h
  lib/Format/FormatTokenLexer.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -7935,6 +7935,48 @@
"_T(\"Xn\"));"));
 }
 
+TEST_F(FormatTest, BreaksStringLiteralsWithin_GenericTMacro) {
+  FormatStyle Style = getLLVMStyleWithColumns(25);
+  Style.TMacros.push_back("blablaT");
+  EXPECT_EQ(
+  "blablaT(\"aa\")\n"
+  "blablaT(\"aa\")\n"
+  "blablaT(\"\")",
+  format("  blablaT(\"\")", Style));
+  EXPECT_EQ("f(x,\n"
+"  blablaT(\"\")\n"
+"  blablaT(\"aaa\"),\n"
+"  z);",
+format("f(x, blablaT(\"aaa\"), z);", Style));
+
+  // FIXME: Handle embedded spaces in one iteration.
+  //  EXPECT_EQ("blablaT(\"a\")\n"
+  //"blablaT(\"a\")\n"
+  //"blablaT(\"a\")\n"
+  //"blablaT(\"a\")",
+  //format("  blablaT ( \"\" )",
+  //   getLLVMStyleWithColumns(20)));
+  EXPECT_EQ(
+  "blablaT ( \"\" )",
+  format("  blablaT ( \"\" )", Style));
+  EXPECT_EQ("f(\n"
+"#if !TEST\n"
+"blablaT(\"Xn\")\n"
+"#endif\n"
+");",
+format("f(\n"
+   "#if !TEST\n"
+   "blablaT(\"Xn\")\n"
+   "#endif\n"
+   ");"));
+  EXPECT_EQ("f(\n"
+"\n"
+"blablaT(\"Xn\"));",
+format("f(\n"
+   "\n"
+   "blablaT(\"Xn\"));"));
+}
+
 TEST_F(FormatTest, BreaksStringLiteralOperands) {
   // In a function call with two operands, the second can be broken with no line
   // break before it.
@@ -10647,6 +10689,10 @@
   "  - 'CPPEVAL'\n"
   "CanonicalDelimiter: 'cc'",
   RawStringFormats, ExpectedRawStringFormats);
+
+  Style.TMacros.clear();
+  std::vector ExpectedTMacros = {"_T", "myT"};
+  CHECK_PARSE("TMacros: [_T, myT]", TMacros, ExpectedTMacros);
 }
 
 TEST_F(FormatTest, ParsesConfigurationWithLanguages) {
Index: lib/Format/FormatTokenLexer.cpp
===
--- lib/Format/FormatTokenLexer.cpp
+++ lib/Format/FormatTokenLexer.cpp
@@ -368,7 +368,8 @@
 return false;
 
   FormatToken *Macro = Tokens[Tokens.size() - 4];
-  if (Macro->TokenText != "_T")
+  if (std::find(Style.TMacros.begin(), Style.TMacros.end(), Macro->TokenText) ==
+  Style.TMacros.end())
 return false;
 
   const char *Start = Macro->TokenText.data();
@@ -382,6 +383,7 @@
   String->TokenText, String->OriginalColumn, Style.TabWidth, Encoding);
   String->NewlinesBefore = Macro->NewlinesBefore;
   String->HasUnescapedNewline = Macro->HasUnescapedNewline;
+  String->TMacroStringLiteral = true;
 
   Tokens.pop_back();
   Tokens.pop_back();
Index: lib/Format/FormatToken.h
===
--- lib/Format/FormatToken.h
+++ lib/Format/FormatToken.h
@@ -134,6 +134,9 @@
   /// Token.
   bool HasUnescapedNewline = false;
 
+  /// \brief Whether this is a string literal similar to _T("sdfsdf").
+  bool TMacroStringLiteral = false;
+
   /// \brief The range of the whitespace immediately preceding the \c Token.
   SourceRange WhitespaceRange;
 
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -430,6 +430,7 @@
 IO.mapOptional("SpacesInSquareBrackets", Style.SpacesInSquareBrackets);
 IO.mapOptional("Standard", Style.Standard);
 IO.mapOptional("TabWidth", Style.TabWidth);
+IO.mapOptional("TMacros", Style.TMacros);
 IO.mapOptional("UseTab", Style.UseTab);
   }
 };
@@ -686,6 +687,7 @@
   LLVMStyle.DisableFormat = false;
   LLVMStyle.SortIncludes = true;
   LLVMStyle.SortUsingDeclarations = true;
+  LLVMStyle.TMacros.push_back("_T");
 
   return LLVMStyle;
 }
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -1599,12 +1599,10 @@
 // FIXME: Store Prefix and Suffix (or Pre

[PATCH] D44996: [clang-format] Ensure ObjC selectors with 0 args are annotated correctly

[Jacek Olesiak via Phabricator via cfe-commits]

jolesiak added inline comments.



Comment at: unittests/Format/FormatTestObjC.cpp:527
+  // Make sure selectors with 0, 1, or more arguments are not indented
+  // when IndentWrappedFunctionNames is false.
+  verifyFormat("- (a)\n"

jolesiak wrote:
> I know that `Style.IndentWrappedFunctionNames` is false by default, but how 
> about adding 
> ```
> Style.IndentWrappedFunctionNames = false;
> ```
> before these `verifyFormat` calls? I feel like that makes it more readable 
> and makes test independent on this `Style` constant.
This comment is kind of irrelevant as this change is included in D45005.


Repository:
  rC Clang

https://reviews.llvm.org/D44996



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

[PATCH] D44996: [clang-format] Ensure ObjC selectors with 0 args are annotated correctly

[Jacek Olesiak via Phabricator via cfe-commits]

jolesiak added inline comments.



Comment at: unittests/Format/FormatTestObjC.cpp:527
+  // Make sure selectors with 0, 1, or more arguments are not indented
+  // when IndentWrappedFunctionNames is false.
+  verifyFormat("- (a)\n"

jolesiak wrote:
> jolesiak wrote:
> > I know that `Style.IndentWrappedFunctionNames` is false by default, but how 
> > about adding 
> > ```
> > Style.IndentWrappedFunctionNames = false;
> > ```
> > before these `verifyFormat` calls? I feel like that makes it more readable 
> > and makes test independent on this `Style` constant.
> This comment is kind of irrelevant as this change is included in D45005.
My bad, this change is NOT included in D45005. (I don't know how to remove 
inline comment)


Repository:
  rC Clang

https://reviews.llvm.org/D44996



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

[PATCH] D44381: [mips] Prevent PIC to be set to level 2

[Simon Dardis via Phabricator via cfe-commits]

sdardis added inline comments.



Comment at: lib/Driver/ToolChains/CommonArgs.cpp:983
+  return std::make_tuple(llvm::Reloc::Static, 0U, false);
+// It's never PIC level 2 for mips.
+IsPICLevelTwo = false;

Can you reformulate this comment to say that MIPS does not use PIC level 2? 
Also, please add a note stating that even with -mxgot / -fPIC / multigot , MIPS 
does not use PIC level 2 unlike other architecutres for historical reasons.


Repository:
  rC Clang

https://reviews.llvm.org/D44381



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

[PATCH] D44765: PR36643 Make clang-format support more generic TMarcos

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added inline comments.



Comment at: lib/Format/FormatTokenLexer.cpp:386
   String->HasUnescapedNewline = Macro->HasUnescapedNewline;
+  String->TMacroStringLiteral = true;
 

In the original code, TMacro detection was done as:
```
(Text.startswith(Prefix = "_T(\"") && Text.endswith(Postfix = "\")"))
```
In the new code the left part is saved in TMacroStringLiteral, and the right 
part is checked in ContinuationIndenter. Could you keep them together in 
`FormatTokenLexer`.
@alexfh, why are we checking for the right check at all? What would be an 
example where this is needed to disambiguate?



https://reviews.llvm.org/D44765



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

[PATCH] D41808: Rename clang link from clang-X.Y to clang-X

[Sylvestre Ledru via Phabricator via cfe-commits]

sylvestre.ledru updated this revision to Diff 140202.
sylvestre.ledru edited the summary of this revision.
sylvestre.ledru added a comment.

Add to the release notes


Repository:
  rC Clang

https://reviews.llvm.org/D41808

Files:
  CMakeLists.txt
  docs/ReleaseNotes.rst


Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -62,6 +62,9 @@
 Non-comprehensive list of changes in this release
 -
 
+- clang binary and libraries have been renamed from 7.0 to 7.
+  For example, the clang binary will be called clang-7 instead of clang-7.0.
+
 - ...
 
 New Compiler Flags
Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -424,11 +424,11 @@
 
 # Clang version information
 set(CLANG_EXECUTABLE_VERSION
- "${CLANG_VERSION_MAJOR}.${CLANG_VERSION_MINOR}" CACHE STRING
-"Version number that will be placed into the clang executable, in the form 
XX.YY")
+"${CLANG_VERSION_MAJOR}" CACHE STRING
+"Major version number that will be appended to the clang executable name")
 set(LIBCLANG_LIBRARY_VERSION
- "${CLANG_VERSION_MAJOR}.${CLANG_VERSION_MINOR}" CACHE STRING
-"Version number that will be placed into the libclang library , in the 
form XX.YY")
+"${CLANG_VERSION_MAJOR}" CACHE STRING
+"Major version number that will be appended to the libclang library")
 mark_as_advanced(CLANG_EXECUTABLE_VERSION LIBCLANG_LIBRARY_VERSION)
 
 option(CLANG_INCLUDE_TESTS


Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -62,6 +62,9 @@
 Non-comprehensive list of changes in this release
 -
 
+- clang binary and libraries have been renamed from 7.0 to 7.
+  For example, the clang binary will be called clang-7 instead of clang-7.0.
+
 - ...
 
 New Compiler Flags
Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -424,11 +424,11 @@
 
 # Clang version information
 set(CLANG_EXECUTABLE_VERSION
- "${CLANG_VERSION_MAJOR}.${CLANG_VERSION_MINOR}" CACHE STRING
-"Version number that will be placed into the clang executable, in the form XX.YY")
+"${CLANG_VERSION_MAJOR}" CACHE STRING
+"Major version number that will be appended to the clang executable name")
 set(LIBCLANG_LIBRARY_VERSION
- "${CLANG_VERSION_MAJOR}.${CLANG_VERSION_MINOR}" CACHE STRING
-"Version number that will be placed into the libclang library , in the form XX.YY")
+"${CLANG_VERSION_MAJOR}" CACHE STRING
+"Major version number that will be appended to the libclang library")
 mark_as_advanced(CLANG_EXECUTABLE_VERSION LIBCLANG_LIBRARY_VERSION)
 
 option(CLANG_INCLUDE_TESTS
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44815: [AArch64]: Add support for parsing rN registers.

[Peter Smith via Phabricator via cfe-commits]

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

Given that this is important for compiling the Linux kernel with clang I think 
that it is worth improving compatibility with GCC. LGTM.


Repository:
  rC Clang

https://reviews.llvm.org/D44815



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

r328769 - Rename clang link from clang-X.Y to clang-X

[Sylvestre Ledru via cfe-commits]

Author: sylvestre
Date: Thu Mar 29 03:05:46 2018
New Revision: 328769

URL: http://llvm.org/viewvc/llvm-project?rev=328769&view=rev
Log:
Rename clang link from clang-X.Y to clang-X

Summary:
As we are only doing X.0.Z releases (not using the minor version), there is no 
need to keep -X.Y in the version.
So, instead, I propose the following:
Instead of having clang-7.0 in bin/, we will have clang-7

Since also matches was gcc is doing.

Reviewers: tstellar, dlj, dim, hans

Reviewed By: dim, hans

Subscribers: dim, mgorny, cfe-commits

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

Modified:
cfe/trunk/CMakeLists.txt
cfe/trunk/docs/ReleaseNotes.rst

Modified: cfe/trunk/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/CMakeLists.txt?rev=328769&r1=328768&r2=328769&view=diff
==
--- cfe/trunk/CMakeLists.txt (original)
+++ cfe/trunk/CMakeLists.txt Thu Mar 29 03:05:46 2018
@@ -424,11 +424,11 @@ endif()
 
 # Clang version information
 set(CLANG_EXECUTABLE_VERSION
- "${CLANG_VERSION_MAJOR}.${CLANG_VERSION_MINOR}" CACHE STRING
-"Version number that will be placed into the clang executable, in the form 
XX.YY")
+"${CLANG_VERSION_MAJOR}" CACHE STRING
+"Major version number that will be appended to the clang executable name")
 set(LIBCLANG_LIBRARY_VERSION
- "${CLANG_VERSION_MAJOR}.${CLANG_VERSION_MINOR}" CACHE STRING
-"Version number that will be placed into the libclang library , in the 
form XX.YY")
+"${CLANG_VERSION_MAJOR}" CACHE STRING
+"Major version number that will be appended to the libclang library")
 mark_as_advanced(CLANG_EXECUTABLE_VERSION LIBCLANG_LIBRARY_VERSION)
 
 option(CLANG_INCLUDE_TESTS

Modified: cfe/trunk/docs/ReleaseNotes.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ReleaseNotes.rst?rev=328769&r1=328768&r2=328769&view=diff
==
--- cfe/trunk/docs/ReleaseNotes.rst (original)
+++ cfe/trunk/docs/ReleaseNotes.rst Thu Mar 29 03:05:46 2018
@@ -62,6 +62,9 @@ Improvements to Clang's diagnostics
 Non-comprehensive list of changes in this release
 -
 
+- clang binary and libraries have been renamed from 7.0 to 7.
+  For example, the clang binary will be called clang-7 instead of clang-7.0.
+
 - ...
 
 New Compiler Flags


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

[PATCH] D41808: Rename clang link from clang-X.Y to clang-X

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC328769: Rename clang link from clang-X.Y to clang-X 
(authored by sylvestre, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D41808?vs=140202&id=140203#toc

Repository:
  rC Clang

https://reviews.llvm.org/D41808

Files:
  CMakeLists.txt
  docs/ReleaseNotes.rst


Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -62,6 +62,9 @@
 Non-comprehensive list of changes in this release
 -
 
+- clang binary and libraries have been renamed from 7.0 to 7.
+  For example, the clang binary will be called clang-7 instead of clang-7.0.
+
 - ...
 
 New Compiler Flags
Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -424,11 +424,11 @@
 
 # Clang version information
 set(CLANG_EXECUTABLE_VERSION
- "${CLANG_VERSION_MAJOR}.${CLANG_VERSION_MINOR}" CACHE STRING
-"Version number that will be placed into the clang executable, in the form 
XX.YY")
+"${CLANG_VERSION_MAJOR}" CACHE STRING
+"Major version number that will be appended to the clang executable name")
 set(LIBCLANG_LIBRARY_VERSION
- "${CLANG_VERSION_MAJOR}.${CLANG_VERSION_MINOR}" CACHE STRING
-"Version number that will be placed into the libclang library , in the 
form XX.YY")
+"${CLANG_VERSION_MAJOR}" CACHE STRING
+"Major version number that will be appended to the libclang library")
 mark_as_advanced(CLANG_EXECUTABLE_VERSION LIBCLANG_LIBRARY_VERSION)
 
 option(CLANG_INCLUDE_TESTS


Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -62,6 +62,9 @@
 Non-comprehensive list of changes in this release
 -
 
+- clang binary and libraries have been renamed from 7.0 to 7.
+  For example, the clang binary will be called clang-7 instead of clang-7.0.
+
 - ...
 
 New Compiler Flags
Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -424,11 +424,11 @@
 
 # Clang version information
 set(CLANG_EXECUTABLE_VERSION
- "${CLANG_VERSION_MAJOR}.${CLANG_VERSION_MINOR}" CACHE STRING
-"Version number that will be placed into the clang executable, in the form XX.YY")
+"${CLANG_VERSION_MAJOR}" CACHE STRING
+"Major version number that will be appended to the clang executable name")
 set(LIBCLANG_LIBRARY_VERSION
- "${CLANG_VERSION_MAJOR}.${CLANG_VERSION_MINOR}" CACHE STRING
-"Version number that will be placed into the libclang library , in the form XX.YY")
+"${CLANG_VERSION_MAJOR}" CACHE STRING
+"Major version number that will be appended to the libclang library")
 mark_as_advanced(CLANG_EXECUTABLE_VERSION LIBCLANG_LIBRARY_VERSION)
 
 option(CLANG_INCLUDE_TESTS
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44970: [XRay][clang] Add flag to choose instrumentation bundles

[Martin Pelikán via Phabricator via cfe-commits]

pelikan added a comment.

I would probably add more tests for the different configurations, but I suspect 
more diffs are coming after this.




Comment at: clang/include/clang/Driver/Options.td:1112
+  Group, Flags<[CC1Option]>,
+  HelpText<"Select which bundle of XRay instrumentation points to emit. 
Options: all, none, function-extents, custom-only.">;
+

I'd suggest making them "fn-only" and "custom-only", or just plain "function" 
and "custom".



Comment at: clang/include/clang/Driver/Options.td:1112
+  Group, Flags<[CC1Option]>,
+  HelpText<"Select which bundle of XRay instrumentation points to emit. 
Options: all, none, function-extents, custom-only.">;
+

pelikan wrote:
> I'd suggest making them "fn-only" and "custom-only", or just plain "function" 
> and "custom".
I also don't get why "none" is an option here, if it's equivalent to 
-fnoxray-instrument.  Why duplicate the functionality and add more points the 
users will have to debug?



Comment at: clang/include/clang/Frontend/CodeGenOptions.h:110
 
+  enum XRayInstrumentationPointBundle {
+XRay_All, // Always emit all the instrumentation points.

To me, this feels like a bitfield would be a better match.
All = Function | Custom
None = 0



Comment at: clang/lib/Driver/XRayArgs.cpp:62
+  << (std::string(XRayInstrumentOption) +
+  " on non-supported target OS");
 }

I would also print the triple.  Something like:

"-fomit-bugs is not supported on target win64-ppc-none"

will be much more informative, especially when you collect logs from build 
machines on lots of architectures (like Linux distro/BSD package builders do).



Comment at: clang/lib/Driver/XRayArgs.cpp:86
+Args.getLastArg(options::OPT_fxray_instrumentation_bundle)) {
+  StringRef B = A->getValue();
+  if (B != "all" && B != "none" && B != "function-extents" &&

echristo wrote:
> How about a more descriptive name here and a string switch below?
+1


https://reviews.llvm.org/D44970



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

[PATCH] D43341: [clang-doc] Implement reducer portion of the frontend framework

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: clang-doc/BitcodeWriter.h:129
 // Check that the static size is large-enough.
 assert(Record.capacity() > BitCodeConstants::RecordSize);
   }

lebedev.ri wrote:
> juliehockett wrote:
> > lebedev.ri wrote:
> > > Isn't that the opposite of what was that assert supposed to do?
> > > It would have been better to just `// disable` it, and add a `FIXME` note.
> > I'm not sure I'm understanding you -- my understanding was that it existed 
> > to check that the record size was large enough.
> https://reviews.llvm.org/D41102?id=136520#inline-384087
> ```
> #ifndef NDEBUG // Don't want explicit dtor unless needed
> ~ClangDocBitcodeWriter() {
>   // Check that the static size is large-enough.
>   assert(Record.capacity() == BitCodeConstants::RecordSize);
> }
> #endif
> ```
> I.e. it checked that it still only had the static size, and did not transform 
> into normal `vector` with data stored on heap-allocated buffer.
Ok, let me use more words this time.
There are several storage container types with contiguous storage:
* `array` - fixed-size stack storage, size == capacity == compile-time constant
* `vector` - dynamic heap storage, size <= capacity
* `smallvector` - fixed-size stack storage, and if the data does not fit, it 
degrades into the `vector`.

But there is also one more option:
* `fixedvector` - which is a `array` - has only fixed-size stack storage 
(capacity == compile-time constant), but with vector-like interface, i.e. size 
<= capacity, and size can be changed at run-time.

In llvm, i'm not aware of existence of `llvm::fixedvector`, so that assert was 
ensuring that the `smallvector` behaved like the `fixedvector` - it only had 
the fixed-size stack storage, and 'did not have' the heap buffer.

This is a kinda ugly hack, but i think the current assert clearly does 
something different...

Does that make any sense?



Comment at: clang-doc/Representation.cpp:28
+  if (L.Namespace.empty())
+L.Namespace = std::move(R.Namespace);
+  std::move(R.Description.begin(), R.Description.end(),

This was changed, but no test changes followed.
Needs more tests. The test coverage is clearly bad.


https://reviews.llvm.org/D43341



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

[PATCH] D45002: [test] Conservatively re-enable a FreeBSD/XRay test

[David CARLIER via Phabricator via cfe-commits]

devnexen added a comment.

Looks ok to me except I would change a bit the title maybe.


Repository:
  rC Clang

https://reviews.llvm.org/D45002



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

[PATCH] D45004: [clang-format] New style option IndentWrappedObjCMethodNames

[Daniel Jasper via Phabricator via cfe-commits]

djasper added a comment.

Do we have a public style guide that explicitly says to do this?
That's a requirement for new style options as per 
https://clang.llvm.org/docs/ClangFormatStyleOptions.html#adding-additional-style-options.

Also, are we sure that somebody wants the other behavior? Does that make sense 
for ObjC? I'd like to avoid options that nobody actually sets to a different 
value.


Repository:
  rC Clang

https://reviews.llvm.org/D45004



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

[PATCH] D44994: [clang-format] Ensure wrapped ObjC selectors with 1 arg obey IndentWrappedFunctionNames

[Daniel Jasper via Phabricator via cfe-commits]

djasper added inline comments.



Comment at: lib/Format/ContinuationIndenter.cpp:899
 if (!State.Stack.back().ObjCSelectorNameFound) {
+  unsigned MinIndent =
+  (Style.IndentWrappedFunctionNames

I think I'd now find this slightly easier to read as:

  unsigned MinIndent = State.Stack.back().Indent;
  if (Style.IndentWrappedFunctionNames)
MinIndent = std::max(MinIndent, State.FirstIndent + 
Style.ContinuationIndentWidth);



Comment at: lib/Format/ContinuationIndenter.cpp:904
+   : State.Stack.back().Indent);
   if (NextNonComment->LongestObjCSelectorName == 0)
+return MinIndent;

Does this if actually change the behavior in any way? With 
LongestObjCSelectorName being 0, isn't that:

  return MinIndent +
 std::max(0, ColumnWidth) - ColumnWidth;

(and with ColumnWidth being >= 0, this should be just MinIndent)


Repository:
  rC Clang

https://reviews.llvm.org/D44994



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

r328776 - [Hexagon] Aid bit-reverse load intrinsics lowering with bitcode

[Krzysztof Parzyszek via cfe-commits]

Author: kparzysz
Date: Thu Mar 29 06:54:31 2018
New Revision: 328776

URL: http://llvm.org/viewvc/llvm-project?rev=328776&view=rev
Log:
[Hexagon] Aid bit-reverse load intrinsics lowering with bitcode

The conversion of operatios to bitcode helps to eliminate an additional
store in certain cases. We used to lower these load intrinsics in DAG to
DAG conversion by which time, the "Dead Store Elimination" pass is
already run. There is an associated LLVM patch.

Patch by Sumanth Gundapaneni.

Added:
cfe/trunk/test/CodeGen/hexagon-brev-ld-ptr-incdec.c
cfe/trunk/test/CodeGen/hexagon-brev-store-elm.c
Modified:
cfe/trunk/include/clang/Basic/BuiltinsHexagon.def
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/test/CodeGen/builtins-hexagon.c

Modified: cfe/trunk/include/clang/Basic/BuiltinsHexagon.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsHexagon.def?rev=328776&r1=328775&r2=328776&view=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsHexagon.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsHexagon.def Thu Mar 29 06:54:31 2018
@@ -17,23 +17,23 @@
 // The builtins below are not autogenerated from iset.py.
 // Make sure you do not overwrite these.
 
-BUILTIN(__builtin_brev_ldd,   "LLi*LLi*LLi*i", "")
-BUILTIN(__builtin_brev_ldw,   "i*i*i*i", "")
-BUILTIN(__builtin_brev_ldh,   "s*s*s*i", "")
-BUILTIN(__builtin_brev_lduh,  "Us*Us*Us*i", "")
-BUILTIN(__builtin_brev_ldb,   "c*c*c*i", "")
-BUILTIN(__builtin_brev_ldub,  "Uc*Uc*Uc*i", "")
+BUILTIN(__builtin_brev_ldd,   "v*LLi*CLLi*iC", "")
+BUILTIN(__builtin_brev_ldw,   "v*i*Ci*iC", "")
+BUILTIN(__builtin_brev_ldh,   "v*s*Cs*iC", "")
+BUILTIN(__builtin_brev_lduh,  "v*Us*CUs*iC", "")
+BUILTIN(__builtin_brev_ldb,   "v*Sc*CSc*iC", "")
+BUILTIN(__builtin_brev_ldub,  "v*Uc*CUc*iC", "")
 BUILTIN(__builtin_circ_ldd,   "LLi*LLi*LLi*iIi", "")
 BUILTIN(__builtin_circ_ldw,   "i*i*i*iIi", "")
 BUILTIN(__builtin_circ_ldh,   "s*s*s*iIi", "")
 BUILTIN(__builtin_circ_lduh,  "Us*Us*Us*iIi", "")
 BUILTIN(__builtin_circ_ldb,   "c*c*c*iIi", "")
 BUILTIN(__builtin_circ_ldub,  "Uc*Uc*Uc*iIi", "")
-BUILTIN(__builtin_brev_std,   "LLi*LLi*LLii", "")
-BUILTIN(__builtin_brev_stw,   "i*i*ii", "")
-BUILTIN(__builtin_brev_sth,   "s*s*ii", "")
-BUILTIN(__builtin_brev_sthhi, "s*s*ii", "")
-BUILTIN(__builtin_brev_stb,   "c*c*ii", "")
+BUILTIN(__builtin_brev_std,   "LLi*CLLi*LLiiC", "")
+BUILTIN(__builtin_brev_stw,   "i*Ci*iiC", "")
+BUILTIN(__builtin_brev_sth,   "s*Cs*iiC", "")
+BUILTIN(__builtin_brev_sthhi, "s*Cs*iiC", "")
+BUILTIN(__builtin_brev_stb,   "c*Cc*iiC", "")
 BUILTIN(__builtin_circ_std,   "LLi*LLi*LLiiIi", "")
 BUILTIN(__builtin_circ_stw,   "i*i*iiIi", "")
 BUILTIN(__builtin_circ_sth,   "s*s*iiIi", "")

Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=328776&r1=328775&r2=328776&view=diff
==
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Thu Mar 29 06:54:31 2018
@@ -10820,6 +10820,45 @@ Value *CodeGenFunction::EmitHexagonBuilt
 return Builder.CreateAlignedStore(NewBase, LV, Dest.getAlignment());
   };
 
+  // Handle the conversion of bit-reverse load intrinsics to bit code.
+  // The intrinsic call after this function only reads from memory and the
+  // write to memory is dealt by the store instruction.
+  auto MakeBrevLd = [&](unsigned IntID, llvm::Type *DestTy) {
+// The intrinsic generates one result, which is the new value for the base
+// pointer. It needs to be returned. The result of the load instruction is
+// passed to intrinsic by address, so the value needs to be stored.
+llvm::Value *BaseAddress =
+Builder.CreateBitCast(EmitScalarExpr(E->getArg(0)), Int8PtrTy);
+
+// Expressions like &(*pt++) will be incremented per evaluation.
+// EmitPointerWithAlignment and EmitScalarExpr evaluates the expression
+// per call.
+Address DestAddr = EmitPointerWithAlignment(E->getArg(1));
+DestAddr = Address(Builder.CreateBitCast(DestAddr.getPointer(), Int8PtrTy),
+   DestAddr.getAlignment());
+llvm::Value *DestAddress = DestAddr.getPointer();
+
+// Operands are Base, Dest, Modifier.
+// The intrinsic format in LLVM IR is defined as
+// { ValueType, i8* } (i8*, i32).
+Ops = {BaseAddress, EmitScalarExpr(E->getArg(2))};
+
+llvm::Value *Result = Builder.CreateCall(CGM.getIntrinsic(IntID), Ops);
+// The value needs to be stored as the variable is passed by reference.
+llvm::Value *DestVal = Builder.CreateExtractValue(Result, 0);
+
+// The store needs to be truncated to fit the destination type.
+// While i32 and i64 are natively supported on Hexagon, i8 and i16 needs
+// to be handled with stores of respective destination type.
+DestVal = Builder.Creat

[libcxx] r328782 - Creating release candidate rc2 from release_502 branch

[Tom Stellard via cfe-commits]

Author: tstellar
Date: Thu Mar 29 07:03:58 2018
New Revision: 328782

URL: http://llvm.org/viewvc/llvm-project?rev=328782&view=rev
Log:
Creating release candidate rc2 from release_502 branch

Added:
libcxx/tags/RELEASE_502/rc2/   (props changed)
  - copied from r328781, libcxx/branches/release_50/

Propchange: libcxx/tags/RELEASE_502/rc2/
--
--- svn:mergeinfo (added)
+++ svn:mergeinfo Thu Mar 29 07:03:58 2018
@@ -0,0 +1,2 @@
+/libcxx/branches/apple:136569-137939
+/libcxx/trunk:309296,309307,309474,309838,309851,309917,309920


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

[libcxxabi] r328783 - Creating release candidate rc2 from release_502 branch

[Tom Stellard via cfe-commits]

Author: tstellar
Date: Thu Mar 29 07:04:02 2018
New Revision: 328783

URL: http://llvm.org/viewvc/llvm-project?rev=328783&view=rev
Log:
Creating release candidate rc2 from release_502 branch

Added:
libcxxabi/tags/RELEASE_502/rc2/
  - copied from r328782, libcxxabi/branches/release_50/

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

[libunwind] r328789 - Creating release candidate rc2 from release_502 branch

[Tom Stellard via cfe-commits]

Author: tstellar
Date: Thu Mar 29 07:04:30 2018
New Revision: 328789

URL: http://llvm.org/viewvc/llvm-project?rev=328789&view=rev
Log:
Creating release candidate rc2 from release_502 branch

Added:
libunwind/tags/RELEASE_502/rc2/   (props changed)
  - copied from r328788, libunwind/branches/release_50/

Propchange: libunwind/tags/RELEASE_502/rc2/
--
svn:mergeinfo = /libunwind/trunk:308871,309147


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

[PATCH] D44786: Lowering x86 adds/addus/subs/subus intrinsics (clang)

[Tomasz Krupa via Phabricator via cfe-commits]

tkrupa marked 3 inline comments as done.
tkrupa added inline comments.



Comment at: lib/CodeGen/CGBuiltin.cpp:7897
+  Value *MaxVec = CGF.Builder.CreateVectorSplat(NumElements, Max);
+  Value *ExtMaxVec = Signed ? CGF.Builder.CreateSExt(MaxVec, ExtType)
+: CGF.Builder.CreateZExt(MaxVec, ExtType);

craig.topper wrote:
> Can we just create the constant in the extended type?
We can't - SignedMaxValue is created in non-extended type and 
llvm::ConstantInt::get makes an assertion that the input is of the same type. 
We can perform sext() on SignedMaxValue but the result will be the same.


Repository:
  rC Clang

https://reviews.llvm.org/D44786



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

[PATCH] D44786: Lowering x86 adds/addus/subs/subus intrinsics (clang)

[Tomasz Krupa via Phabricator via cfe-commits]

tkrupa updated this revision to Diff 140244.

Repository:
  rC Clang

https://reviews.llvm.org/D44786

Files:
  lib/CodeGen/CGBuiltin.cpp
  test/CodeGen/avx2-builtins.c
  test/CodeGen/avx512bw-builtins.c
  test/CodeGen/avx512vlbw-builtins.c
  test/CodeGen/sse2-builtins.c

Index: test/CodeGen/sse2-builtins.c
===
--- test/CodeGen/sse2-builtins.c
+++ test/CodeGen/sse2-builtins.c
@@ -47,25 +47,53 @@
 
 __m128i test_mm_adds_epi8(__m128i A, __m128i B) {
   // CHECK-LABEL: test_mm_adds_epi8
-  // CHECK: call <16 x i8> @llvm.x86.sse2.padds.b(<16 x i8> %{{.*}}, <16 x i8> %{{.*}})
+  // CHECK-NOT: call <16 x i8> @llvm.x86.sse2.padds.b(<16 x i8> %{{.*}}, <16 x i8> %{{.*}})
+  // CHECK: sext <16 x i8> %{{.*}} to <16 x i16>
+  // CHECK: sext <16 x i8> %{{.*}} to <16 x i16>
+  // CHECK: add <16 x i16> %{{.*}}, %{{.*}}
+  // CHECK: icmp sle <16 x i16> %{{.*}}, 
+  // CHECK: select <16 x i1> %{{.*}}, <16 x i16> %{{.*}}, <16 x i16> 
+  // CHECK: icmp slt <16 x i16> %{{.*}}, 
+  // CHECK: select <16 x i1> %{{.*}}, <16 x i16> , <16 x i16> %{{.*}}
+  // CHECK: trunc <16 x i16> %{{.*}} to <16 x i8>
   return _mm_adds_epi8(A, B);
 }
 
 __m128i test_mm_adds_epi16(__m128i A, __m128i B) {
   // CHECK-LABEL: test_mm_adds_epi16
-  // CHECK: call <8 x i16> @llvm.x86.sse2.padds.w(<8 x i16> %{{.*}}, <8 x i16> %{{.*}})
+  // CHECK-NOT: call <8 x i16> @llvm.x86.sse2.padds.w(<8 x i16> %{{.*}}, <8 x i16> %{{.*}})
+  // CHECK: sext <8 x i16> %{{.*}} to <8 x i32>
+  // CHECK: sext <8 x i16> %{{.*}} to <8 x i32>
+  // CHECK: add <8 x i32> %{{.*}}, %{{.*}}
+  // CHECK: icmp sle <8 x i32> %{{.*}}, 
+  // CHECK: select <8 x i1> %{{.*}}, <8 x i32> %{{.*}}, <8 x i32> 
+  // CHECK: icmp slt <8 x i32> %{{.*}}, 
+  // CHECK: select <8 x i1> %{{.*}}, <8 x i32> , <8 x i32> %{{.*}}
+  // CHECK: trunc <8 x i32> %{{.*}} to <8 x i16>
   return _mm_adds_epi16(A, B);
 }
 
 __m128i test_mm_adds_epu8(__m128i A, __m128i B) {
   // CHECK-LABEL: test_mm_adds_epu8
-  // CHECK: call <16 x i8> @llvm.x86.sse2.paddus.b(<16 x i8> %{{.*}}, <16 x i8> %{{.*}})
+  // CHECK-NOT: call <16 x i8> @llvm.x86.sse2.paddus.b(<16 x i8> %{{.*}}, <16 x i8> %{{.*}})
+  // CHECK: zext <16 x i8> %{{.*}} to <16 x i16>
+  // CHECK: zext <16 x i8> %{{.*}} to <16 x i16>
+  // CHECK: add <16 x i16> %{{.*}}, %{{.*}}
+  // CHECK: icmp ule <16 x i16> %{{.*}}, 
+  // CHECK: select <16 x i1> %{{.*}}, <16 x i16> %{{.*}}, <16 x i16> 
+  // CHECK: trunc <16 x i16> %{{.*}} to <16 x i8>
   return _mm_adds_epu8(A, B);
 }
 
 __m128i test_mm_adds_epu16(__m128i A, __m128i B) {
   // CHECK-LABEL: test_mm_adds_epu16
-  // CHECK: call <8 x i16> @llvm.x86.sse2.paddus.w(<8 x i16> %{{.*}}, <8 x i16> %{{.*}})
+  // CHECK-NOT: call <8 x i16> @llvm.x86.sse2.paddus.w(<8 x i16> %{{.*}}, <8 x i16> %{{.*}})
+  // CHECK: zext <8 x i16> %{{.*}} to <8 x i32>
+  // CHECK: zext <8 x i16> %{{.*}} to <8 x i32>
+  // CHECK: add <8 x i32> %{{.*}}, %{{.*}}
+  // CHECK: icmp ule <8 x i32> %{{.*}}, 
+  // CHECK: select <8 x i1> %{{.*}}, <8 x i32> %{{.*}}, <8 x i32> 
+  // CHECK: trunc <8 x i32> %{{.*}} to <8 x i16>
   return _mm_adds_epu16(A, B);
 }
 
@@ -1414,25 +1442,47 @@
 
 __m128i test_mm_subs_epi8(__m128i A, __m128i B) {
   // CHECK-LABEL: test_mm_subs_epi8
-  // CHECK: call <16 x i8> @llvm.x86.sse2.psubs.b(<16 x i8> %{{.*}}, <16 x i8> %{{.*}})
+  // CHECK-NOT: call <16 x i8> @llvm.x86.sse2.psubs.b(<16 x i8> %{{.*}}, <16 x i8> %{{.*}})
+  // CHECK: sext <16 x i8> %{{.*}} to <16 x i16>
+  // CHECK: sext <16 x i8> %{{.*}} to <16 x i16>
+  // CHECK: sub <16 x i16> %{{.*}}, %{{.*}}
+  // CHECK: icmp sle <16 x i16> %{{.*}}, 
+  // CHECK: select <16 x i1> %{{.*}}, <16 x i16> %{{.*}}, <16 x i16> 
+  // CHECK: icmp slt <16 x i16> %{{.*}}, 
+  // CHECK: select <16 x i1> %{{.*}}, <16 x i16> , <16 x i16> %{{.*}}
+  // CHECK: trunc <16 x i16> %{{.*}} to <16 x i8>
   return _mm_subs_epi8(A, B);
 }
 
 __m128i test_mm_subs_epi16(__m128i A, __m128i B) {
   // CHECK-LABEL: test_mm_subs_epi16
-  // CHECK: call <8 x i16> @llvm.x86.sse2.psubs.w(<8 x i16> %{{.*}}, <8 x i16> %{{.*}})
+  // CHECK-NOT: call <8 x i16> @llvm.x86.sse2.psubs.w(<8 x i16> %{{.*}}, <8 x i16> %{{.*}})
+  // CHECK: sext <8 x i16> %{{.*}} to <8 x i32>
+  // CHECK: sext <8 x i16> %{{.*}} to <8 x i32>
+  // CHECK: sub <8 x i32> %{{.*}}, %{{.*}}
+  // CHECK: icmp sle <8 x i32> %{{.*}}, 
+  // CHECK: select <8 x i1> %{{.*}}, <8 x i32> %{{.*}}, <8 x i32> 
+  // CHECK: icmp slt <8 x i32> %{{.*}},  
+  // CHECK: select <8 x i1> %{{.*}}, <8 x i32>  , <8 x i32> %{{.*}}
+  // CHECK: trunc <8 x i32> %{{.*}} to <8 x i16>
   return _mm_subs_epi16(A, B);
 }
 
 __m128i test_mm_subs_epu8(__m128i A, __m128i B) {
   // CHECK-LABEL: test_mm_subs_epu8
-  // CHECK: call <16 x i8> @llvm.x86.sse2.psubus.b(<16 x i8> %{{.*}}, <16 x i8> %{{.*}})
+  // CHECK-NOT: call <16 x i8> @llvm.x86.sse2.psubus.b(<16 x i8> %{{.*}}, <16 x i8> %{{.*}})
+  // CHECK: icmp ugt <16 x i8> {{.*}}, {{.*}}
+  // CHECK: select <16 x i1> {{.*}}, <16 x i8> {{.*}}

r328791 - Fix typo

[Matt Arsenault via cfe-commits]

Author: arsenm
Date: Thu Mar 29 07:31:59 2018
New Revision: 328791

URL: http://llvm.org/viewvc/llvm-project?rev=328791&view=rev
Log:
Fix typo

Modified:
cfe/trunk/include/clang/Basic/AttrDocs.td
cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp

Modified: cfe/trunk/include/clang/Basic/AttrDocs.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/AttrDocs.td?rev=328791&r1=328790&r2=328791&view=diff
==
--- cfe/trunk/include/clang/Basic/AttrDocs.td (original)
+++ cfe/trunk/include/clang/Basic/AttrDocs.td Thu Mar 29 07:31:59 2018
@@ -273,7 +273,7 @@ def AllocSizeDocs : Documentation {
   let Content = [{
 The ``alloc_size`` attribute can be placed on functions that return pointers in
 order to hint to the compiler how many bytes of memory will be available at the
-returned poiner. ``alloc_size`` takes one or two arguments.
+returned pointer. ``alloc_size`` takes one or two arguments.
 
 - ``alloc_size(N)`` implies that argument number N equals the number of
   available bytes at the returned pointer.
@@ -311,7 +311,7 @@ def AllocAlignDocs : Documentation {
   let Content = [{
 Use ``__attribute__((alloc_align())`` on a function
 declaration to specify that the return value of the function (which must be a
-pointer type) is at least as aligned as the value of the indicated parameter. 
The 
+pointer type) is at least as aligned as the value of the indicated parameter. 
The
 parameter is given by its index in the list of formal parameters; the first
 parameter has index 1 unless the function is a C++ non-static member function,
 in which case the first parameter has index 2 to account for the implicit 
``this``
@@ -330,7 +330,7 @@ parameter.
   void *Foo::b(void *v, size_t align) __attribute__((alloc_align(3)));
 
 Note that this attribute merely informs the compiler that a function always
-returns a sufficiently aligned pointer. It does not cause the compiler to 
+returns a sufficiently aligned pointer. It does not cause the compiler to
 emit code to enforce that alignment.  The behavior is undefined if the returned
 poitner is not sufficiently aligned.
   }];
@@ -913,10 +913,10 @@ in the metadata name for that object. Th
 attribute allows annotated interfaces or protocols to use the
 specified string argument in the object's metadata name instead of the
 default name.
-
+
 **Usage**: ``__attribute__((objc_runtime_name("MyLocalName")))``.  This 
attribute
 can only be placed before an @protocol or @interface declaration:
-
+
 .. code-block:: objc
 
   __attribute__((objc_runtime_name("MyLocalName")))
@@ -2626,7 +2626,7 @@ The ``_Nullable`` nullability qualifier
 
 int fetch_or_zero(int * _Nullable ptr);
 
-a caller of ``fetch_or_zero`` can provide null. 
+a caller of ``fetch_or_zero`` can provide null.
   }];
 }
 
@@ -2861,7 +2861,7 @@ Use this attribute to indicate that the
 caller-saved registers. That is, all registers are callee-saved except for
 registers used for passing parameters to the function or returning parameters
 from the function.
-The compiler saves and restores any modified registers that were not used for 
+The compiler saves and restores any modified registers that were not used for
 passing or returning arguments to the function.
 
 The user can call functions specified with the 'no_caller_saved_registers'
@@ -3298,7 +3298,7 @@ def ArtificialDocs : Documentation {
   let Content = [{
 The ``artificial`` attribute can be applied to an inline function. If such a
 function is inlined, the attribute indicates that debuggers should associate
-the resulting instructions with the call site, rather than with the 
+the resulting instructions with the call site, rather than with the
 corresponding line within the inlined callee.
   }];
 }

Modified: cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp?rev=328791&r1=328790&r2=328791&view=diff
==
--- cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp Thu Mar 29 07:31:59 2018
@@ -206,7 +206,7 @@ void bad_downcast_pointer(S *p) {
   // CHECK: %[[NONNULL:.*]] = icmp ne {{.*}}, null
   // CHECK: br i1 %[[NONNULL]],
 
-  // A null poiner access is guarded without -fsanitize=null.
+  // A null pointer access is guarded without -fsanitize=null.
   // DOWNCAST-NULL: %[[NONNULL:.*]] = icmp ne {{.*}}, null
   // DOWNCAST-NULL: br i1 %[[NONNULL]],
 


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

[PATCH] D45044: [clangd] Mark "Source Hover" as implemented in the docs

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

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

Signed-off-by: Marc-Andre Laperle 


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45044

Files:
  docs/clangd.rst


Index: docs/clangd.rst
===
--- docs/clangd.rst
+++ docs/clangd.rst
@@ -75,7 +75,7 @@
 +-++--+
 | Rename  | Yes|   Yes|
 +-++--+
-| Source hover| Yes|   No |
+| Source hover| Yes|   Yes|
 +-++--+
 | Find References | Yes|   No |
 +-++--+


Index: docs/clangd.rst
===
--- docs/clangd.rst
+++ docs/clangd.rst
@@ -75,7 +75,7 @@
 +-++--+
 | Rename  | Yes|   Yes|
 +-++--+
-| Source hover| Yes|   No |
+| Source hover| Yes|   Yes|
 +-++--+
 | Find References | Yes|   No |
 +-++--+
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45044: [clangd] Mark "Source Hover" as implemented in the docs

[Simon Marchi via Phabricator via cfe-commits]

simark added a comment.

Yay!


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45044



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

[PATCH] D44994: [clang-format] Ensure wrapped ObjC selectors with 1 arg obey IndentWrappedFunctionNames

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton updated this revision to Diff 140251.
benhamilton marked 2 inline comments as done.
benhamilton added a comment.

@djasper comments


Repository:
  rC Clang

https://reviews.llvm.org/D44994

Files:
  lib/Format/ContinuationIndenter.cpp
  unittests/Format/FormatTestObjC.cpp


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -537,6 +537,22 @@
"   aShortf:(NSRect)theRect {\n"
"}");
 
+  // Make sure selectors with 0, 1, or more arguments are indented
+  // when IndentWrappedFunctionNames is true.
+  verifyFormat("- (a)\n"
+   ";\n");
+  verifyFormat("- (a)\n"
+   ":(int)a;\n");
+  verifyFormat("- (a)\n"
+   ":(int)a\n"
+   ":(int)a;\n");
+  verifyFormat("- (a)\n"
+   " aaa:(int)a\n"
+   ":(int)a;\n");
+  verifyFormat("- (a)\n"
+   ":(int)a\n"
+   " aaa:(int)a;\n");
+
   // Format pairs correctly.
   Style.ColumnLimit = 80;
   verifyFormat("- (void)drawRectOn:(id)surface\n"
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -896,12 +896,19 @@
 return std::max(State.Stack.back().LastSpace, State.Stack.back().Indent);
   if (NextNonComment->is(TT_SelectorName)) {
 if (!State.Stack.back().ObjCSelectorNameFound) {
+  unsigned MinIndent = State.Stack.back().Indent;
+  if (Style.IndentWrappedFunctionNames)
+MinIndent = std::max(State.Stack.back().Indent,
+ State.FirstIndent + 
Style.ContinuationIndentWidth);
+  // If LongestObjCSelectorName is 0, we are indenting the first
+  // part of an ObjC selector (or a selector component which is
+  // not colon-aligned due to block formatting).
   if (NextNonComment->LongestObjCSelectorName == 0)
-return State.Stack.back().Indent;
-  return (Style.IndentWrappedFunctionNames
-  ? std::max(State.Stack.back().Indent,
- State.FirstIndent + Style.ContinuationIndentWidth)
-  : State.Stack.back().Indent) +
+return MinIndent;
+  // Otherwise, we are indenting a subsequent part of an ObjC
+  // selector which should be colon-aligned to the longest
+  // component of the ObjC selector.
+  return MinIndent +
  std::max(NextNonComment->LongestObjCSelectorName,
   NextNonComment->ColumnWidth) -
  NextNonComment->ColumnWidth;


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -537,6 +537,22 @@
"   aShortf:(NSRect)theRect {\n"
"}");
 
+  // Make sure selectors with 0, 1, or more arguments are indented
+  // when IndentWrappedFunctionNames is true.
+  verifyFormat("- (a)\n"
+   ";\n");
+  verifyFormat("- (a)\n"
+   ":(int)a;\n");
+  verifyFormat("- (a)\n"
+   ":(int)a\n"
+   ":(int)a;\n");
+  verifyFormat("- (a)\n"
+   " aaa:(int)a\n"
+   ":(int)a;\n");
+  verifyFormat("- (a)\n"
+   ":(int)a\n"
+   " aaa:(int)a;\n");
+
   // Format pairs correctly.
   Style.ColumnLimit = 80;
   verifyFormat("- (void)drawRectOn:(id)surface\n"
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -896,12 +896,19 @@
 return std::max(State.Stack.back().LastSpace, State.Stack.back().Indent);
   if (NextNonComment->is(TT_SelectorName)) {
 if (!State.Stack.back().ObjCSelectorNameFound) {
+  unsigned MinIndent = State.Stack.back().Indent;
+  if (Style.IndentWrappedFunctionNames)
+MinIndent = std::max(State.Stack.back().Indent,
+ State.FirstIndent + Style.Continuation

[PATCH] D44994: [clang-format] Ensure wrapped ObjC selectors with 1 arg obey IndentWrappedFunctionNames

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton added a comment.

Thanks, applied your suggested change.




Comment at: lib/Format/ContinuationIndenter.cpp:899
 if (!State.Stack.back().ObjCSelectorNameFound) {
+  unsigned MinIndent =
+  (Style.IndentWrappedFunctionNames

djasper wrote:
> I think I'd now find this slightly easier to read as:
> 
>   unsigned MinIndent = State.Stack.back().Indent;
>   if (Style.IndentWrappedFunctionNames)
> MinIndent = std::max(MinIndent, State.FirstIndent + 
> Style.ContinuationIndentWidth);
OK, done.



Comment at: lib/Format/ContinuationIndenter.cpp:904
+   : State.Stack.back().Indent);
   if (NextNonComment->LongestObjCSelectorName == 0)
+return MinIndent;

djasper wrote:
> Does this if actually change the behavior in any way? With 
> LongestObjCSelectorName being 0, isn't that:
> 
>   return MinIndent +
>  std::max(0, ColumnWidth) - ColumnWidth;
> 
> (and with ColumnWidth being >= 0, this should be just MinIndent)
The `- ColumnWidth` part is only for the case where `LongestObjCSelectorName` 
is *not* 0. If it's 0, we return `MinIndent` which ensures we obey 
`Style.IndentWrappedFunctionNames`.

The problem with the code before this diff is when `LongestObjCSelectorName` 
was 0, we ignored `Style.IndentWrappedFunctionNames` and always returned 
`State.Stack.back().Indent` regardless of that setting.

After this diff, when `LongestObjCSelectorName` is 0 (i.e., this is either the 
first part of the selector or a selector which is not colon-aligned due to 
block formatting), we change the behavior to indent to at least 
`State.FirstIndent + Style.ContinuationIndentWidth`, like all other indentation 
logic in this file.

I've added some comments explaining what's going on, since this code is quite 
complex.


Repository:
  rC Clang

https://reviews.llvm.org/D44994



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

[PATCH] D44994: [clang-format] Ensure wrapped ObjC selectors with 1 arg obey IndentWrappedFunctionNames

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton updated this revision to Diff 140252.
benhamilton added a comment.

One more tidy-up


Repository:
  rC Clang

https://reviews.llvm.org/D44994

Files:
  lib/Format/ContinuationIndenter.cpp
  unittests/Format/FormatTestObjC.cpp


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -537,6 +537,22 @@
"   aShortf:(NSRect)theRect {\n"
"}");
 
+  // Make sure selectors with 0, 1, or more arguments are indented
+  // when IndentWrappedFunctionNames is true.
+  verifyFormat("- (a)\n"
+   ";\n");
+  verifyFormat("- (a)\n"
+   ":(int)a;\n");
+  verifyFormat("- (a)\n"
+   ":(int)a\n"
+   ":(int)a;\n");
+  verifyFormat("- (a)\n"
+   " aaa:(int)a\n"
+   ":(int)a;\n");
+  verifyFormat("- (a)\n"
+   ":(int)a\n"
+   " aaa:(int)a;\n");
+
   // Format pairs correctly.
   Style.ColumnLimit = 80;
   verifyFormat("- (void)drawRectOn:(id)surface\n"
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -896,12 +896,19 @@
 return std::max(State.Stack.back().LastSpace, State.Stack.back().Indent);
   if (NextNonComment->is(TT_SelectorName)) {
 if (!State.Stack.back().ObjCSelectorNameFound) {
+  unsigned MinIndent = State.Stack.back().Indent;
+  if (Style.IndentWrappedFunctionNames)
+MinIndent = std::max(MinIndent,
+ State.FirstIndent + 
Style.ContinuationIndentWidth);
+  // If LongestObjCSelectorName is 0, we are indenting the first
+  // part of an ObjC selector (or a selector component which is
+  // not colon-aligned due to block formatting).
   if (NextNonComment->LongestObjCSelectorName == 0)
-return State.Stack.back().Indent;
-  return (Style.IndentWrappedFunctionNames
-  ? std::max(State.Stack.back().Indent,
- State.FirstIndent + Style.ContinuationIndentWidth)
-  : State.Stack.back().Indent) +
+return MinIndent;
+  // Otherwise, we are indenting a subsequent part of an ObjC
+  // selector which should be colon-aligned to the longest
+  // component of the ObjC selector.
+  return MinIndent +
  std::max(NextNonComment->LongestObjCSelectorName,
   NextNonComment->ColumnWidth) -
  NextNonComment->ColumnWidth;


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -537,6 +537,22 @@
"   aShortf:(NSRect)theRect {\n"
"}");
 
+  // Make sure selectors with 0, 1, or more arguments are indented
+  // when IndentWrappedFunctionNames is true.
+  verifyFormat("- (a)\n"
+   ";\n");
+  verifyFormat("- (a)\n"
+   ":(int)a;\n");
+  verifyFormat("- (a)\n"
+   ":(int)a\n"
+   ":(int)a;\n");
+  verifyFormat("- (a)\n"
+   " aaa:(int)a\n"
+   ":(int)a;\n");
+  verifyFormat("- (a)\n"
+   ":(int)a\n"
+   " aaa:(int)a;\n");
+
   // Format pairs correctly.
   Style.ColumnLimit = 80;
   verifyFormat("- (void)drawRectOn:(id)surface\n"
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -896,12 +896,19 @@
 return std::max(State.Stack.back().LastSpace, State.Stack.back().Indent);
   if (NextNonComment->is(TT_SelectorName)) {
 if (!State.Stack.back().ObjCSelectorNameFound) {
+  unsigned MinIndent = State.Stack.back().Indent;
+  if (Style.IndentWrappedFunctionNames)
+MinIndent = std::max(MinIndent,
+ State.FirstIndent + Style.ContinuationIndentWidth);
+  // If LongestObjCSelectorName is 0, we are indenting the f

[clang-tools-extra] r328792 - [clangd] Mark "Source Hover" as implemented in the docs

[Marc-Andre Laperle via cfe-commits]

Author: malaperle
Date: Thu Mar 29 07:49:21 2018
New Revision: 328792

URL: http://llvm.org/viewvc/llvm-project?rev=328792&view=rev
Log:
[clangd] Mark "Source Hover" as implemented in the docs

Summary: Signed-off-by: Marc-Andre Laperle 

Reviewers: simark

Reviewed By: simark

Subscribers: klimek, ilya-biryukov, ioeric, MaskRay, cfe-commits

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

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

Modified: clang-tools-extra/trunk/docs/clangd.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clangd.rst?rev=328792&r1=328791&r2=328792&view=diff
==
--- clang-tools-extra/trunk/docs/clangd.rst (original)
+++ clang-tools-extra/trunk/docs/clangd.rst Thu Mar 29 07:49:21 2018
@@ -75,7 +75,7 @@ extension to the protocol.
 +-++--+
 | Rename  | Yes|   Yes|
 +-++--+
-| Source hover| Yes|   No |
+| Source hover| Yes|   Yes|
 +-++--+
 | Find References | Yes|   No |
 +-++--+


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

[PATCH] D45044: [clangd] Mark "Source Hover" as implemented in the docs

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rL328792: [clangd] Mark "Source Hover" as 
implemented in the docs (authored by malaperle, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D45044

Files:
  clang-tools-extra/trunk/docs/clangd.rst


Index: clang-tools-extra/trunk/docs/clangd.rst
===
--- clang-tools-extra/trunk/docs/clangd.rst
+++ clang-tools-extra/trunk/docs/clangd.rst
@@ -75,7 +75,7 @@
 +-++--+
 | Rename  | Yes|   Yes|
 +-++--+
-| Source hover| Yes|   No |
+| Source hover| Yes|   Yes|
 +-++--+
 | Find References | Yes|   No |
 +-++--+


Index: clang-tools-extra/trunk/docs/clangd.rst
===
--- clang-tools-extra/trunk/docs/clangd.rst
+++ clang-tools-extra/trunk/docs/clangd.rst
@@ -75,7 +75,7 @@
 +-++--+
 | Rename  | Yes|   Yes|
 +-++--+
-| Source hover| Yes|   No |
+| Source hover| Yes|   Yes|
 +-++--+
 | Find References | Yes|   No |
 +-++--+
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r328793 - Disable emitting static extern C aliases for amdgcn target for CUDA

[Yaxun Liu via cfe-commits]

Author: yaxunl
Date: Thu Mar 29 07:50:00 2018
New Revision: 328793

URL: http://llvm.org/viewvc/llvm-project?rev=328793&view=rev
Log:
Disable emitting static extern C aliases for amdgcn target for CUDA

Patch by Greg Rodgers.
Revised and lit test added by Yaxun Liu.

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

Modified:
cfe/trunk/lib/CodeGen/CodeGenModule.cpp
cfe/trunk/lib/CodeGen/TargetInfo.cpp
cfe/trunk/lib/CodeGen/TargetInfo.h
cfe/trunk/test/CodeGenCUDA/alias.cu

Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=328793&r1=328792&r2=328793&view=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Thu Mar 29 07:50:00 2018
@@ -4677,9 +4677,7 @@ static void EmitGlobalDeclMetadata(CodeG
 /// to such functions with an unmangled name from inline assembly within the
 /// same translation unit.
 void CodeGenModule::EmitStaticExternCAliases() {
-  // Don't do anything if we're generating CUDA device code -- the NVPTX
-  // assembly target doesn't support aliases.
-  if (Context.getTargetInfo().getTriple().isNVPTX())
+  if (!getTargetCodeGenInfo().shouldEmitStaticExternCAliases())
 return;
   for (auto &I : StaticExternCValues) {
 IdentifierInfo *Name = I.first;

Modified: cfe/trunk/lib/CodeGen/TargetInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/TargetInfo.cpp?rev=328793&r1=328792&r2=328793&view=diff
==
--- cfe/trunk/lib/CodeGen/TargetInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/TargetInfo.cpp Thu Mar 29 07:50:00 2018
@@ -6154,6 +6154,7 @@ public:
 
   void setTargetAttributes(const Decl *D, llvm::GlobalValue *GV,
CodeGen::CodeGenModule &M) const override;
+  bool shouldEmitStaticExternCAliases() const override;
 
 private:
   // Adds a NamedMDNode with F, Name, and Operand as operands, and adds the
@@ -6275,6 +6276,10 @@ void NVPTXTargetCodeGenInfo::addNVVMMeta
   // Append metadata to nvvm.annotations
   MD->addOperand(llvm::MDNode::get(Ctx, MDVals));
 }
+
+bool NVPTXTargetCodeGenInfo::shouldEmitStaticExternCAliases() const {
+  return false;
+}
 }
 
 
//===--===//
@@ -7646,6 +7651,7 @@ public:
   createEnqueuedBlockKernel(CodeGenFunction &CGF,
 llvm::Function *BlockInvokeFunc,
 llvm::Value *BlockLiteral) const override;
+  bool shouldEmitStaticExternCAliases() const override;
 };
 }
 
@@ -,6 +7783,10 @@ AMDGPUTargetCodeGenInfo::getLLVMSyncScop
   return C.getOrInsertSyncScopeID(Name);
 }
 
+bool AMDGPUTargetCodeGenInfo::shouldEmitStaticExternCAliases() const {
+  return false;
+}
+
 
//===--===//
 // SPARC v8 ABI Implementation.
 // Based on the SPARC Compliance Definition version 2.4.1.

Modified: cfe/trunk/lib/CodeGen/TargetInfo.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/TargetInfo.h?rev=328793&r1=328792&r2=328793&view=diff
==
--- cfe/trunk/lib/CodeGen/TargetInfo.h (original)
+++ cfe/trunk/lib/CodeGen/TargetInfo.h Thu Mar 29 07:50:00 2018
@@ -296,6 +296,11 @@ public:
   createEnqueuedBlockKernel(CodeGenFunction &CGF,
 llvm::Function *BlockInvokeFunc,
 llvm::Value *BlockLiteral) const;
+
+  /// \return true if the target supports alias from the unmangled name to the
+  /// mangled name of functions declared within an extern "C" region and marked
+  /// as 'used', and having internal linkage.
+  virtual bool shouldEmitStaticExternCAliases() const { return true; }
 };
 
 } // namespace CodeGen

Modified: cfe/trunk/test/CodeGenCUDA/alias.cu
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCUDA/alias.cu?rev=328793&r1=328792&r2=328793&view=diff
==
--- cfe/trunk/test/CodeGenCUDA/alias.cu (original)
+++ cfe/trunk/test/CodeGenCUDA/alias.cu Thu Mar 29 07:50:00 2018
@@ -1,8 +1,11 @@
 // REQUIRES: x86-registered-target
 // REQUIRES: nvptx-registered-target
+// REQUIRES: amdgpu-registered-target
 
 // RUN: %clang_cc1 -fcuda-is-device -triple nvptx-nvidia-cuda -emit-llvm \
 // RUN:   -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fcuda-is-device -triple amdgcn -emit-llvm \
+// RUN:   -o - %s | FileCheck %s
 
 #include "Inputs/cuda.h"
 


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

[PATCH] D44987: Disable emitting static extern C aliases for amdgcn target for CUDA

[Yaxun Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL328793: Disable emitting static extern C aliases for amdgcn 
target for CUDA (authored by yaxunl, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D44987?vs=140129&id=140254#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D44987

Files:
  cfe/trunk/lib/CodeGen/CodeGenModule.cpp
  cfe/trunk/lib/CodeGen/TargetInfo.cpp
  cfe/trunk/lib/CodeGen/TargetInfo.h
  cfe/trunk/test/CodeGenCUDA/alias.cu


Index: cfe/trunk/test/CodeGenCUDA/alias.cu
===
--- cfe/trunk/test/CodeGenCUDA/alias.cu
+++ cfe/trunk/test/CodeGenCUDA/alias.cu
@@ -1,8 +1,11 @@
 // REQUIRES: x86-registered-target
 // REQUIRES: nvptx-registered-target
+// REQUIRES: amdgpu-registered-target
 
 // RUN: %clang_cc1 -fcuda-is-device -triple nvptx-nvidia-cuda -emit-llvm \
 // RUN:   -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fcuda-is-device -triple amdgcn -emit-llvm \
+// RUN:   -o - %s | FileCheck %s
 
 #include "Inputs/cuda.h"
 
Index: cfe/trunk/lib/CodeGen/TargetInfo.cpp
===
--- cfe/trunk/lib/CodeGen/TargetInfo.cpp
+++ cfe/trunk/lib/CodeGen/TargetInfo.cpp
@@ -6154,6 +6154,7 @@
 
   void setTargetAttributes(const Decl *D, llvm::GlobalValue *GV,
CodeGen::CodeGenModule &M) const override;
+  bool shouldEmitStaticExternCAliases() const override;
 
 private:
   // Adds a NamedMDNode with F, Name, and Operand as operands, and adds the
@@ -6275,6 +6276,10 @@
   // Append metadata to nvvm.annotations
   MD->addOperand(llvm::MDNode::get(Ctx, MDVals));
 }
+
+bool NVPTXTargetCodeGenInfo::shouldEmitStaticExternCAliases() const {
+  return false;
+}
 }
 
 
//===--===//
@@ -7646,6 +7651,7 @@
   createEnqueuedBlockKernel(CodeGenFunction &CGF,
 llvm::Function *BlockInvokeFunc,
 llvm::Value *BlockLiteral) const override;
+  bool shouldEmitStaticExternCAliases() const override;
 };
 }
 
@@ -,6 +7783,10 @@
   return C.getOrInsertSyncScopeID(Name);
 }
 
+bool AMDGPUTargetCodeGenInfo::shouldEmitStaticExternCAliases() const {
+  return false;
+}
+
 
//===--===//
 // SPARC v8 ABI Implementation.
 // Based on the SPARC Compliance Definition version 2.4.1.
Index: cfe/trunk/lib/CodeGen/TargetInfo.h
===
--- cfe/trunk/lib/CodeGen/TargetInfo.h
+++ cfe/trunk/lib/CodeGen/TargetInfo.h
@@ -296,6 +296,11 @@
   createEnqueuedBlockKernel(CodeGenFunction &CGF,
 llvm::Function *BlockInvokeFunc,
 llvm::Value *BlockLiteral) const;
+
+  /// \return true if the target supports alias from the unmangled name to the
+  /// mangled name of functions declared within an extern "C" region and marked
+  /// as 'used', and having internal linkage.
+  virtual bool shouldEmitStaticExternCAliases() const { return true; }
 };
 
 } // namespace CodeGen
Index: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
===
--- cfe/trunk/lib/CodeGen/CodeGenModule.cpp
+++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp
@@ -4677,9 +4677,7 @@
 /// to such functions with an unmangled name from inline assembly within the
 /// same translation unit.
 void CodeGenModule::EmitStaticExternCAliases() {
-  // Don't do anything if we're generating CUDA device code -- the NVPTX
-  // assembly target doesn't support aliases.
-  if (Context.getTargetInfo().getTriple().isNVPTX())
+  if (!getTargetCodeGenInfo().shouldEmitStaticExternCAliases())
 return;
   for (auto &I : StaticExternCValues) {
 IdentifierInfo *Name = I.first;


Index: cfe/trunk/test/CodeGenCUDA/alias.cu
===
--- cfe/trunk/test/CodeGenCUDA/alias.cu
+++ cfe/trunk/test/CodeGenCUDA/alias.cu
@@ -1,8 +1,11 @@
 // REQUIRES: x86-registered-target
 // REQUIRES: nvptx-registered-target
+// REQUIRES: amdgpu-registered-target
 
 // RUN: %clang_cc1 -fcuda-is-device -triple nvptx-nvidia-cuda -emit-llvm \
 // RUN:   -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fcuda-is-device -triple amdgcn -emit-llvm \
+// RUN:   -o - %s | FileCheck %s
 
 #include "Inputs/cuda.h"
 
Index: cfe/trunk/lib/CodeGen/TargetInfo.cpp
===
--- cfe/trunk/lib/CodeGen/TargetInfo.cpp
+++ cfe/trunk/lib/CodeGen/TargetInfo.cpp
@@ -6154,6 +6154,7 @@
 
   void setTargetAttributes(const Decl *D, llvm::GlobalValue *GV,
CodeGen::CodeGenModule &M) const override;
+  bool shouldEmitStaticExternCAliases() const override;
 
 private:
   // Adds a NamedMDNode with F, Name, and Operand as operands, and adds

r328795 - Set calling convention for CUDA kernel

[Yaxun Liu via cfe-commits]

Author: yaxunl
Date: Thu Mar 29 08:02:08 2018
New Revision: 328795

URL: http://llvm.org/viewvc/llvm-project?rev=328795&view=rev
Log:
Set calling convention for CUDA kernel

This patch sets target specific calling convention for CUDA kernels in IR.

Patch by Greg Rodgers.
Revised and lit test added by Yaxun Liu.

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

Added:
cfe/trunk/test/CodeGenCUDA/kernel-amdgcn.cu
Modified:
cfe/trunk/include/clang/Basic/Specifiers.h
cfe/trunk/lib/AST/ItaniumMangle.cpp
cfe/trunk/lib/AST/Type.cpp
cfe/trunk/lib/AST/TypePrinter.cpp
cfe/trunk/lib/CodeGen/CGCall.cpp
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
cfe/trunk/lib/CodeGen/TargetInfo.cpp
cfe/trunk/lib/CodeGen/TargetInfo.h
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Sema/SemaOverload.cpp
cfe/trunk/lib/Sema/SemaType.cpp
cfe/trunk/tools/libclang/CXType.cpp

Modified: cfe/trunk/include/clang/Basic/Specifiers.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Specifiers.h?rev=328795&r1=328794&r2=328795&view=diff
==
--- cfe/trunk/include/clang/Basic/Specifiers.h (original)
+++ cfe/trunk/include/clang/Basic/Specifiers.h Thu Mar 29 08:02:08 2018
@@ -231,23 +231,24 @@ namespace clang {
 
   /// \brief CallingConv - Specifies the calling convention that a function 
uses.
   enum CallingConv {
-CC_C,   // __attribute__((cdecl))
-CC_X86StdCall,  // __attribute__((stdcall))
-CC_X86FastCall, // __attribute__((fastcall))
-CC_X86ThisCall, // __attribute__((thiscall))
+CC_C, // __attribute__((cdecl))
+CC_X86StdCall,// __attribute__((stdcall))
+CC_X86FastCall,   // __attribute__((fastcall))
+CC_X86ThisCall,   // __attribute__((thiscall))
 CC_X86VectorCall, // __attribute__((vectorcall))
-CC_X86Pascal,   // __attribute__((pascal))
-CC_Win64,   // __attribute__((ms_abi))
-CC_X86_64SysV,  // __attribute__((sysv_abi))
-CC_X86RegCall, // __attribute__((regcall))
-CC_AAPCS,   // __attribute__((pcs("aapcs")))
-CC_AAPCS_VFP,   // __attribute__((pcs("aapcs-vfp")))
-CC_IntelOclBicc, // __attribute__((intel_ocl_bicc))
-CC_SpirFunction, // default for OpenCL functions on SPIR target
-CC_OpenCLKernel, // inferred for OpenCL kernels
-CC_Swift,// __attribute__((swiftcall))
-CC_PreserveMost, // __attribute__((preserve_most))
-CC_PreserveAll,  // __attribute__((preserve_all))
+CC_X86Pascal, // __attribute__((pascal))
+CC_Win64, // __attribute__((ms_abi))
+CC_X86_64SysV,// __attribute__((sysv_abi))
+CC_X86RegCall,// __attribute__((regcall))
+CC_AAPCS, // __attribute__((pcs("aapcs")))
+CC_AAPCS_VFP, // __attribute__((pcs("aapcs-vfp")))
+CC_IntelOclBicc,  // __attribute__((intel_ocl_bicc))
+CC_SpirFunction,  // default for OpenCL functions on SPIR target
+CC_OpenCLKernel,  // inferred for OpenCL kernels
+CC_Swift, // __attribute__((swiftcall))
+CC_PreserveMost,  // __attribute__((preserve_most))
+CC_PreserveAll,   // __attribute__((preserve_all))
+CC_CUDAKernel,// inferred for CUDA kernels
   };
 
   /// \brief Checks whether the given calling convention supports variadic

Modified: cfe/trunk/lib/AST/ItaniumMangle.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ItaniumMangle.cpp?rev=328795&r1=328794&r2=328795&view=diff
==
--- cfe/trunk/lib/AST/ItaniumMangle.cpp (original)
+++ cfe/trunk/lib/AST/ItaniumMangle.cpp Thu Mar 29 08:02:08 2018
@@ -2628,6 +2628,7 @@ StringRef CXXNameMangler::getCallingConv
   case CC_OpenCLKernel:
   case CC_PreserveMost:
   case CC_PreserveAll:
+  case CC_CUDAKernel:
 // FIXME: we should be mangling all of the above.
 return "";
 

Modified: cfe/trunk/lib/AST/Type.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Type.cpp?rev=328795&r1=328794&r2=328795&view=diff
==
--- cfe/trunk/lib/AST/Type.cpp (original)
+++ cfe/trunk/lib/AST/Type.cpp Thu Mar 29 08:02:08 2018
@@ -2752,6 +2752,7 @@ StringRef FunctionType::getNameForCallCo
   case CC_Swift: return "swiftcall";
   case CC_PreserveMost: return "preserve_most";
   case CC_PreserveAll: return "preserve_all";
+  case CC_CUDAKernel: return "cuda_kernel";
   }
 
   llvm_unreachable("Invalid calling convention.");

Modified: cfe/trunk/lib/AST/TypePrinter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/TypePrinter.cpp?rev=328795&r1=328794&r2=328795&view=diff
==
--- cfe/trunk/lib/AST/TypePrinter.cpp (original)
+++ cfe/trunk/lib/AST/TypePrinter.cpp Thu Mar 29 08:02:08 2018
@@ -780,6 +780,10 @@ void TypePrinter::printFunctionAfter(con
 case CC_OpenCL

[PATCH] D44747: Set calling convention for CUDA kernel

[Yaxun Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL328795: Set calling convention for CUDA kernel (authored by 
yaxunl, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D44747?vs=140131&id=140256#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D44747

Files:
  cfe/trunk/include/clang/Basic/Specifiers.h
  cfe/trunk/lib/AST/ItaniumMangle.cpp
  cfe/trunk/lib/AST/Type.cpp
  cfe/trunk/lib/AST/TypePrinter.cpp
  cfe/trunk/lib/CodeGen/CGCall.cpp
  cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
  cfe/trunk/lib/CodeGen/TargetInfo.cpp
  cfe/trunk/lib/CodeGen/TargetInfo.h
  cfe/trunk/lib/Sema/SemaExpr.cpp
  cfe/trunk/lib/Sema/SemaOverload.cpp
  cfe/trunk/lib/Sema/SemaType.cpp
  cfe/trunk/test/CodeGenCUDA/kernel-amdgcn.cu
  cfe/trunk/tools/libclang/CXType.cpp

Index: cfe/trunk/lib/CodeGen/TargetInfo.h
===
--- cfe/trunk/lib/CodeGen/TargetInfo.h
+++ cfe/trunk/lib/CodeGen/TargetInfo.h
@@ -223,6 +223,9 @@
   /// Get LLVM calling convention for OpenCL kernel.
   virtual unsigned getOpenCLKernelCallingConv() const;
 
+  /// Get LLVM calling convention for CUDA kernel.
+  virtual unsigned getCUDAKernelCallingConv() const;
+
   /// Get target specific null pointer.
   /// \param T is the LLVM type of the null pointer.
   /// \param QT is the clang QualType of the null pointer.
Index: cfe/trunk/lib/CodeGen/CGCall.cpp
===
--- cfe/trunk/lib/CodeGen/CGCall.cpp
+++ cfe/trunk/lib/CodeGen/CGCall.cpp
@@ -64,6 +64,7 @@
   case CC_PreserveMost: return llvm::CallingConv::PreserveMost;
   case CC_PreserveAll: return llvm::CallingConv::PreserveAll;
   case CC_Swift: return llvm::CallingConv::Swift;
+  case CC_CUDAKernel: return CGM.getTargetCodeGenInfo().getCUDAKernelCallingConv();
   }
 }
 
Index: cfe/trunk/lib/CodeGen/TargetInfo.cpp
===
--- cfe/trunk/lib/CodeGen/TargetInfo.cpp
+++ cfe/trunk/lib/CodeGen/TargetInfo.cpp
@@ -431,6 +431,10 @@
   return llvm::CallingConv::SPIR_KERNEL;
 }
 
+unsigned TargetCodeGenInfo::getCUDAKernelCallingConv() const {
+  return llvm::CallingConv::C;
+}
+
 llvm::Constant *TargetCodeGenInfo::getNullPointer(const CodeGen::CodeGenModule &CGM,
 llvm::PointerType *T, QualType QT) const {
   return llvm::ConstantPointerNull::get(T);
@@ -7635,6 +7639,7 @@
   void setTargetAttributes(const Decl *D, llvm::GlobalValue *GV,
CodeGen::CodeGenModule &M) const override;
   unsigned getOpenCLKernelCallingConv() const override;
+  unsigned getCUDAKernelCallingConv() const override;
 
   llvm::Constant *getNullPointer(const CodeGen::CodeGenModule &CGM,
   llvm::PointerType *T, QualType QT) const override;
@@ -7722,6 +7727,10 @@
   return llvm::CallingConv::AMDGPU_KERNEL;
 }
 
+unsigned AMDGPUTargetCodeGenInfo::getCUDAKernelCallingConv() const {
+  return llvm::CallingConv::AMDGPU_KERNEL;
+}
+
 // Currently LLVM assumes null pointers always have value 0,
 // which results in incorrectly transformed IR. Therefore, instead of
 // emitting null pointers in private and local address spaces, a null
Index: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
===
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
@@ -1022,6 +1022,9 @@
 return llvm::dwarf::DW_CC_LLVM_PreserveAll;
   case CC_X86RegCall:
 return llvm::dwarf::DW_CC_LLVM_X86RegCall;
+  case CC_CUDAKernel:
+// ToDo: Add llvm::dwarf::DW_CC_LLVM_CUDAKernel;
+return 0;
   }
   return 0;
 }
Index: cfe/trunk/lib/AST/ItaniumMangle.cpp
===
--- cfe/trunk/lib/AST/ItaniumMangle.cpp
+++ cfe/trunk/lib/AST/ItaniumMangle.cpp
@@ -2628,6 +2628,7 @@
   case CC_OpenCLKernel:
   case CC_PreserveMost:
   case CC_PreserveAll:
+  case CC_CUDAKernel:
 // FIXME: we should be mangling all of the above.
 return "";
 
Index: cfe/trunk/lib/AST/Type.cpp
===
--- cfe/trunk/lib/AST/Type.cpp
+++ cfe/trunk/lib/AST/Type.cpp
@@ -2752,6 +2752,7 @@
   case CC_Swift: return "swiftcall";
   case CC_PreserveMost: return "preserve_most";
   case CC_PreserveAll: return "preserve_all";
+  case CC_CUDAKernel: return "cuda_kernel";
   }
 
   llvm_unreachable("Invalid calling convention.");
Index: cfe/trunk/lib/AST/TypePrinter.cpp
===
--- cfe/trunk/lib/AST/TypePrinter.cpp
+++ cfe/trunk/lib/AST/TypePrinter.cpp
@@ -780,6 +780,10 @@
 case CC_OpenCLKernel:
   // Do nothing. These CCs are not available as attributes.
   break;
+case CC_CUDAKernel:
+  // ToDo: print this before the function.
+  OS << " __global__";
+  break;
 case CC_Swift:
   OS << " __attribute__((swiftcall))"

[PATCH] D44747: Set calling convention for CUDA kernel

[Yaxun Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC328795: Set calling convention for CUDA kernel (authored by 
yaxunl, committed by ).

Repository:
  rL LLVM

https://reviews.llvm.org/D44747

Files:
  include/clang/Basic/Specifiers.h
  lib/AST/ItaniumMangle.cpp
  lib/AST/Type.cpp
  lib/AST/TypePrinter.cpp
  lib/CodeGen/CGCall.cpp
  lib/CodeGen/CGDebugInfo.cpp
  lib/CodeGen/TargetInfo.cpp
  lib/CodeGen/TargetInfo.h
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaOverload.cpp
  lib/Sema/SemaType.cpp
  test/CodeGenCUDA/kernel-amdgcn.cu
  tools/libclang/CXType.cpp

Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -1022,6 +1022,9 @@
 return llvm::dwarf::DW_CC_LLVM_PreserveAll;
   case CC_X86RegCall:
 return llvm::dwarf::DW_CC_LLVM_X86RegCall;
+  case CC_CUDAKernel:
+// ToDo: Add llvm::dwarf::DW_CC_LLVM_CUDAKernel;
+return 0;
   }
   return 0;
 }
Index: lib/CodeGen/TargetInfo.h
===
--- lib/CodeGen/TargetInfo.h
+++ lib/CodeGen/TargetInfo.h
@@ -223,6 +223,9 @@
   /// Get LLVM calling convention for OpenCL kernel.
   virtual unsigned getOpenCLKernelCallingConv() const;
 
+  /// Get LLVM calling convention for CUDA kernel.
+  virtual unsigned getCUDAKernelCallingConv() const;
+
   /// Get target specific null pointer.
   /// \param T is the LLVM type of the null pointer.
   /// \param QT is the clang QualType of the null pointer.
Index: lib/CodeGen/CGCall.cpp
===
--- lib/CodeGen/CGCall.cpp
+++ lib/CodeGen/CGCall.cpp
@@ -64,6 +64,7 @@
   case CC_PreserveMost: return llvm::CallingConv::PreserveMost;
   case CC_PreserveAll: return llvm::CallingConv::PreserveAll;
   case CC_Swift: return llvm::CallingConv::Swift;
+  case CC_CUDAKernel: return CGM.getTargetCodeGenInfo().getCUDAKernelCallingConv();
   }
 }
 
Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -431,6 +431,10 @@
   return llvm::CallingConv::SPIR_KERNEL;
 }
 
+unsigned TargetCodeGenInfo::getCUDAKernelCallingConv() const {
+  return llvm::CallingConv::C;
+}
+
 llvm::Constant *TargetCodeGenInfo::getNullPointer(const CodeGen::CodeGenModule &CGM,
 llvm::PointerType *T, QualType QT) const {
   return llvm::ConstantPointerNull::get(T);
@@ -7635,6 +7639,7 @@
   void setTargetAttributes(const Decl *D, llvm::GlobalValue *GV,
CodeGen::CodeGenModule &M) const override;
   unsigned getOpenCLKernelCallingConv() const override;
+  unsigned getCUDAKernelCallingConv() const override;
 
   llvm::Constant *getNullPointer(const CodeGen::CodeGenModule &CGM,
   llvm::PointerType *T, QualType QT) const override;
@@ -7722,6 +7727,10 @@
   return llvm::CallingConv::AMDGPU_KERNEL;
 }
 
+unsigned AMDGPUTargetCodeGenInfo::getCUDAKernelCallingConv() const {
+  return llvm::CallingConv::AMDGPU_KERNEL;
+}
+
 // Currently LLVM assumes null pointers always have value 0,
 // which results in incorrectly transformed IR. Therefore, instead of
 // emitting null pointers in private and local address spaces, a null
Index: lib/AST/Type.cpp
===
--- lib/AST/Type.cpp
+++ lib/AST/Type.cpp
@@ -2752,6 +2752,7 @@
   case CC_Swift: return "swiftcall";
   case CC_PreserveMost: return "preserve_most";
   case CC_PreserveAll: return "preserve_all";
+  case CC_CUDAKernel: return "cuda_kernel";
   }
 
   llvm_unreachable("Invalid calling convention.");
Index: lib/AST/TypePrinter.cpp
===
--- lib/AST/TypePrinter.cpp
+++ lib/AST/TypePrinter.cpp
@@ -780,6 +780,10 @@
 case CC_OpenCLKernel:
   // Do nothing. These CCs are not available as attributes.
   break;
+case CC_CUDAKernel:
+  // ToDo: print this before the function.
+  OS << " __global__";
+  break;
 case CC_Swift:
   OS << " __attribute__((swiftcall))";
   break;
Index: lib/AST/ItaniumMangle.cpp
===
--- lib/AST/ItaniumMangle.cpp
+++ lib/AST/ItaniumMangle.cpp
@@ -2628,6 +2628,7 @@
   case CC_OpenCLKernel:
   case CC_PreserveMost:
   case CC_PreserveAll:
+  case CC_CUDAKernel:
 // FIXME: we should be mangling all of the above.
 return "";
 
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -3316,6 +3316,18 @@
   CallingConv CC = S.Context.getDefaultCallingConvention(FTI.isVariadic,
  IsCXXInstanceMethod);
 
+  // Attribute AT_CUDAGlobal affects the calling convention for AMDGPU targets.
+  // This is the simplest place to infe

[PATCH] D44996: [clang-format] Ensure ObjC selectors with 0 args are annotated correctly

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton updated this revision to Diff 140259.
benhamilton added a comment.

@jolesiak


Repository:
  rC Clang

https://reviews.llvm.org/D44996

Files:
  lib/Format/TokenAnnotator.cpp
  unittests/Format/FormatTestObjC.cpp


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -522,6 +522,23 @@
   verifyFormat("- (void)drawRectOn:(id)surface\n"
"ofSize:(size_t)height\n"
"  :(size_t)width;");
+  Style.ColumnLimit = 40;
+  // Make sure selectors with 0, 1, or more arguments are not indented
+  // when IndentWrappedFunctionNames is false.
+  Style.IndentWrappedFunctionNames = false;
+  verifyFormat("- (a)\n"
+   ";\n");
+  verifyFormat("- (a)\n"
+   ":(int)a;\n");
+  verifyFormat("- (a)\n"
+   ":(int)a\n"
+   ":(int)a;\n");
+  verifyFormat("- (a)\n"
+   " aaa:(int)a\n"
+   ":(int)a;\n");
+  verifyFormat("- (a)\n"
+   ":(int)a\n"
+   " aaa:(int)a;\n");
 
   // Continuation indent width should win over aligning colons if the function
   // name is long.
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -1343,6 +1343,16 @@
  TT_LeadingJavaAnnotation)) {
 Current.Type = Current.Previous->Type;
   }
+} else if (Current.isOneOf(tok::identifier, tok::kw_new) &&
+   Current.Previous && Current.Previous->is(TT_CastRParen) &&
+   Current.Previous->MatchingParen &&
+   Current.Previous->MatchingParen->Previous &&
+   Current.Previous->MatchingParen->Previous->is(
+   TT_ObjCMethodSpecifier)) {
+  // This is the first part of an Objective-C selector name. (If there's no
+  // colon after this, this is the only place which annotates the 
identifier
+  // as a selector.)
+  Current.Type = TT_SelectorName;
 } else if (Current.isOneOf(tok::identifier, tok::kw_const) &&
Current.Previous &&
!Current.Previous->isOneOf(tok::equal, tok::at) &&


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -522,6 +522,23 @@
   verifyFormat("- (void)drawRectOn:(id)surface\n"
"ofSize:(size_t)height\n"
"  :(size_t)width;");
+  Style.ColumnLimit = 40;
+  // Make sure selectors with 0, 1, or more arguments are not indented
+  // when IndentWrappedFunctionNames is false.
+  Style.IndentWrappedFunctionNames = false;
+  verifyFormat("- (a)\n"
+   ";\n");
+  verifyFormat("- (a)\n"
+   ":(int)a;\n");
+  verifyFormat("- (a)\n"
+   ":(int)a\n"
+   ":(int)a;\n");
+  verifyFormat("- (a)\n"
+   " aaa:(int)a\n"
+   ":(int)a;\n");
+  verifyFormat("- (a)\n"
+   ":(int)a\n"
+   " aaa:(int)a;\n");
 
   // Continuation indent width should win over aligning colons if the function
   // name is long.
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -1343,6 +1343,16 @@
  TT_LeadingJavaAnnotation)) {
 Current.Type = Current.Previous->Type;
   }
+} else if (Current.isOneOf(tok::identifier, tok::kw_new) &&
+   Current.Previous && Current.Previous->is(TT_CastRParen) &&
+   Current.Previous->MatchingParen &&
+   Current.Previous->MatchingParen->Previous &&
+   Current.Previous->MatchingParen->Previous->is(
+   TT_ObjCMethodSpecifier)) {
+  // This is the first part of an Objective-C selector name. (If there's no
+  // colon after this, this is the only place which annotates the identifier
+  // as a selector.)
+  Current.Type = TT_Se

[PATCH] D44996: [clang-format] Ensure ObjC selectors with 0 args are annotated correctly

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton marked 3 inline comments as done.
benhamilton added inline comments.



Comment at: unittests/Format/FormatTestObjC.cpp:527
+  // Make sure selectors with 0, 1, or more arguments are not indented
+  // when IndentWrappedFunctionNames is false.
+  verifyFormat("- (a)\n"

jolesiak wrote:
> jolesiak wrote:
> > jolesiak wrote:
> > > I know that `Style.IndentWrappedFunctionNames` is false by default, but 
> > > how about adding 
> > > ```
> > > Style.IndentWrappedFunctionNames = false;
> > > ```
> > > before these `verifyFormat` calls? I feel like that makes it more 
> > > readable and makes test independent on this `Style` constant.
> > This comment is kind of irrelevant as this change is included in D45005.
> My bad, this change is NOT included in D45005. (I don't know how to remove 
> inline comment)
Sure, done.


Repository:
  rC Clang

https://reviews.llvm.org/D44996



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

[PATCH] D45004: [clang-format] New style option IndentWrappedObjCMethodNames

[Ben Hamilton via Phabricator via cfe-commits]

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



Comment at: include/clang/Format/Format.h:1154-1163
+  /// \brief The style of indenting long function or method names wrapped
+  /// onto the next line.
+  enum IndentWrappedMethodStyle {
+/// Automatically determine indenting style.
+IWM_Auto,
+/// Always indent wrapped method names.
+IWM_Always,

stephanemoore wrote:
> Do we explicitly want these to be generic with the intent to later reuse the 
> enum for C++ method wrapping style?
I figured languages like Java and JS might want to customize this.


Repository:
  rC Clang

https://reviews.llvm.org/D45004



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

[PATCH] D45004: [clang-format] New style option IndentWrappedObjCMethodNames

[Ben Hamilton via Phabricator via cfe-commits]

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

> Do we have a public style guide that explicitly says to do this?

Good question! This was the result of internal discussion with the Google 
Objective-C style guide maintainers based on analysis of ObjC code at Google.

I took a look, and the C++ style guide doesn't seem to explicitly say to align 
wrapped function names on column 0 (although clang-format does that):

http://google.github.io/styleguide/cppguide.html#Function_Declarations_and_Definitions
http://google.github.io/styleguide/cppguide.html#Function_Calls

I will confirm with the ObjC style guide maintainers whether they want to 
explicitly list the desired ObjC behavior in the style guide.


Repository:
  rC Clang

https://reviews.llvm.org/D45004



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

[PATCH] D44985: Remove initializer for CUDA shared varirable

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In https://reviews.llvm.org/D44985#1050876, @rjmccall wrote:

> In https://reviews.llvm.org/D44985#1050674, @yaxunl wrote:
>
> > In https://reviews.llvm.org/D44985#1050670, @rjmccall wrote:
> >
> > > What exactly are you trying to express here?  Are you just trying to make 
> > > these external declarations when compiling for the device because 
> > > `__shared__` variables are actually defined on the host?  That should be 
> > > handled by the frontend by setting up the AST so that these declarations 
> > > are not definitions.
> >
> >
> > No. These variables are not like external symbols defined on the host. They 
> > behave like global variables in the kernel code but never initialized. 
> > Currently no targets are able to initialize them and it is users' 
> > responsibility to initialize them explicitly.
> >
> > Giving them an initial value will cause error in some backends since they 
> > cannot handle them, therefore put undef as initializer.
>
>
> So undef is being used as a special marker to the backends that it's okay not 
> to try to initialize these variables?


I think undef as the initializer tells the llvm passes and backend that this 
global variable contains undefined value. I am not sure if this is better than 
without an initializer. I saw code in CodeGenModule::getOrCreateStaticVarDecl

  // Local address space cannot have an initializer.
  llvm::Constant *Init = nullptr;
  if (Ty.getAddressSpace() != LangAS::opencl_local)
Init = EmitNullConstant(Ty);
  else
Init = llvm::UndefValue::get(LTy);

which means OpenCL static variable in local address space (equivalent to CUDA 
shared address space) gets an undef initializer.

For CUDA shared variable, in CodeGenFunction::EmitStaticVarDecl, it first goes 
through call of CodeGenModule::getOrCreateStaticVarDecl and gets a 
zeroinitializer, then it reaches line 400

  // Whatever initializer such variable may have when it gets here is
// a no-op and should not be emitted.
bool isCudaSharedVar = getLangOpts().CUDA && getLangOpts().CUDAIsDevice &&
   D.hasAttr();
// If this value has an initializer, emit it.
if (D.getInit() && !isCudaSharedVar)
  var = AddInitializerToStaticVarDecl(D, var);

Although this disables adding initializer from D, var already has a 
zeroinitializer from CodeGenModule::getOrCreateStaticVarDecl, therefore its 
initializer needs to be overwritten by undef.

Probably a better solution would be do it in  
CodeGenModule::getOrCreateStaticVarDecl, side by side by the OpenCL code.


https://reviews.llvm.org/D44985



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

[PATCH] D45004: [clang-format] New style option IndentWrappedObjCMethodNames

[Daniel Jasper via Phabricator via cfe-commits]

djasper added a comment.

Well, I disagree. It says: "If you break after the return type of a function 
declaration or definition, do not indent."


Repository:
  rC Clang

https://reviews.llvm.org/D45004



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

[PATCH] D45004: [clang-format] New style option IndentWrappedObjCMethodNames

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton added a comment.

> Well, I disagree. It says: "If you break after the return type of a function 
> declaration or definition, do not indent."

Great, thanks for clarifying!


Repository:
  rC Clang

https://reviews.llvm.org/D45004



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

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

[Csaba Dabis via Phabricator via cfe-commits]

Charusso created this revision.
Charusso added reviewers: alexfh, aaron.ballman, xazax.hun.
Charusso added a project: clang-tools-extra.
Herald added subscribers: cfe-commits, rnkovacs, mgorny.

New checker called bugprone-not-null-terminated-result. This check can be used 
to find function calls where `strlen` or `wcslen` are passed as an argument and 
cause a not null-terminated result. Usually the proper length is the source's 
own length + 1: `strlen(src) + 1` because the null-terminator need an extra 
space. Depending on the case of use, it may insert or remove the increase 
operation to ensure the result is null-terminated.

The following function calls are checked:
`alloca`, `calloc`, `malloc`, `realloc`, `memcpy`, `wmemcpy`, `memcpy_s`, 
`wmemcpy_s`, `memchr`, `wmemchr`, `memmove`, `wmemmove`, `memmove_s`, 
`wmemmove_s`, `memset`, `wmemset`, `strerror_s`, `strncmp`, `wcsncmp`, 
`strxfrm`, `wcsxfrm`

Example problematic code:

  void bad_memcpy(char *dest, const char *src) {
memcpy(dest, src, strlen(src));
  }

After running the tool it would be the following if the target is C++11:

  void good_memcpy_cxx11(char *dest, const char *src) {
strncpy_s(dest, src, strlen(src));
  }

or if the target is older, then it would be following:

  void good_memcpy_not_cxx11(char *dest, const char *src) {
strncpy(dest, src, (strlen(src) + 1));
  }


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45050

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
  clang-tidy/bugprone/NotNullTerminatedResultCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-not-null-terminated-result-strlen-before-cxx11.cpp
  test/clang-tidy/bugprone-not-null-terminated-result-strlen-cxx11.cpp
  test/clang-tidy/bugprone-not-null-terminated-result-wcslen-before-cxx11.cpp
  test/clang-tidy/bugprone-not-null-terminated-result-wcslen-cxx11.cpp

Index: test/clang-tidy/bugprone-not-null-terminated-result-wcslen-cxx11.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-not-null-terminated-result-wcslen-cxx11.cpp
@@ -0,0 +1,153 @@
+// RUN: %check_clang_tidy %s bugprone-not-null-terminated-result %t -- -- -std=c++11
+
+typedef unsigned int size_t;
+typedef int errno_t;
+size_t wcslen(const wchar_t *);
+wchar_t *wcschr(const wchar_t *, int);
+errno_t *wcsncpy_s(wchar_t *, const wchar_t *, size_t);
+
+void *alloca(size_t);
+void *calloc(size_t, size_t);
+void *malloc(size_t);
+void *realloc(void *, size_t);
+
+void *wmemcpy(void *, const void *, size_t);
+errno_t wmemcpy_s(void *, size_t, const void *, size_t);
+void *wmemchr(const void *, int, size_t);
+void *wmemmove(void *, const void *, size_t);
+errno_t wmemmove_s(void *, size_t, const void *, size_t);
+void *wmemset(void *, int, size_t);
+
+int wcsncmp(const wchar_t *, const wchar_t *, size_t);
+size_t wcsxfrm(wchar_t *, const wchar_t *, size_t);
+
+namespace std {
+using ::wcsncpy_s;
+using ::wmemcpy;
+using ::wcslen;
+}
+
+void bad_alloca(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)alloca(wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 'alloca' function's allocated memory cannot hold the null-terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: alloca(wcslen(src) + 1);
+}
+
+void good_alloca(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)alloca(wcslen(src) + 1);
+}
+
+void bad_calloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)calloc(wcslen(src), sizeof(wchar_t));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 'calloc' function's allocated memory cannot hold the null-terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: calloc((wcslen(src) + 1), sizeof(wchar_t));
+}
+
+void good_calloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)calloc((wcslen(src) + 1), sizeof(wchar_t));
+}
+
+void bad_malloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)malloc(wcslen(src) * 2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 'malloc' function's allocated memory cannot hold the null-terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: malloc((wcslen(src) * 2) + 1);
+}
+
+void good_malloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)malloc((wcslen(src) * 2) + 1);
+}
+
+void bad_realloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)realloc(dest, (wcslen(dest) + wcslen(src)));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 'realloc' function's allocated memory cannot hold the null-terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: realloc(dest, (wcslen(dest) + (wcslen(src) + 1)));
+}
+
+void good_realloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)realloc(dest, (wcslen(dest) + (wcslen(src) + 1)));
+}
+
+void bad_wmemcpy(wchar_t *dest, const wchar_t

[PATCH] D45002: [test] Fix an XRay test on FreeBSD

[David CARLIER via Phabricator via cfe-commits]

devnexen added a comment.

Since I do not have commit permissions hopefully @dberris will come by or kamil 
if he s less busy.


Repository:
  rC Clang

https://reviews.llvm.org/D45002



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

[PATCH] D44883: [Sema] Extend -Wself-assign and -Wself-assign-field to warn on overloaded self-assignment (classes)

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Ok, tried llvm stage2 build, and it failed as expected, in code that 
intentionally does self-assignment:
https://github.com/llvm-mirror/llvm/blob/1aa8147054e7ba8f2b7ea25daaed804662b4c6b2/unittests/ADT/DenseMapTest.cpp#L249-L250

  [2/311 1.1/sec] Building CXX object 
unittests/ADT/CMakeFiles/ADTTests.dir/DenseMapTest.cpp.o
  FAILED: unittests/ADT/CMakeFiles/ADTTests.dir/DenseMapTest.cpp.o 
  /build/llvm-build-Clang-release/bin/clang++  -DGTEST_HAS_RTTI=0 
-DGTEST_HAS_TR1_TUPLE=0 -DGTEST_LANG_CXX11=1 -D_DEBUG -D_GNU_SOURCE 
-D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS 
-Iunittests/ADT -I/build/llvm/unittests/ADT -I/usr/include/libxml2 -Iinclude 
-I/build/llvm/include -I/build/llvm/utils/unittest/googletest/include 
-I/build/llvm/utils/unittest/googlemock/include -g0 -fPIC 
-fvisibility-inlines-hidden -Werror -Werror=date-time 
-Werror=unguarded-availability-new -std=c++11 -Wall -W -Wno-unused-parameter 
-Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic 
-Wno-long-long -Wcovered-switch-default -Wnon-virtual-dtor 
-Wdelete-non-virtual-dtor -Wstring-conversion -fcolor-diagnostics 
-ffunction-sections -fdata-sections -g0   -UNDEBUG  -Wno-variadic-macros 
-Wno-gnu-zero-variadic-macro-arguments -fno-exceptions -fno-rtti -MD -MT 
unittests/ADT/CMakeFiles/ADTTests.dir/DenseMapTest.cpp.o -MF 
unittests/ADT/CMakeFiles/ADTTests.dir/DenseMapTest.cpp.o.d -o 
unittests/ADT/CMakeFiles/ADTTests.dir/DenseMapTest.cpp.o -c 
/build/llvm/unittests/ADT/DenseMapTest.cpp
  /build/llvm/unittests/ADT/DenseMapTest.cpp:250:11: error: explicitly 
assigning value of variable of type '(anonymous 
namespace)::DenseMapTest_AssignmentTest_Test::TypeParam' (aka 
'gtest_TypeParam_') to itself [-Werror,-Wself-assign]
copyMap = copyMap;
~~~ ^ ~~~
  /build/llvm/unittests/ADT/DenseMapTest.cpp:265:11: error: explicitly 
assigning value of variable of type '(anonymous 
namespace)::DenseMapTest_AssignmentTestNotSmall_Test::TypeParam' (aka 
'gtest_TypeParam_') to itself [-Werror,-Wself-assign]
copyMap = copyMap;
~~~ ^ ~~~

The `operator=` for `DenseMap`/`SmallDenseMap` is user-provided, non-trivial:
https://github.com/llvm-mirror/llvm/blob/1aa8147054e7ba8f2b7ea25daaed804662b4c6b2/include/llvm/ADT/DenseMap.h#L693-L705
https://github.com/llvm-mirror/llvm/blob/1aa8147054e7ba8f2b7ea25daaed804662b4c6b2/include/llvm/ADT/DenseMap.h#L926-L938

So i guess this will have to be split after all.
I'll try what we have discussed (`-Wself-assign`, and 
`-Wself-assign-nontrivial` for non-trivial methods.)


Repository:
  rC Clang

https://reviews.llvm.org/D44883



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

[PATCH] D39053: [Bitfield] Add more cases to making the bitfield a separate location

[Strahinja Petrovic via Phabricator via cfe-commits]

spetrovic added a comment.

ping


https://reviews.llvm.org/D39053



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

r328797 - [test] Fix an XRay test on FreeBSD

[Zhihao Yuan via cfe-commits]

Author: lichray
Date: Thu Mar 29 08:50:44 2018
New Revision: 328797

URL: http://llvm.org/viewvc/llvm-project?rev=328797&view=rev
Log:
[test] Fix an XRay test on FreeBSD

Summary: Fixing clang-test on FreeBSD as a follow-up of 
https://reviews.llvm.org/D43378 to handle the revert happened in r325749.

Reviewers: devnexen, krytarowski, dberris

Subscribers: emaste, dberris, cfe-commits

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

Modified:
cfe/trunk/test/Driver/XRay/xray-instrument-os.c

Modified: cfe/trunk/test/Driver/XRay/xray-instrument-os.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/XRay/xray-instrument-os.c?rev=328797&r1=328796&r2=328797&view=diff
==
--- cfe/trunk/test/Driver/XRay/xray-instrument-os.c (original)
+++ cfe/trunk/test/Driver/XRay/xray-instrument-os.c Thu Mar 29 08:50:44 2018
@@ -1,4 +1,4 @@
 // RUN: not %clang -o /dev/null -v -fxray-instrument -c %s
-// XFAIL: -linux-
+// XFAIL: -linux-, -freebsd
 // REQUIRES-ANY: amd64, x86_64, x86_64h, arm, aarch64, arm64
 typedef int a;


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

[PATCH] D45002: [test] Fix an XRay test on FreeBSD

[Zhihao Yuan via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rC328797: [test] Fix an XRay test on FreeBSD (authored by 
lichray, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D45002?vs=140143&id=140264#toc

Repository:
  rC Clang

https://reviews.llvm.org/D45002

Files:
  test/Driver/XRay/xray-instrument-os.c


Index: test/Driver/XRay/xray-instrument-os.c
===
--- test/Driver/XRay/xray-instrument-os.c
+++ test/Driver/XRay/xray-instrument-os.c
@@ -1,4 +1,4 @@
 // RUN: not %clang -o /dev/null -v -fxray-instrument -c %s
-// XFAIL: -linux-
+// XFAIL: -linux-, -freebsd
 // REQUIRES-ANY: amd64, x86_64, x86_64h, arm, aarch64, arm64
 typedef int a;


Index: test/Driver/XRay/xray-instrument-os.c
===
--- test/Driver/XRay/xray-instrument-os.c
+++ test/Driver/XRay/xray-instrument-os.c
@@ -1,4 +1,4 @@
 // RUN: not %clang -o /dev/null -v -fxray-instrument -c %s
-// XFAIL: -linux-
+// XFAIL: -linux-, -freebsd
 // REQUIRES-ANY: amd64, x86_64, x86_64h, arm, aarch64, arm64
 typedef int a;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44994: [clang-format] Ensure wrapped ObjC selectors with 1 arg obey IndentWrappedFunctionNames

[Daniel Jasper via Phabricator via cfe-commits]

djasper added inline comments.



Comment at: lib/Format/ContinuationIndenter.cpp:904
+   : State.Stack.back().Indent);
   if (NextNonComment->LongestObjCSelectorName == 0)
+return MinIndent;

benhamilton wrote:
> djasper wrote:
> > Does this if actually change the behavior in any way? With 
> > LongestObjCSelectorName being 0, isn't that:
> > 
> >   return MinIndent +
> >  std::max(0, ColumnWidth) - ColumnWidth;
> > 
> > (and with ColumnWidth being >= 0, this should be just MinIndent)
> The `- ColumnWidth` part is only for the case where `LongestObjCSelectorName` 
> is *not* 0. If it's 0, we return `MinIndent` which ensures we obey 
> `Style.IndentWrappedFunctionNames`.
> 
> The problem with the code before this diff is when `LongestObjCSelectorName` 
> was 0, we ignored `Style.IndentWrappedFunctionNames` and always returned 
> `State.Stack.back().Indent` regardless of that setting.
> 
> After this diff, when `LongestObjCSelectorName` is 0 (i.e., this is either 
> the first part of the selector or a selector which is not colon-aligned due 
> to block formatting), we change the behavior to indent to at least 
> `State.FirstIndent + Style.ContinuationIndentWidth`, like all other 
> indentation logic in this file.
> 
> I've added some comments explaining what's going on, since this code is quite 
> complex.
I am not saying your change is wrong.  And I might be getting out of practice 
with coding. My question is, what changes if you remove lines 906 and 907 (the 
"if (...) return MinIndent")?


Repository:
  rC Clang

https://reviews.llvm.org/D44994



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

[PATCH] D44720: [clangd] Simplify fuzzy matcher (sequence alignment) by removing some condition checks.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clangd/FuzzyMatch.cpp:230
 void FuzzyMatcher::buildGraph() {
+  Scores[0][0][Miss] = Scores[0][0][Match] = {0, Miss};
   for (int W = 0; W < WordN; ++W) {

sammccall wrote:
> why this change?
> this has also been moved from the cheaper constructor to the more expensive 
> per-match call. (also the diagonal assignment added in the next loop)
> 
> Also, shouldn't [0][0][Match] be AwfulScore?
> 
"The more expensive per-match call" is just two value assignments.

I have removed the expensive table initialization in the constructor.

[0][0][*] can be any value.



Comment at: clangd/FuzzyMatch.cpp:325
+  int W = PatN;
+  for (int I = PatN; ++I <= WordN; )
+if (Scores[PatN][I][Match].Score > Scores[PatN][W][Match].Score)

sammccall wrote:
> nit: I -> P, move increment to the increment expression of the for loop?
I -> P.

> move increment to the increment expression of the for loop?

Not sure about the coding standard here, but if you insist I'll have to change 
it as you are the reviewer. If the loop variable was an iterator, `for (It I = 
std::next(...); I != E; ++I)` would be uglier than `for (It I = ...; ++I != E; 
)`



Comment at: clangd/FuzzyMatch.cpp:340
+  A[WordN] = Miss;
+  for (int I = WordN, P = PatN; I > 0; I--) {
+if (I == W)

sammccall wrote:
> sammccall wrote:
> > sammccall wrote:
> > > W is the right name in this file for a variable iterating over word 
> > > indices, please don't change this.
> > > The new variable above could be EndW or so?
> > As far as I can see, this loop is setting `A[W+1:...] = Miss` and 
> > populating `A[0...W]` with the exsting logic.
> > I think this would be clearer as two loops, currently there's a lot of 
> > conditionals around Last that obscure what's actually happening.
> You've shifted P (and the old W, new I) by 1. This does reduce the number of 
> +1 and -1 in this function, but it's inconsistent with how these are used 
> elsewhere: P should be the index into Pat of the character that we're 
> considering.
I don't understand the rationale not to use the shifted indices. The code 
actually use `Scores[P][W][*]` to mean the optimal match of the first `P` 
characters of the pattern with the first `W` characters of the word, not the 
position of the character.

On the other hand, C++ reverse iterators use the shifted one for `for (I = 
rend(); I != rbegin(); ++I)`. The shifted one makes ending condition check 
easier.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44720



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

[PATCH] D44720: [clangd] Simplify fuzzy matcher (sequence alignment) by removing some condition checks.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 140265.
MaskRay edited the summary of this revision.
MaskRay added a comment.

Address comments


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44720

Files:
  clangd/FuzzyMatch.cpp
  clangd/FuzzyMatch.h

Index: clangd/FuzzyMatch.h
===
--- clangd/FuzzyMatch.h
+++ clangd/FuzzyMatch.h
@@ -58,8 +58,8 @@
   void buildGraph();
   void calculateRoles(const char *Text, CharRole *Out, int &Types, int N);
   bool allowMatch(int P, int W) const;
-  int skipPenalty(int W, Action Last) const;
-  int matchBonus(int P, int W, Action Last) const;
+  int missScore(int W, Action Last) const;
+  int matchScore(int P, int W, Action Last) const;
 
   // Pattern data is initialized by the constructor, then constant.
   char Pat[MaxPat]; // Pattern data
Index: clangd/FuzzyMatch.cpp
===
--- clangd/FuzzyMatch.cpp
+++ clangd/FuzzyMatch.cpp
@@ -58,6 +58,7 @@
 
 #include "FuzzyMatch.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/Format.h"
 
 namespace clang {
@@ -67,7 +68,6 @@
 constexpr int FuzzyMatcher::MaxPat;
 constexpr int FuzzyMatcher::MaxWord;
 
-static char lower(char C) { return C >= 'A' && C <= 'Z' ? C + ('a' - 'A') : C; }
 // A "negative infinity" score that won't overflow.
 // We use this to mark unreachable states and forbidden solutions.
 // Score field is 15 bits wide, min value is -2^14, we use half of that.
@@ -80,25 +80,17 @@
   ScoreScale(PatN ? float{1} / (PerfectBonus * PatN) : 0), WordN(0) {
   std::copy(Pattern.begin(), Pattern.begin() + PatN, Pat);
   for (int I = 0; I < PatN; ++I)
-LowPat[I] = lower(Pat[I]);
-  Scores[0][0][Miss] = {0, Miss};
-  Scores[0][0][Match] = {AwfulScore, Miss};
-  for (int P = 0; P <= PatN; ++P)
-for (int W = 0; W < P; ++W)
-  for (Action A : {Miss, Match})
-Scores[P][W][A] = {AwfulScore, Miss};
-  if (PatN > 0)
-calculateRoles(Pat, PatRole, PatTypeSet, PatN);
+LowPat[I] = toLower(Pat[I]);
+  calculateRoles(Pat, PatRole, PatTypeSet, PatN);
 }
 
 Optional FuzzyMatcher::match(StringRef Word) {
   if (!(WordContainsPattern = init(Word)))
 return None;
-  if (!PatN)
-return 1;
   buildGraph();
-  auto Best = std::max(Scores[PatN][WordN][Miss].Score,
-   Scores[PatN][WordN][Match].Score);
+  int Best = AwfulScore;
+  for (int I = PatN; I <= WordN; I++)
+Best = std::max(Best, Scores[PatN][I][Match].Score);
   if (isAwful(Best))
 return None;
   return ScoreScale * std::min(PerfectBonus * PatN, std::max(0, Best));
@@ -179,7 +171,8 @@
 }
 void FuzzyMatcher::calculateRoles(const char *Text, CharRole *Out, int &TypeSet,
   int N) {
-  assert(N > 0);
+  if (!N)
+return;
   CharType Type = packedLookup(CharTypes, Text[0]);
   TypeSet = 1 << Type;
   // Types holds a sliding window of (Prev, Curr, Next) types.
@@ -206,10 +199,8 @@
   if (PatN > WordN)
 return false;
   std::copy(NewWord.begin(), NewWord.begin() + WordN, Word);
-  if (PatN == 0)
-return true;
   for (int I = 0; I < WordN; ++I)
-LowWord[I] = lower(Word[I]);
+LowWord[I] = toLower(Word[I]);
 
   // Cheap subsequence check.
   for (int W = 0, P = 0; P != PatN; ++W) {
@@ -236,31 +227,29 @@
 // This range is not strict: we can apply larger bonuses/penalties, or penalize
 // non-matched characters.
 void FuzzyMatcher::buildGraph() {
+  Scores[0][0][Miss] = Scores[0][0][Match] = {0, Miss};
   for (int W = 0; W < WordN; ++W) {
-Scores[0][W + 1][Miss] = {Scores[0][W][Miss].Score - skipPenalty(W, Miss),
+Scores[0][W + 1][Miss] = {Scores[0][W][Miss].Score + missScore(W, Miss),
   Miss};
 Scores[0][W + 1][Match] = {AwfulScore, Miss};
   }
   for (int P = 0; P < PatN; ++P) {
+Scores[P + 1][P][Miss] = Scores[P + 1][P][Match] = {AwfulScore, Miss};
 for (int W = P; W < WordN; ++W) {
   auto &Score = Scores[P + 1][W + 1], &PreMiss = Scores[P + 1][W];
 
-  auto MatchMissScore = PreMiss[Match].Score;
-  auto MissMissScore = PreMiss[Miss].Score;
-  if (P < PatN - 1) { // Skipping trailing characters is always free.
-MatchMissScore -= skipPenalty(W, Match);
-MissMissScore -= skipPenalty(W, Miss);
-  }
+  auto MatchMissScore = PreMiss[Match].Score + missScore(W, Match);
+  auto MissMissScore = PreMiss[Miss].Score + missScore(W, Miss);
   Score[Miss] = (MatchMissScore > MissMissScore)
 ? ScoreInfo{MatchMissScore, Match}
 : ScoreInfo{MissMissScore, Miss};
 
   if (!allowMatch(P, W)) {
 Score[Match] = {AwfulScore, Miss};
   } else {
 auto &PreMatch = Scores[P][W];
-auto MatchMatchScore = PreMatch[Match].Score + matchBonus(P, W, Match);
-auto MissMatchScore = PreMatch[Miss].Score + matchBonus(P, W, Miss);
+auto Mat

[PATCH] D44720: [clangd] Simplify fuzzy matcher (sequence alignment) by removing some condition checks.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 140268.
MaskRay added a comment.

Add comment

  // Find the optimal prefix of Word to match Pattern.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44720

Files:
  clangd/FuzzyMatch.cpp
  clangd/FuzzyMatch.h

Index: clangd/FuzzyMatch.h
===
--- clangd/FuzzyMatch.h
+++ clangd/FuzzyMatch.h
@@ -58,8 +58,8 @@
   void buildGraph();
   void calculateRoles(const char *Text, CharRole *Out, int &Types, int N);
   bool allowMatch(int P, int W) const;
-  int skipPenalty(int W, Action Last) const;
-  int matchBonus(int P, int W, Action Last) const;
+  int missScore(int W, Action Last) const;
+  int matchScore(int P, int W, Action Last) const;
 
   // Pattern data is initialized by the constructor, then constant.
   char Pat[MaxPat]; // Pattern data
Index: clangd/FuzzyMatch.cpp
===
--- clangd/FuzzyMatch.cpp
+++ clangd/FuzzyMatch.cpp
@@ -58,6 +58,7 @@
 
 #include "FuzzyMatch.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/Format.h"
 
 namespace clang {
@@ -67,7 +68,6 @@
 constexpr int FuzzyMatcher::MaxPat;
 constexpr int FuzzyMatcher::MaxWord;
 
-static char lower(char C) { return C >= 'A' && C <= 'Z' ? C + ('a' - 'A') : C; }
 // A "negative infinity" score that won't overflow.
 // We use this to mark unreachable states and forbidden solutions.
 // Score field is 15 bits wide, min value is -2^14, we use half of that.
@@ -80,25 +80,18 @@
   ScoreScale(PatN ? float{1} / (PerfectBonus * PatN) : 0), WordN(0) {
   std::copy(Pattern.begin(), Pattern.begin() + PatN, Pat);
   for (int I = 0; I < PatN; ++I)
-LowPat[I] = lower(Pat[I]);
-  Scores[0][0][Miss] = {0, Miss};
-  Scores[0][0][Match] = {AwfulScore, Miss};
-  for (int P = 0; P <= PatN; ++P)
-for (int W = 0; W < P; ++W)
-  for (Action A : {Miss, Match})
-Scores[P][W][A] = {AwfulScore, Miss};
-  if (PatN > 0)
-calculateRoles(Pat, PatRole, PatTypeSet, PatN);
+LowPat[I] = toLower(Pat[I]);
+  calculateRoles(Pat, PatRole, PatTypeSet, PatN);
 }
 
 Optional FuzzyMatcher::match(StringRef Word) {
   if (!(WordContainsPattern = init(Word)))
 return None;
-  if (!PatN)
-return 1;
   buildGraph();
-  auto Best = std::max(Scores[PatN][WordN][Miss].Score,
-   Scores[PatN][WordN][Match].Score);
+  // Find the optimal prefix of Word to match Pattern.
+  int Best = AwfulScore;
+  for (int I = PatN; I <= WordN; I++)
+Best = std::max(Best, Scores[PatN][I][Match].Score);
   if (isAwful(Best))
 return None;
   return ScoreScale * std::min(PerfectBonus * PatN, std::max(0, Best));
@@ -179,7 +172,8 @@
 }
 void FuzzyMatcher::calculateRoles(const char *Text, CharRole *Out, int &TypeSet,
   int N) {
-  assert(N > 0);
+  if (!N)
+return;
   CharType Type = packedLookup(CharTypes, Text[0]);
   TypeSet = 1 << Type;
   // Types holds a sliding window of (Prev, Curr, Next) types.
@@ -206,10 +200,8 @@
   if (PatN > WordN)
 return false;
   std::copy(NewWord.begin(), NewWord.begin() + WordN, Word);
-  if (PatN == 0)
-return true;
   for (int I = 0; I < WordN; ++I)
-LowWord[I] = lower(Word[I]);
+LowWord[I] = toLower(Word[I]);
 
   // Cheap subsequence check.
   for (int W = 0, P = 0; P != PatN; ++W) {
@@ -236,31 +228,29 @@
 // This range is not strict: we can apply larger bonuses/penalties, or penalize
 // non-matched characters.
 void FuzzyMatcher::buildGraph() {
+  Scores[0][0][Miss] = Scores[0][0][Match] = {0, Miss};
   for (int W = 0; W < WordN; ++W) {
-Scores[0][W + 1][Miss] = {Scores[0][W][Miss].Score - skipPenalty(W, Miss),
+Scores[0][W + 1][Miss] = {Scores[0][W][Miss].Score + missScore(W, Miss),
   Miss};
 Scores[0][W + 1][Match] = {AwfulScore, Miss};
   }
   for (int P = 0; P < PatN; ++P) {
+Scores[P + 1][P][Miss] = Scores[P + 1][P][Match] = {AwfulScore, Miss};
 for (int W = P; W < WordN; ++W) {
   auto &Score = Scores[P + 1][W + 1], &PreMiss = Scores[P + 1][W];
 
-  auto MatchMissScore = PreMiss[Match].Score;
-  auto MissMissScore = PreMiss[Miss].Score;
-  if (P < PatN - 1) { // Skipping trailing characters is always free.
-MatchMissScore -= skipPenalty(W, Match);
-MissMissScore -= skipPenalty(W, Miss);
-  }
+  auto MatchMissScore = PreMiss[Match].Score + missScore(W, Match);
+  auto MissMissScore = PreMiss[Miss].Score + missScore(W, Miss);
   Score[Miss] = (MatchMissScore > MissMissScore)
 ? ScoreInfo{MatchMissScore, Match}
 : ScoreInfo{MissMissScore, Miss};
 
   if (!allowMatch(P, W)) {
 Score[Match] = {AwfulScore, Miss};
   } else {
 auto &PreMatch = Scores[P][W];
-auto MatchMatchScore = PreMatch[Match].Score + matchBonus(P, W, Match);
-auto MissMatchScore = Pr

[PATCH] D44720: [clangd] Simplify fuzzy matcher (sequence alignment) by removing some condition checks.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clangd/FuzzyMatch.cpp:93
+  for (int I = PatN; I <= WordN; I++)
+Best = std::max(Best, Scores[PatN][I][Match].Score);
   if (isAwful(Best))

sammccall wrote:
> MaskRay wrote:
> > sammccall wrote:
> > > this looks like a behavior change - why?
> > This is a behavior change. Instead of choosing between `Match/Miss` in the 
> > last position, we enumerate the last matching position in `Word`.
> > 
> > This saves `if (P < PatN - 1) {` check in the main loop at the cost of a 
> > for loop here (use sites of ending values)
> Ah, I see - the case where we match only part of the word is handled up here 
> now.
> (I think you mean this is not a behavior change? The result is the same 
> AFAICS)
> 
> That does make more sense, but it's pretty subtle.
> Can you add a comment like
>  `// The pattern doesn't have to match the whole word (but the whole pattern 
> must match).`
Added
```
  // Find the optimal prefix of Word to match Pattern.
```

I meant this is a behavior change but it makes the first row and the rest rows 
of the score table more consistent.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44720



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

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp:1
+#include "NotNullTerminatedResultCheck.h"
+#include "clang/AST/ASTContext.h"

Missing header blurb



Comment at: clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp:14
+  const MatchFinder::MatchResult &Result) {
+  return Lexer::getSourceText(
+  CharSourceRange::getTokenRange(Expr->getSourceRange()),

Doesn't `Lexer::getSourceText()` return `StringRef`?



Comment at: clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp:264
+const auto FirstArg = FuncExpr->getArg(0);
+std::string NewSecondArg = " strlen(" + exprToStr(FirstArg, Result) + "),";
+

This should probably be `SmallString<32>`



Comment at: clang-tidy/bugprone/NotNullTerminatedResultCheck.h:40
+   DiagnosticBuilder &Diag);
+  void memchrFix(StringRef Name,
+ const ast_matchers::MatchFinder::MatchResult &Result,

Why are all these internal functions `public`?
They should be either in anonymous namespace (best), or at least have `private` 
visibility.



Comment at: docs/ReleaseNotes.rst:68
+
+- The 'misc-incorrect-roundings' check was renamed to 
`bugprone-incorrect-roundings
+  
`_

This seems out-of-place. Why is this in the diff?



Comment at: 
test/clang-tidy/bugprone-not-null-terminated-result-strlen-before-cxx11.cpp:11
+void bad_memcpy(char *dest, const char *src) {
+  memcpy(dest, src, strlen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memcpy' function's result is 
not null-terminated [bugprone-not-null-terminated-result]

What about these functions, but in `std::` namespace?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45050



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

[PATCH] D44720: [clangd] Simplify fuzzy matcher (sequence alignment) by removing some condition checks.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clangd/FuzzyMatch.cpp:96
 return None;
   return ScoreScale * std::min(PerfectBonus * PatN, std::max(0, Best));
 }

sammccall wrote:
> MaskRay wrote:
> > I also don't understand why it clamps the value to zero here. Negative 
> > values are also meaningful to me. Given that perfectBonus is only 3 it is 
> > very easy to get a negative value here.
> An important part of the contract of `match()` is that it returns a value in 
> `[0,1]`.
> We rely on this range to combine this with other scoring signals - we 
> multiply this by a quality signal in code completion.
> (Currently the quality signal is just derived from Sema, but the global index 
> will provide the number of uses).
> 
> It would be possible to use a different squash function here, but I found 
> max(kFloor,x) worked well for the examples I looked at - anything <= some 
> floor value was "not really a useful match at all", and most of the variance 
> below the floor seemed to be noise to me.
> (Then I tuned the bonuses/penalties so the floor was at zero)
We could try other criteria in the future. I believe the current one can be 
improved because negative scores may be returned but the scoring shouldn't 
return 0 for all the cases.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44720



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

[PATCH] D44994: [clang-format] Ensure wrapped ObjC selectors with 1 arg obey IndentWrappedFunctionNames

[Ben Hamilton via Phabricator via cfe-commits]

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



Comment at: lib/Format/ContinuationIndenter.cpp:904
+   : State.Stack.back().Indent);
   if (NextNonComment->LongestObjCSelectorName == 0)
+return MinIndent;

djasper wrote:
> benhamilton wrote:
> > djasper wrote:
> > > Does this if actually change the behavior in any way? With 
> > > LongestObjCSelectorName being 0, isn't that:
> > > 
> > >   return MinIndent +
> > >  std::max(0, ColumnWidth) - ColumnWidth;
> > > 
> > > (and with ColumnWidth being >= 0, this should be just MinIndent)
> > The `- ColumnWidth` part is only for the case where 
> > `LongestObjCSelectorName` is *not* 0. If it's 0, we return `MinIndent` 
> > which ensures we obey `Style.IndentWrappedFunctionNames`.
> > 
> > The problem with the code before this diff is when 
> > `LongestObjCSelectorName` was 0, we ignored 
> > `Style.IndentWrappedFunctionNames` and always returned 
> > `State.Stack.back().Indent` regardless of that setting.
> > 
> > After this diff, when `LongestObjCSelectorName` is 0 (i.e., this is either 
> > the first part of the selector or a selector which is not colon-aligned due 
> > to block formatting), we change the behavior to indent to at least 
> > `State.FirstIndent + Style.ContinuationIndentWidth`, like all other 
> > indentation logic in this file.
> > 
> > I've added some comments explaining what's going on, since this code is 
> > quite complex.
> I am not saying your change is wrong.  And I might be getting out of practice 
> with coding. My question is, what changes if you remove lines 906 and 907 
> (the "if (...) return MinIndent")?
Oh, I see what you're saying now! Thanks for clarifying.

Yes, we can remove these lines now. Done.


Repository:
  rC Clang

https://reviews.llvm.org/D44994



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

[PATCH] D44994: [clang-format] Ensure wrapped ObjC selectors with 1 arg obey IndentWrappedFunctionNames

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton updated this revision to Diff 140271.
benhamilton added a comment.

Remove if (...) return MinIndent.


Repository:
  rC Clang

https://reviews.llvm.org/D44994

Files:
  lib/Format/ContinuationIndenter.cpp
  unittests/Format/FormatTestObjC.cpp


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -537,6 +537,22 @@
"   aShortf:(NSRect)theRect {\n"
"}");
 
+  // Make sure selectors with 0, 1, or more arguments are indented
+  // when IndentWrappedFunctionNames is true.
+  verifyFormat("- (a)\n"
+   ";\n");
+  verifyFormat("- (a)\n"
+   ":(int)a;\n");
+  verifyFormat("- (a)\n"
+   ":(int)a\n"
+   ":(int)a;\n");
+  verifyFormat("- (a)\n"
+   " aaa:(int)a\n"
+   ":(int)a;\n");
+  verifyFormat("- (a)\n"
+   ":(int)a\n"
+   " aaa:(int)a;\n");
+
   // Format pairs correctly.
   Style.ColumnLimit = 80;
   verifyFormat("- (void)drawRectOn:(id)surface\n"
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -896,12 +896,20 @@
 return std::max(State.Stack.back().LastSpace, State.Stack.back().Indent);
   if (NextNonComment->is(TT_SelectorName)) {
 if (!State.Stack.back().ObjCSelectorNameFound) {
-  if (NextNonComment->LongestObjCSelectorName == 0)
-return State.Stack.back().Indent;
-  return (Style.IndentWrappedFunctionNames
-  ? std::max(State.Stack.back().Indent,
- State.FirstIndent + Style.ContinuationIndentWidth)
-  : State.Stack.back().Indent) +
+  unsigned MinIndent = State.Stack.back().Indent;
+  if (Style.IndentWrappedFunctionNames)
+MinIndent = std::max(MinIndent,
+ State.FirstIndent + 
Style.ContinuationIndentWidth);
+  // If LongestObjCSelectorName is 0, we are indenting the first
+  // part of an ObjC selector (or a selector component which is
+  // not colon-aligned due to block formatting).
+  //
+  // Otherwise, we are indenting a subsequent part of an ObjC
+  // selector which should be colon-aligned to the longest
+  // component of the ObjC selector.
+  //
+  // In either case, we want to respect Style.IndentWrappedFunctionNames.
+  return MinIndent +
  std::max(NextNonComment->LongestObjCSelectorName,
   NextNonComment->ColumnWidth) -
  NextNonComment->ColumnWidth;


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -537,6 +537,22 @@
"   aShortf:(NSRect)theRect {\n"
"}");
 
+  // Make sure selectors with 0, 1, or more arguments are indented
+  // when IndentWrappedFunctionNames is true.
+  verifyFormat("- (a)\n"
+   ";\n");
+  verifyFormat("- (a)\n"
+   ":(int)a;\n");
+  verifyFormat("- (a)\n"
+   ":(int)a\n"
+   ":(int)a;\n");
+  verifyFormat("- (a)\n"
+   " aaa:(int)a\n"
+   ":(int)a;\n");
+  verifyFormat("- (a)\n"
+   ":(int)a\n"
+   " aaa:(int)a;\n");
+
   // Format pairs correctly.
   Style.ColumnLimit = 80;
   verifyFormat("- (void)drawRectOn:(id)surface\n"
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -896,12 +896,20 @@
 return std::max(State.Stack.back().LastSpace, State.Stack.back().Indent);
   if (NextNonComment->is(TT_SelectorName)) {
 if (!State.Stack.back().ObjCSelectorNameFound) {
-  if (NextNonComment->LongestObjCSelectorName == 0)
-return State.Stack.back().Indent;
-  return (Style.IndentWrappedFunctionNames
-  ? std::max(State.Stack.back().In

[PATCH] D44882: [clangd] Implementation of workspace/symbol request

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

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

Use the DraftStore for offset -> line/col when there is a draft available.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44882

Files:
  clangd/CMakeLists.txt
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/FindSymbols.cpp
  clangd/FindSymbols.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/ProtocolHandlers.cpp
  clangd/ProtocolHandlers.h
  clangd/SourceCode.cpp
  clangd/SourceCode.h
  clangd/XRefs.cpp
  clangd/index/SymbolCollector.cpp
  clangd/tool/ClangdMain.cpp
  test/clangd/initialize-params-invalid.test
  test/clangd/initialize-params.test
  unittests/clangd/CMakeLists.txt
  unittests/clangd/FindSymbolsTests.cpp
  unittests/clangd/SyncAPI.cpp
  unittests/clangd/SyncAPI.h

Index: unittests/clangd/SyncAPI.h
===
--- unittests/clangd/SyncAPI.h
+++ unittests/clangd/SyncAPI.h
@@ -41,6 +41,10 @@
 
 std::string runDumpAST(ClangdServer &Server, PathRef File);
 
+llvm::Expected>
+runWorkspaceSymbols(ClangdServer &Server, StringRef Query,
+const WorkspaceSymbolOptions &Opts, const DraftStore &DS);
+
 } // namespace clangd
 } // namespace clang
 
Index: unittests/clangd/SyncAPI.cpp
===
--- unittests/clangd/SyncAPI.cpp
+++ unittests/clangd/SyncAPI.cpp
@@ -110,5 +110,13 @@
   return std::move(*Result);
 }
 
+llvm::Expected>
+runWorkspaceSymbols(ClangdServer &Server, StringRef Query,
+const WorkspaceSymbolOptions &Opts, const DraftStore &DS) {
+  llvm::Optional>> Result;
+  Server.workspaceSymbols(Query, Opts, DS, capture(Result));
+  return std::move(*Result);
+}
+
 } // namespace clangd
 } // namespace clang
Index: unittests/clangd/FindSymbolsTests.cpp
===
--- /dev/null
+++ unittests/clangd/FindSymbolsTests.cpp
@@ -0,0 +1,383 @@
+//===-- FindSymbolsTests.cpp -*- C++ -*===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+#include "Annotations.h"
+#include "ClangdServer.h"
+#include "DraftStore.h"
+#include "FindSymbols.h"
+#include "SyncAPI.h"
+#include "TestFS.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+
+void PrintTo(const SymbolInformation &I, std::ostream *O) {
+  llvm::raw_os_ostream OS(*O);
+  OS << I.containerName << I.name << " - " << toJSON(I);
+}
+
+namespace {
+
+using ::testing::AllOf;
+using ::testing::AnyOf;
+using ::testing::ElementsAre;
+using ::testing::IsEmpty;
+using ::testing::UnorderedElementsAre;
+
+class IgnoreDiagnostics : public DiagnosticsConsumer {
+  void onDiagnosticsReady(PathRef File,
+  std::vector Diagnostics) override {}
+};
+
+// GMock helpers for matching SymbolInfos items.
+MATCHER_P(Named, Name, "") { return arg.name == Name; }
+MATCHER_P(InContainer, ContainerName, "") {
+  return arg.containerName == ContainerName;
+}
+MATCHER_P(WithKind, Kind, "") { return arg.kind == Kind; }
+
+class WorkspaceSymbolsTest : public ::testing::Test {
+protected:
+  std::unique_ptr FSProvider;
+  std::unique_ptr CDB;
+  std::unique_ptr DiagConsumer;
+  std::unique_ptr Server;
+  std::unique_ptr Opts;
+  std::unique_ptr DS;
+
+  virtual void SetUp() override {
+FSProvider = llvm::make_unique();
+CDB = llvm::make_unique();
+DiagConsumer = llvm::make_unique();
+auto ServerOpts = ClangdServer::optsForTest();
+ServerOpts.BuildDynamicSymbolIndex = true;
+Server = llvm::make_unique(*CDB, *FSProvider, *DiagConsumer,
+ ServerOpts);
+Opts = llvm::make_unique();
+DS = llvm::make_unique();
+  }
+
+  std::vector getSymbols(StringRef Query) {
+EXPECT_TRUE(Server->blockUntilIdleForTest()) << "Waiting for preamble";
+auto SymbolInfos = runWorkspaceSymbols(*Server, Query, *Opts, *DS);
+EXPECT_TRUE(bool(SymbolInfos)) << "workspaceSymbols returned an error";
+return *SymbolInfos;
+  }
+
+  void addFile(StringRef FileName, StringRef Contents) {
+auto Path = testPath(FileName);
+FSProvider->Files[Path] = Contents;
+Server->addDocument(Path, Contents);
+  }
+};
+
+} // namespace
+
+TEST_F(WorkspaceSymbolsTest, NoMacro) {
+  addFile("foo.cpp", R"cpp(
+  #define MACRO X
+  )cpp");
+
+  // Macros are not in the index.
+  EXPECT_THAT(getSymbols("macro"), IsEmpty());
+}
+
+TEST_F(WorkspaceSymbolsTest, NoLocals) {
+  addFile("foo.cpp", R"cpp(
+  void test() {
+struct LocalClass {};
+int local_var;
+  })cpp");
+  EXPECT_THAT(getSymbols("local_var"), IsEmpty());
+  EXPE

[PATCH] D44882: [clangd] Implementation of workspace/symbol request

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

malaperle added a comment.

I realized that using the draft store has limited improvement for now because 
not many symbol locations come from main files (.cpp, etc) and when headers are 
modified, the main files are not reindexed right now. So the draft store will 
give better location for example when navigating to a function definition which 
moved in the unsaved main file. But it will be more general once header updates 
are handled better.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44882



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

[PATCH] D44883: [Sema] Extend -Wself-assign and -Wself-assign-field to warn on overloaded self-assignment (classes)

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In https://reviews.llvm.org/D44883#1051878, @lebedev.ri wrote:

> Ok, tried llvm stage2 build, and it failed as expected, in code that 
> intentionally does self-assignment:


Actually, this is not the sort of failure that I think should worry you.  Any 
new warning has the ability to break -Werror builds; if I saw this failure in a 
build, I would totally consider it a project bug, not a false-positive.

In this case, you should just make sure there's a reasonable way to suppress 
the warning and then fix the code in LLVM by doing that.  Explicitly casting 
the RHS to the right reference type should shut the compiler up.


Repository:
  rC Clang

https://reviews.llvm.org/D44883



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

[PATCH] D44985: Remove initializer for CUDA shared varirable

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In https://reviews.llvm.org/D44985#1051840, @yaxunl wrote:

> In https://reviews.llvm.org/D44985#1050876, @rjmccall wrote:
>
> > In https://reviews.llvm.org/D44985#1050674, @yaxunl wrote:
> >
> > > In https://reviews.llvm.org/D44985#1050670, @rjmccall wrote:
> > >
> > > > What exactly are you trying to express here?  Are you just trying to 
> > > > make these external declarations when compiling for the device because 
> > > > `__shared__` variables are actually defined on the host?  That should 
> > > > be handled by the frontend by setting up the AST so that these 
> > > > declarations are not definitions.
> > >
> > >
> > > No. These variables are not like external symbols defined on the host. 
> > > They behave like global variables in the kernel code but never 
> > > initialized. Currently no targets are able to initialize them and it is 
> > > users' responsibility to initialize them explicitly.
> > >
> > > Giving them an initial value will cause error in some backends since they 
> > > cannot handle them, therefore put undef as initializer.
> >
> >
> > So undef is being used as a special marker to the backends that it's okay 
> > not to try to initialize these variables?
>
>
> I think undef as the initializer tells the llvm passes and backend that this 
> global variable contains undefined value. I am not sure if this is better 
> than without an initializer. I saw code in 
> CodeGenModule::getOrCreateStaticVarDecl
>
>   // Local address space cannot have an initializer.
>   llvm::Constant *Init = nullptr;
>   if (Ty.getAddressSpace() != LangAS::opencl_local)
> Init = EmitNullConstant(Ty);
>   else
> Init = llvm::UndefValue::get(LTy);
>   
>   
>
> which means OpenCL static variable in local address space (equivalent to CUDA 
> shared address space) gets an undef initializer.
>
> For CUDA shared variable, in CodeGenFunction::EmitStaticVarDecl, it first 
> goes through call of CodeGenModule::getOrCreateStaticVarDecl and gets a 
> zeroinitializer, then it reaches line 400
>
>   // Whatever initializer such variable may have when it gets here is
> // a no-op and should not be emitted.
> bool isCudaSharedVar = getLangOpts().CUDA && getLangOpts().CUDAIsDevice &&
>D.hasAttr();
> // If this value has an initializer, emit it.
> if (D.getInit() && !isCudaSharedVar)
>   var = AddInitializerToStaticVarDecl(D, var);
>  
>
>
> Although this disables adding initializer from D, var already has a 
> zeroinitializer from CodeGenModule::getOrCreateStaticVarDecl, therefore its 
> initializer needs to be overwritten by undef.
>
> Probably a better solution would be do it in  
> CodeGenModule::getOrCreateStaticVarDecl, side by side by the OpenCL code.


Yes, I agree, just updating the condition to trigger if either language mode is 
set is the right fix.


https://reviews.llvm.org/D44985



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

[PATCH] D44882: [clangd] Implementation of workspace/symbol request

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clangd/FindSymbols.cpp:31
+
+  if (supportedSymbolKinds &&
+  std::find(supportedSymbolKinds->begin(), supportedSymbolKinds->end(),

This std::find loop adds some overhead to each candidate... In my experience 
the client usually doesn't care about the returned symbol kinds, they are used 
to give a category name. You can always patch the upstream to add missing 
categories.

This is one instance where LSP is over specified. nvm I don't have strong 
opinion here



Comment at: clangd/Protocol.cpp:206
+
+bool fromJSON(const json::Expr &Params, WorkspaceClientCapabilities &R) {
+  json::ObjectMapper O(Params);

This copy-pasting exposes another problem that the current  `fromJSON` `toJSON` 
approach is too verbose and you may find other space-efficient serialization 
format convenient  Anyway, they can always be improved in the future.



Comment at: clangd/tool/ClangdMain.cpp:235
 
+  clangd::WorkspaceSymbolOptions WorkspaceSymOpts;
+  WorkspaceSymOpts.Limit = LimitResults;

I know command line options are easy for testing purpose but they are clumsy 
for users. You need a "initialization option" <-> "command line option" bridge.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44882



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

r328800 - Add a dllimport test.

[Rafael Espindola via cfe-commits]

Author: rafael
Date: Thu Mar 29 09:35:52 2018
New Revision: 328800

URL: http://llvm.org/viewvc/llvm-project?rev=328800&view=rev
Log:
Add a dllimport test.

Thanks to rnk for the suggestion.

Added:
cfe/trunk/test/CodeGen/flip-dllimport.c

Added: cfe/trunk/test/CodeGen/flip-dllimport.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/flip-dllimport.c?rev=328800&view=auto
==
--- cfe/trunk/test/CodeGen/flip-dllimport.c (added)
+++ cfe/trunk/test/CodeGen/flip-dllimport.c Thu Mar 29 09:35:52 2018
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -triple x86_64-windows-msvc -fms-extensions -emit-llvm -o - 
%s | FileCheck %s
+
+__declspec(dllimport) void f();
+void g() { f(); } // use it
+
+// CHECK: define dso_local dllexport void @f
+void f() { }


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

[PATCH] D43341: [clang-doc] Implement reducer portion of the frontend framework

[Julie Hockett via Phabricator via cfe-commits]

juliehockett updated this revision to Diff 140275.
juliehockett added a comment.

Fixing assert on vector size.


https://reviews.llvm.org/D43341

Files:
  clang-doc/BitcodeReader.cpp
  clang-doc/BitcodeReader.h
  clang-doc/BitcodeWriter.cpp
  clang-doc/BitcodeWriter.h
  clang-doc/CMakeLists.txt
  clang-doc/Reducer.cpp
  clang-doc/Reducer.h
  clang-doc/Representation.cpp
  clang-doc/Representation.h
  clang-doc/tool/ClangDocMain.cpp
  docs/ReleaseNotes.rst
  test/clang-doc/bc-comment.cpp
  test/clang-doc/bc-namespace.cpp
  test/clang-doc/bc-record.cpp

Index: test/clang-doc/bc-record.cpp
===
--- /dev/null
+++ test/clang-doc/bc-record.cpp
@@ -0,0 +1,178 @@
+// This test requires Linux due to the system-dependent USR for the
+// inner class in function H.
+// REQUIRES: system-linux
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: echo "" > %t/compile_flags.txt
+// RUN: cp "%s" "%t/test.cpp"
+// RUN: clang-doc --dump -doxygen -p %t %t/test.cpp -output=%t/docs
+// RUN: llvm-bcanalyzer %t/docs/bc/docs.bc --dump | FileCheck %s
+
+union A { int X; int Y; };
+
+enum B { X, Y };
+
+enum class Bc { A, B };
+
+struct C { int i; };
+
+class D {};
+
+class E {
+public:
+  E() {}
+  ~E() {}
+
+protected:
+  void ProtectedMethod();
+};
+
+void E::ProtectedMethod() {}
+
+class F : virtual private D, public E {};
+
+class X {
+  class Y {};
+};
+
+void H() {
+  class I {};
+}
+
+// CHECK: 
+// CHECK-NEXT: 
+  // CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+  // CHECK-NEXT: 
+  // CHECK-NEXT:  blob data = 'E'
+  // CHECK-NEXT:  blob data = '289584A8E0FF4178A794622A547AA622503967A1'
+  // CHECK-NEXT: 
+  // CHECK-NEXT:  blob data = '{{.*}}'
+  // CHECK-NEXT:  blob data = '289584A8E0FF4178A794622A547AA622503967A1'
+  // CHECK-NEXT: 
+// CHECK-NEXT:  blob data = 'void'
+  // CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+  // CHECK-NEXT: 
+  // CHECK-NEXT:  blob data = '~E'
+  // CHECK-NEXT:  blob data = '289584A8E0FF4178A794622A547AA622503967A1'
+  // CHECK-NEXT: 
+  // CHECK-NEXT:  blob data = '{{.*}}'
+  // CHECK-NEXT:  blob data = '289584A8E0FF4178A794622A547AA622503967A1'
+  // CHECK-NEXT: 
+// CHECK-NEXT:  blob data = 'void'
+  // CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+  // CHECK-NEXT: 
+  // CHECK-NEXT:  blob data = 'ProtectedMethod'
+  // CHECK-NEXT:  blob data = '289584A8E0FF4178A794622A547AA622503967A1'
+  // CHECK-NEXT: 
+  // CHECK-NEXT:  blob data = '{{.*}}'
+  // CHECK-NEXT:  blob data = '{{.*}}'
+  // CHECK-NEXT:  blob data = '289584A8E0FF4178A794622A547AA622503967A1'
+  // CHECK-NEXT: 
+// CHECK-NEXT:  blob data = 'void'
+  // CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+  // CHECK-NEXT: 
+  // CHECK-NEXT:  blob data = 'H'
+  // CHECK-NEXT:  blob data = '3433664532ABFCC39301646A10E768C1882BF194'
+  // CHECK-NEXT:  blob data = '{{.*}}'
+  // CHECK-NEXT: 
+// CHECK-NEXT:  blob data = 'void'
+  // CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+  // CHECK-NEXT: 
+  // CHECK-NEXT:  blob data = 'A'
+  // CHECK-NEXT:  blob data = '{{.*}}'
+  // CHECK-NEXT: 
+  // CHECK-NEXT: 
+// CHECK-NEXT:  blob data = 'int'
+// CHECK-NEXT:  blob data = 'A::X'
+// CHECK-NEXT: 
+  // CHECK-NEXT: 
+  // CHECK-NEXT: 
+// CHECK-NEXT:  blob data = 'int'
+// CHECK-NEXT:  blob data = 'A::Y'
+// CHECK-NEXT: 
+  // CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+  // CHECK-NEXT: 
+  // CHECK-NEXT:  blob data = 'C'
+  // CHECK-NEXT:  blob data = '{{.*}}'
+  // CHECK-NEXT: 
+// CHECK-NEXT:  blob data = 'int'
+// CHECK-NEXT:  blob data = 'C::i'
+// CHECK-NEXT: 
+  // CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+  // CHECK-NEXT: 
+  // CHECK-NEXT:  blob data = 'D'
+  // CHECK-NEXT:  blob data = 'E3B54702FABFF4037025BA194FC27C47006330B5'
+  // CHECK-NEXT:  blob data = '{{.*}}'
+  // CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+  // CHECK-NEXT: 
+  // CHECK-NEXT:  blob data = 'E'
+  // CHECK-NEXT:  blob data = 'E3B54702FABFF4037025BA194FC27C47006330B5'
+  // CHECK-NEXT:  blob data = 'DEB4AC1CD9253CD9EF7FBE6BCAC506D77984ABD4'
+  // CHECK-NEXT:  blob data = 'DEB4AC1CD9253CD9EF7FBE6BCAC506D77984ABD4'
+  // CHECK-NEXT:  blob data = 'BD2BDEBD423F80BACCEA75DE6D6622D355FC2D17'
+  // CHECK-NEXT:  blob data = 'BD2BDEBD423F80BACCEA75DE6D6622D355FC2D17'
+  // CHECK-NEXT:  blob data = '5093D428CDC62096A67547BA52566E4FB9404EEE'
+  // CHECK-NEXT:  blob data = '5093D428CDC62096A67547BA52566E4FB9404EEE'
+  // CHECK-NEXT:  blob data = '{{.*}}'
+  // CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+  // CHECK-NEXT: 
+  // CHECK-NEXT:  blob data = 'F'
+  // CHECK-NEXT:  blob data = '{{.*}}'
+  // CHECK-NEXT: 
+  // CHECK-NEXT:  blob data = '289584A8E0FF4178A794622A547AA622503967A1'
+  // CHECK-NEXT:  blob data = '0921737541208B8FA9BB42B60F78AC1D779AA054'
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+  // CHECK-NEXT: 
+  // CHECK-NEXT:  blob data = 'X'
+  // CHECK-NEXT:  blob data = '641AB4A3D36399954ACDE29C7A8833032BF40472'
+  // CHECK-NEXT:  b

[PATCH] D44786: Lowering x86 adds/addus/subs/subus intrinsics (clang)

[Craig Topper via Phabricator via cfe-commits]

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

LGTM with that one comment.




Comment at: lib/CodeGen/CGBuiltin.cpp:8288
+llvm::Type *ElementType = ResultType->getVectorElementType();
+llvm::Type *ExtElementType = ElementType == CGF.Builder.getInt8Ty()
+ ? CGF.Builder.getInt16Ty()

You could probably just do ResultType->getScalarSizeInBits() == 8


Repository:
  rC Clang

https://reviews.llvm.org/D44786



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

r328801 - Set dso_local when clearing dllimport.

[Rafael Espindola via cfe-commits]

Author: rafael
Date: Thu Mar 29 09:45:18 2018
New Revision: 328801

URL: http://llvm.org/viewvc/llvm-project?rev=328801&view=rev
Log:
Set dso_local when clearing dllimport.

Modified:
cfe/trunk/lib/CodeGen/CodeGenModule.cpp
cfe/trunk/test/CodeGenCXX/dllimport.cpp

Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=328801&r1=328800&r2=328801&view=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Thu Mar 29 09:45:18 2018
@@ -2398,8 +2398,10 @@ llvm::Constant *CodeGenModule::GetOrCrea
 }
 
 // Handle dropped DLL attributes.
-if (D && !D->hasAttr() && !D->hasAttr())
+if (D && !D->hasAttr() && !D->hasAttr()) {
   Entry->setDLLStorageClass(llvm::GlobalValue::DefaultStorageClass);
+  setDSOLocal(Entry);
+}
 
 // If there are two attempts to define the same mangled name, issue an
 // error.

Modified: cfe/trunk/test/CodeGenCXX/dllimport.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/dllimport.cpp?rev=328801&r1=328800&r2=328801&view=diff
==
--- cfe/trunk/test/CodeGenCXX/dllimport.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/dllimport.cpp Thu Mar 29 09:45:18 2018
@@ -203,6 +203,8 @@ USEVAR(VarTmpl)
 // Functions
 
//===--===//
 
+// GNU-DAG: declare dso_local void @_ZdlPv(i8*)
+
 // Import function declaration.
 // MSC-DAG: declare dllimport void @"?decl@@YAXXZ"()
 // GNU-DAG: declare dllimport void @_Z4declv()


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

[PATCH] D44723: Set dso_local when clearing dllimport

[Rafael Avila de Espindola via Phabricator via cfe-commits]

espindola closed this revision.
espindola added a comment.

328801


https://reviews.llvm.org/D44723



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

[PATCH] D42034: [clang-format] In tests, expected code should be format-stable

[Mark Zeren via Phabricator via cfe-commits]

mzeren-vmw updated this revision to Diff 140276.
mzeren-vmw added a comment.

Update other verifyFormat implementations.


Repository:
  rC Clang

https://reviews.llvm.org/D42034

Files:
  unittests/Format/FormatTest.cpp
  unittests/Format/FormatTestComments.cpp
  unittests/Format/FormatTestJS.cpp
  unittests/Format/FormatTestJava.cpp
  unittests/Format/FormatTestObjC.cpp
  unittests/Format/FormatTestProto.cpp
  unittests/Format/FormatTestTextProto.cpp


Index: unittests/Format/FormatTestTextProto.cpp
===
--- unittests/Format/FormatTestTextProto.cpp
+++ unittests/Format/FormatTestTextProto.cpp
@@ -36,6 +36,7 @@
   }
 
   static void verifyFormat(llvm::StringRef Code, const FormatStyle &Style) {
+EXPECT_EQ(Code.str(), format(Code, Style)) << "Expected code is not 
stable";
 EXPECT_EQ(Code.str(), format(test::messUp(Code), Style));
   }
 
Index: unittests/Format/FormatTestProto.cpp
===
--- unittests/Format/FormatTestProto.cpp
+++ unittests/Format/FormatTestProto.cpp
@@ -38,6 +38,7 @@
   }
 
   static void verifyFormat(llvm::StringRef Code) {
+EXPECT_EQ(Code.str(), format(Code)) << "Expected code is not stable";
 EXPECT_EQ(Code.str(), format(test::messUp(Code)));
   }
 };
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -58,6 +58,7 @@
   }
 
   void verifyFormat(StringRef Code) {
+EXPECT_EQ(Code.str(), format(Code)) << "Expected code is not stable";
 EXPECT_EQ(Code.str(), format(test::messUp(Code)));
   }
 
Index: unittests/Format/FormatTestJava.cpp
===
--- unittests/Format/FormatTestJava.cpp
+++ unittests/Format/FormatTestJava.cpp
@@ -46,6 +46,7 @@
   static void verifyFormat(
   llvm::StringRef Code,
   const FormatStyle &Style = getGoogleStyle(FormatStyle::LK_Java)) {
+EXPECT_EQ(Code.str(), format(Code, Style)) << "Expected code is not 
stable";
 EXPECT_EQ(Code.str(), format(test::messUp(Code), Style));
   }
 };
Index: unittests/Format/FormatTestJS.cpp
===
--- unittests/Format/FormatTestJS.cpp
+++ unittests/Format/FormatTestJS.cpp
@@ -49,14 +49,18 @@
   static void verifyFormat(
   llvm::StringRef Code,
   const FormatStyle &Style = getGoogleStyle(FormatStyle::LK_JavaScript)) {
+EXPECT_EQ(Code.str(), format(Code, Style))
+<< "Expected code is not stable";
 std::string Result = format(test::messUp(Code), Style);
 EXPECT_EQ(Code.str(), Result) << "Formatted:\n" << Result;
   }
 
   static void verifyFormat(
   llvm::StringRef Expected,
   llvm::StringRef Code,
   const FormatStyle &Style = getGoogleStyle(FormatStyle::LK_JavaScript)) {
+EXPECT_EQ(Expected.str(), format(Expected, Style))
+<< "Expected code is not stable";
 std::string Result = format(Code, Style);
 EXPECT_EQ(Expected.str(), Result) << "Formatted:\n" << Result;
   }
Index: unittests/Format/FormatTestComments.cpp
===
--- unittests/Format/FormatTestComments.cpp
+++ unittests/Format/FormatTestComments.cpp
@@ -70,6 +70,7 @@
 
   void verifyFormat(llvm::StringRef Code,
 const FormatStyle &Style = getLLVMStyle()) {
+EXPECT_EQ(Code.str(), format(Code, Style)) << "Expected code is not 
stable";
 EXPECT_EQ(Code.str(), format(test::messUp(Code), Style));
   }
 
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -72,6 +72,8 @@
 
   void verifyFormat(llvm::StringRef Expected, llvm::StringRef Code,
 const FormatStyle &Style = getLLVMStyle()) {
+EXPECT_EQ(Expected.str(), format(Expected, Style))
+<< "Expected code is not stable";
 EXPECT_EQ(Expected.str(), format(Code, Style));
 if (Style.Language == FormatStyle::LK_Cpp) {
   // Objective-C++ is a superset of C++, so everything checked for C++


Index: unittests/Format/FormatTestTextProto.cpp
===
--- unittests/Format/FormatTestTextProto.cpp
+++ unittests/Format/FormatTestTextProto.cpp
@@ -36,6 +36,7 @@
   }
 
   static void verifyFormat(llvm::StringRef Code, const FormatStyle &Style) {
+EXPECT_EQ(Code.str(), format(Code, Style)) << "Expected code is not stable";
 EXPECT_EQ(Code.str(), format(test::messUp(Code), Style));
   }
 
Index: unittests/Format/FormatTestProto.cpp
===
--- unittests/Format/FormatTestProto.cpp
+++ unittests/Format/FormatTestProto.cpp
@@ -38,6 +38,7 @@
   }
 
   static void verifyFormat(llvm::StringRef

[PATCH] D45052: Mark __cfi_check as dso_local

[Rafael Avila de Espindola via Phabricator via cfe-commits]

espindola created this revision.
espindola added reviewers: rnk, echristo.

https://reviews.llvm.org/D45052

Files:
  lib/CodeGen/CGExpr.cpp
  test/CodeGen/cfi-icall-cross-dso.c


Index: test/CodeGen/cfi-icall-cross-dso.c
===
--- test/CodeGen/cfi-icall-cross-dso.c
+++ test/CodeGen/cfi-icall-cross-dso.c
@@ -66,6 +66,9 @@
 inline void foo() {}
 void bar() { foo(); }
 
+// ITANIUM: define weak void @__cfi_check
+// MS: define weak dso_local void @__cfi_check
+
 // CHECK: !{i32 4, !"Cross-DSO CFI", i32 1}
 
 // Check that the type entries are correct.
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -3010,6 +3010,7 @@
   llvm::Function *F = llvm::Function::Create(
   llvm::FunctionType::get(VoidTy, {Int64Ty, Int8PtrTy, Int8PtrTy}, false),
   llvm::GlobalValue::WeakAnyLinkage, "__cfi_check", M);
+  CGM.setDSOLocal(F);
   llvm::BasicBlock *BB = llvm::BasicBlock::Create(Ctx, "entry", F);
   // FIXME: consider emitting an intrinsic call like
   // call void @llvm.cfi_check(i64 %0, i8* %1, i8* %2)


Index: test/CodeGen/cfi-icall-cross-dso.c
===
--- test/CodeGen/cfi-icall-cross-dso.c
+++ test/CodeGen/cfi-icall-cross-dso.c
@@ -66,6 +66,9 @@
 inline void foo() {}
 void bar() { foo(); }
 
+// ITANIUM: define weak void @__cfi_check
+// MS: define weak dso_local void @__cfi_check
+
 // CHECK: !{i32 4, !"Cross-DSO CFI", i32 1}
 
 // Check that the type entries are correct.
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -3010,6 +3010,7 @@
   llvm::Function *F = llvm::Function::Create(
   llvm::FunctionType::get(VoidTy, {Int64Ty, Int8PtrTy, Int8PtrTy}, false),
   llvm::GlobalValue::WeakAnyLinkage, "__cfi_check", M);
+  CGM.setDSOLocal(F);
   llvm::BasicBlock *BB = llvm::BasicBlock::Create(Ctx, "entry", F);
   // FIXME: consider emitting an intrinsic call like
   // call void @llvm.cfi_check(i64 %0, i8* %1, i8* %2)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37014: [clang-tidy] Add a checker to remove useless intermediate variables before return statements with comparisons

[Tristan Bourvon via Phabricator via cfe-commits]

tbourvon updated this revision to Diff 140278.
tbourvon marked 10 inline comments as done.
tbourvon added a comment.

Fixed review comments


https://reviews.llvm.org/D37014

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tidy/readability/UnnecessaryIntermediateVarCheck.cpp
  clang-tidy/readability/UnnecessaryIntermediateVarCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-unnecessary-intermediate-var.rst
  test/clang-tidy/readability-unnecessary-intermediate-var.cpp
  unittests/clang-tidy/ReadabilityModuleTest.cpp

Index: unittests/clang-tidy/ReadabilityModuleTest.cpp
===
--- unittests/clang-tidy/ReadabilityModuleTest.cpp
+++ unittests/clang-tidy/ReadabilityModuleTest.cpp
@@ -1,6 +1,8 @@
 #include "ClangTidyTest.h"
 #include "readability/BracesAroundStatementsCheck.h"
 #include "readability/NamespaceCommentCheck.h"
+#include "readability/UnnecessaryIntermediateVarCheck.h"
+#include "../../../unittests/ASTMatchers/ASTMatchersTest.h"
 #include "gtest/gtest.h"
 
 namespace clang {
@@ -500,6 +502,19 @@
 runCheckOnCode(Input));
 }
 
+TEST(StatementMatcher, HasSuccessor) {
+  using namespace ast_matchers;
+  using namespace matchers;
+
+  StatementMatcher DeclHasSuccessorReturnStmt =
+  declStmt(hasSuccessor(returnStmt()));
+
+  EXPECT_TRUE(
+  matches("void foo() { int bar; return; }", DeclHasSuccessorReturnStmt));
+  EXPECT_TRUE(notMatches("void foo() { int bar; bar++; return; }",
+ DeclHasSuccessorReturnStmt));
+}
+
 } // namespace test
 } // namespace tidy
 } // namespace clang
Index: test/clang-tidy/readability-unnecessary-intermediate-var.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-unnecessary-intermediate-var.cpp
@@ -0,0 +1,245 @@
+// RUN: %check_clang_tidy %s readability-unnecessary-intermediate-var %t
+
+bool f() {
+  auto test = 1; // Test
+  // CHECK-FIXES: {{^}}  // Test{{$}}
+  return (test == 1);
+  // CHECK-FIXES: {{^}}  return (1 == 1);{{$}}
+  // CHECK-MESSAGES: :[[@LINE-4]]:8: warning: unnecessary intermediate variable 'test' [readability-unnecessary-intermediate-var]
+  // CHECK-MESSAGES: :[[@LINE-3]]:11: note: because it is only used when returning the result of this comparison
+  // CHECK-MESSAGES: :[[@LINE-6]]:8: note: consider removing the variable declaration
+  // CHECK-MESSAGES: :[[@LINE-5]]:11: note: and directly using the variable initialization expression here
+}
+
+bool f2() {
+  auto test1 = 1; // Test1
+  // CHECK-FIXES: {{^}}  // Test1{{$}}
+  auto test2 = 2; // Test2
+  // CHECK-FIXES: {{^}}  // Test2{{$}}
+  return (test1 == test2);
+  // CHECK-FIXES: {{^}}  return (1 == 2);{{$}}
+  // CHECK-MESSAGES: :[[@LINE-6]]:8: warning: unnecessary intermediate variable 'test1' [readability-unnecessary-intermediate-var]
+  // CHECK-MESSAGES: :[[@LINE-5]]:8: warning: and so is 'test2' [readability-unnecessary-intermediate-var]
+  // CHECK-MESSAGES: :[[@LINE-4]]:11: note: because they are only used when returning the result of this comparison
+  // CHECK-MESSAGES: :[[@LINE-9]]:8: note: consider removing both this variable declaration
+  // CHECK-MESSAGES: :[[@LINE-8]]:8: note: and this one
+  // CHECK-MESSAGES: :[[@LINE-7]]:11: note: and directly using the variable initialization expressions here
+}
+
+bool f3() {
+  auto test1 = 1; // Test1
+  // CHECK-FIXES: {{^}}  // Test1{{$}}
+  auto test2 = 2; // Test2
+  return (test1 == 2);
+  // CHECK-FIXES: {{^}}  return (1 == 2);{{$}}
+  // CHECK-MESSAGES: :[[@LINE-5]]:8: warning: unnecessary intermediate variable 'test1' [readability-unnecessary-intermediate-var]
+  // CHECK-MESSAGES: :[[@LINE-3]]:11: note: because it is only used when returning the result of this comparison
+  // CHECK-MESSAGES: :[[@LINE-7]]:8: note: consider removing the variable declaration
+  // CHECK-MESSAGES: :[[@LINE-5]]:11: note: and directly using the variable initialization expression here
+}
+
+bool f4() {
+  auto test1 = 1; // Test1
+  auto test2 = 2; // Test2
+  // CHECK-FIXES: {{^}}  // Test2{{$}}
+  return (test2 == 3);
+  // CHECK-FIXES: {{^}}  return (2 == 3);{{$}}
+  // CHECK-MESSAGES: :[[@LINE-4]]:8: warning: unnecessary intermediate variable 'test2' [readability-unnecessary-intermediate-var]
+  // CHECK-MESSAGES: :[[@LINE-3]]:11: note: because it is only used when returning the result of this comparison
+  // CHECK-MESSAGES: :[[@LINE-6]]:8: note: consider removing the variable declaration
+  // CHECK-MESSAGES: :[[@LINE-5]]:11: note: and directly using the variable initialization expression here
+}
+
+bool f5() {
+  auto test1 = 1; // Test1
+  // CHECK-FIXES: {{^}}  // Test1{{$}}
+  auto test2 = 2; // Test2
+  return (2 == test1);
+  // CHECK-FIXES: {{^}}  return (1 == 2);{{$}}
+  // CHECK-MESSAGES: :[[@LINE-5]]:8: warning: unnecessary intermediate variable 'test1' [read

[PATCH] D37014: [clang-tidy] Add a checker to remove useless intermediate variables before return statements with comparisons

[Tristan Bourvon via Phabricator via cfe-commits]

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



Comment at: docs/ReleaseNotes.rst:62
 
 - New `bugprone-throw-keyword-missing
   
`_
 check

Eugene.Zelenko wrote:
> Please rebase from trunk and use //:doc:// for link.
I'm sorry, what do you mean by "use :doc: for link"?


https://reviews.llvm.org/D37014



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

[PATCH] D37014: [clang-tidy] Add a checker to remove useless intermediate variables before return statements with comparisons

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: docs/ReleaseNotes.rst:137
+  
+- New `readability-unnecessary-intermediate-var
+  
`_
 check

Please use alphabetic order and //:doc:// as well as proper link. See other 
checks as example.


https://reviews.llvm.org/D37014



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

[PATCH] D45054: Set dso_local on cfi_slowpath

[Rafael Avila de Espindola via Phabricator via cfe-commits]

espindola created this revision.
espindola added reviewers: rnk, echristo, eugenis.

https://reviews.llvm.org/D45054

Files:
  lib/CodeGen/CGExpr.cpp
  test/CodeGen/cfi-icall-cross-dso.c


Index: test/CodeGen/cfi-icall-cross-dso.c
===
--- test/CodeGen/cfi-icall-cross-dso.c
+++ test/CodeGen/cfi-icall-cross-dso.c
@@ -39,6 +39,11 @@
 // MS-DIAG: call void @__cfi_slowpath_diag(i64 4195979634929632483, i8* 
%{{.*}}, {{.*}}@[[DATA]]{{.*}}) {{.*}}, !nosanitize
 // MS-TRAP: call void @__cfi_slowpath(i64 4195979634929632483, i8* %{{.*}}) 
{{.*}}, !nosanitize
 
+// ITANIUM-DIAG: declare void @__cfi_slowpath_diag(
+// ITANIUM-TRAP: declare void @__cfi_slowpath(
+// MS-DIAG: declare dso_local void @__cfi_slowpath_diag(
+// MS-TRAP: declare dso_local void @__cfi_slowpath(
+
 void caller(void (*f)()) {
   f();
 }
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -2975,28 +2975,29 @@
   bool WithDiag = !CGM.getCodeGenOpts().SanitizeTrap.has(Kind);
 
   llvm::CallInst *CheckCall;
+  llvm::Constant *SlowPathFn;
   if (WithDiag) {
 llvm::Constant *Info = llvm::ConstantStruct::getAnon(StaticArgs);
 auto *InfoPtr =
 new llvm::GlobalVariable(CGM.getModule(), Info->getType(), false,
  llvm::GlobalVariable::PrivateLinkage, Info);
 InfoPtr->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global);
 CGM.getSanitizerMetadata()->disableSanitizerForGlobal(InfoPtr);
 
-llvm::Constant *SlowPathDiagFn = CGM.getModule().getOrInsertFunction(
+SlowPathFn = CGM.getModule().getOrInsertFunction(
 "__cfi_slowpath_diag",
 llvm::FunctionType::get(VoidTy, {Int64Ty, Int8PtrTy, Int8PtrTy},
 false));
 CheckCall = Builder.CreateCall(
-SlowPathDiagFn,
-{TypeId, Ptr, Builder.CreateBitCast(InfoPtr, Int8PtrTy)});
+SlowPathFn, {TypeId, Ptr, Builder.CreateBitCast(InfoPtr, Int8PtrTy)});
   } else {
-llvm::Constant *SlowPathFn = CGM.getModule().getOrInsertFunction(
+SlowPathFn = CGM.getModule().getOrInsertFunction(
 "__cfi_slowpath",
 llvm::FunctionType::get(VoidTy, {Int64Ty, Int8PtrTy}, false));
 CheckCall = Builder.CreateCall(SlowPathFn, {TypeId, Ptr});
   }
 
+  CGM.setDSOLocal(cast(SlowPathFn->stripPointerCasts()));
   CheckCall->setDoesNotThrow();
 
   EmitBlock(Cont);


Index: test/CodeGen/cfi-icall-cross-dso.c
===
--- test/CodeGen/cfi-icall-cross-dso.c
+++ test/CodeGen/cfi-icall-cross-dso.c
@@ -39,6 +39,11 @@
 // MS-DIAG: call void @__cfi_slowpath_diag(i64 4195979634929632483, i8* %{{.*}}, {{.*}}@[[DATA]]{{.*}}) {{.*}}, !nosanitize
 // MS-TRAP: call void @__cfi_slowpath(i64 4195979634929632483, i8* %{{.*}}) {{.*}}, !nosanitize
 
+// ITANIUM-DIAG: declare void @__cfi_slowpath_diag(
+// ITANIUM-TRAP: declare void @__cfi_slowpath(
+// MS-DIAG: declare dso_local void @__cfi_slowpath_diag(
+// MS-TRAP: declare dso_local void @__cfi_slowpath(
+
 void caller(void (*f)()) {
   f();
 }
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -2975,28 +2975,29 @@
   bool WithDiag = !CGM.getCodeGenOpts().SanitizeTrap.has(Kind);
 
   llvm::CallInst *CheckCall;
+  llvm::Constant *SlowPathFn;
   if (WithDiag) {
 llvm::Constant *Info = llvm::ConstantStruct::getAnon(StaticArgs);
 auto *InfoPtr =
 new llvm::GlobalVariable(CGM.getModule(), Info->getType(), false,
  llvm::GlobalVariable::PrivateLinkage, Info);
 InfoPtr->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global);
 CGM.getSanitizerMetadata()->disableSanitizerForGlobal(InfoPtr);
 
-llvm::Constant *SlowPathDiagFn = CGM.getModule().getOrInsertFunction(
+SlowPathFn = CGM.getModule().getOrInsertFunction(
 "__cfi_slowpath_diag",
 llvm::FunctionType::get(VoidTy, {Int64Ty, Int8PtrTy, Int8PtrTy},
 false));
 CheckCall = Builder.CreateCall(
-SlowPathDiagFn,
-{TypeId, Ptr, Builder.CreateBitCast(InfoPtr, Int8PtrTy)});
+SlowPathFn, {TypeId, Ptr, Builder.CreateBitCast(InfoPtr, Int8PtrTy)});
   } else {
-llvm::Constant *SlowPathFn = CGM.getModule().getOrInsertFunction(
+SlowPathFn = CGM.getModule().getOrInsertFunction(
 "__cfi_slowpath",
 llvm::FunctionType::get(VoidTy, {Int64Ty, Int8PtrTy}, false));
 CheckCall = Builder.CreateCall(SlowPathFn, {TypeId, Ptr});
   }
 
+  CGM.setDSOLocal(cast(SlowPathFn->stripPointerCasts()));
   CheckCall->setDoesNotThrow();
 
   EmitBlock(Cont);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r328807 - [Sema] Make deprecation fix-it replace all multi-parameter ObjC method slots.

[Volodymyr Sapsai via cfe-commits]

Author: vsapsai
Date: Thu Mar 29 10:34:09 2018
New Revision: 328807

URL: http://llvm.org/viewvc/llvm-project?rev=328807&view=rev
Log:
[Sema] Make deprecation fix-it replace all multi-parameter ObjC method slots.

Deprecation replacement can be any text but if it looks like a name of
ObjC method and has the same number of arguments as original method,
replace all slot names so after applying a fix-it you have valid code.

rdar://problem/36660853

Reviewers: aaron.ballman, erik.pilkington, rsmith

Reviewed By: erik.pilkington

Subscribers: cfe-commits, jkorous-apple

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


Added:
cfe/trunk/test/SemaObjC/attr-deprecated-replacement-fixit.m
Modified:
cfe/trunk/include/clang/Basic/CharInfo.h
cfe/trunk/include/clang/Sema/DelayedDiagnostic.h
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/DelayedDiagnostic.cpp
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Sema/SemaExprObjC.cpp
cfe/trunk/unittests/Basic/CharInfoTest.cpp

Modified: cfe/trunk/include/clang/Basic/CharInfo.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/CharInfo.h?rev=328807&r1=328806&r2=328807&view=diff
==
--- cfe/trunk/include/clang/Basic/CharInfo.h (original)
+++ cfe/trunk/include/clang/Basic/CharInfo.h Thu Mar 29 10:34:09 2018
@@ -180,14 +180,15 @@ LLVM_READONLY inline char toUppercase(ch
 
 /// Return true if this is a valid ASCII identifier.
 ///
-/// Note that this is a very simple check; it does not accept '$' or UCNs as
-/// valid identifier characters.
-LLVM_READONLY inline bool isValidIdentifier(StringRef S) {
-  if (S.empty() || !isIdentifierHead(S[0]))
+/// Note that this is a very simple check; it does not accept UCNs as valid
+/// identifier characters.
+LLVM_READONLY inline bool isValidIdentifier(StringRef S,
+bool AllowDollar = false) {
+  if (S.empty() || !isIdentifierHead(S[0], AllowDollar))
 return false;
 
   for (StringRef::iterator I = S.begin(), E = S.end(); I != E; ++I)
-if (!isIdentifierBody(*I))
+if (!isIdentifierBody(*I, AllowDollar))
   return false;
 
   return true;

Modified: cfe/trunk/include/clang/Sema/DelayedDiagnostic.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/DelayedDiagnostic.h?rev=328807&r1=328806&r2=328807&view=diff
==
--- cfe/trunk/include/clang/Sema/DelayedDiagnostic.h (original)
+++ cfe/trunk/include/clang/Sema/DelayedDiagnostic.h Thu Mar 29 10:34:09 2018
@@ -31,6 +31,7 @@
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/Specifiers.h"
 #include "clang/Sema/Sema.h"
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Casting.h"
@@ -138,7 +139,7 @@ public:
   void Destroy();
 
   static DelayedDiagnostic makeAvailability(AvailabilityResult AR,
-SourceLocation Loc,
+ArrayRef Locs,
 const NamedDecl *ReferringDecl,
 const NamedDecl *OffendingDecl,
 const ObjCInterfaceDecl 
*UnknownObjCClass,
@@ -146,7 +147,6 @@ public:
 StringRef Msg,
 bool ObjCPropertyAccess);
 
-
   static DelayedDiagnostic makeAccess(SourceLocation Loc,
   const AccessedEntity &Entity) {
 DelayedDiagnostic DD;
@@ -194,6 +194,12 @@ public:
 return StringRef(AvailabilityData.Message, AvailabilityData.MessageLen);
   }
 
+  ArrayRef getAvailabilitySelectorLocs() const {
+assert(Kind == Availability && "Not an availability diagnostic.");
+return llvm::makeArrayRef(AvailabilityData.SelectorLocs,
+  AvailabilityData.NumSelectorLocs);
+  }
+
   AvailabilityResult getAvailabilityResult() const {
 assert(Kind == Availability && "Not an availability diagnostic.");
 return AvailabilityData.AR;
@@ -238,6 +244,8 @@ private:
 const ObjCPropertyDecl  *ObjCProperty;
 const char *Message;
 size_t MessageLen;
+SourceLocation *SelectorLocs;
+size_t NumSelectorLocs;
 AvailabilityResult AR;
 bool ObjCPropertyAccess;
   };

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=328807&r1=328806&r2=328807&view=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Thu Mar 29 10:34:09 2018
@@ -3947,7 +3947,7 @@ public:
 
   void redelayDiagnostics(sema::DelayedDiagnosticPool &pool);
 
- 

[PATCH] D44589: [Sema] Make deprecation fix-it replace all multi-parameter ObjC method slots.

[Volodymyr Sapsai via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC328807: [Sema] Make deprecation fix-it replace all 
multi-parameter ObjC method slots. (authored by vsapsai, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D44589?vs=139981&id=140287#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D44589

Files:
  include/clang/Basic/CharInfo.h
  include/clang/Sema/DelayedDiagnostic.h
  include/clang/Sema/Sema.h
  lib/Sema/DelayedDiagnostic.cpp
  lib/Sema/SemaDeclAttr.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaExprObjC.cpp
  test/SemaObjC/attr-deprecated-replacement-fixit.m
  unittests/Basic/CharInfoTest.cpp

Index: include/clang/Sema/DelayedDiagnostic.h
===
--- include/clang/Sema/DelayedDiagnostic.h
+++ include/clang/Sema/DelayedDiagnostic.h
@@ -31,6 +31,7 @@
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/Specifiers.h"
 #include "clang/Sema/Sema.h"
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Casting.h"
@@ -138,15 +139,14 @@
   void Destroy();
 
   static DelayedDiagnostic makeAvailability(AvailabilityResult AR,
-SourceLocation Loc,
+ArrayRef Locs,
 const NamedDecl *ReferringDecl,
 const NamedDecl *OffendingDecl,
 const ObjCInterfaceDecl *UnknownObjCClass,
 const ObjCPropertyDecl  *ObjCProperty,
 StringRef Msg,
 bool ObjCPropertyAccess);
 
-
   static DelayedDiagnostic makeAccess(SourceLocation Loc,
   const AccessedEntity &Entity) {
 DelayedDiagnostic DD;
@@ -194,6 +194,12 @@
 return StringRef(AvailabilityData.Message, AvailabilityData.MessageLen);
   }
 
+  ArrayRef getAvailabilitySelectorLocs() const {
+assert(Kind == Availability && "Not an availability diagnostic.");
+return llvm::makeArrayRef(AvailabilityData.SelectorLocs,
+  AvailabilityData.NumSelectorLocs);
+  }
+
   AvailabilityResult getAvailabilityResult() const {
 assert(Kind == Availability && "Not an availability diagnostic.");
 return AvailabilityData.AR;
@@ -238,6 +244,8 @@
 const ObjCPropertyDecl  *ObjCProperty;
 const char *Message;
 size_t MessageLen;
+SourceLocation *SelectorLocs;
+size_t NumSelectorLocs;
 AvailabilityResult AR;
 bool ObjCPropertyAccess;
   };
Index: include/clang/Sema/Sema.h
===
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -3947,7 +3947,7 @@
 
   void redelayDiagnostics(sema::DelayedDiagnosticPool &pool);
 
-  void DiagnoseAvailabilityOfDecl(NamedDecl *D, SourceLocation Loc,
+  void DiagnoseAvailabilityOfDecl(NamedDecl *D, ArrayRef Locs,
   const ObjCInterfaceDecl *UnknownObjCClass,
   bool ObjCPropertyAccess,
   bool AvoidPartialAvailabilityChecks = false);
@@ -3962,7 +3962,7 @@
   // Expression Parsing Callbacks: SemaExpr.cpp.
 
   bool CanUseDecl(NamedDecl *D, bool TreatUnavailableAsInvalid);
-  bool DiagnoseUseOfDecl(NamedDecl *D, SourceLocation Loc,
+  bool DiagnoseUseOfDecl(NamedDecl *D, ArrayRef Locs,
  const ObjCInterfaceDecl *UnknownObjCClass = nullptr,
  bool ObjCPropertyAccess = false,
  bool AvoidPartialAvailabilityChecks = false);
Index: include/clang/Basic/CharInfo.h
===
--- include/clang/Basic/CharInfo.h
+++ include/clang/Basic/CharInfo.h
@@ -180,14 +180,15 @@
 
 /// Return true if this is a valid ASCII identifier.
 ///
-/// Note that this is a very simple check; it does not accept '$' or UCNs as
-/// valid identifier characters.
-LLVM_READONLY inline bool isValidIdentifier(StringRef S) {
-  if (S.empty() || !isIdentifierHead(S[0]))
+/// Note that this is a very simple check; it does not accept UCNs as valid
+/// identifier characters.
+LLVM_READONLY inline bool isValidIdentifier(StringRef S,
+bool AllowDollar = false) {
+  if (S.empty() || !isIdentifierHead(S[0], AllowDollar))
 return false;
 
   for (StringRef::iterator I = S.begin(), E = S.end(); I != E; ++I)
-if (!isIdentifierBody(*I))
+if (!isIdentifierBody(*I, AllowDollar))
   return false;
 
   return true;
Index: test/SemaObjC/attr-deprecated-replacement-fixit.m
===
--- test/SemaObjC/attr-deprecated-replacement-fixit.m
+++ test/SemaObjC/attr-deprec

[PATCH] D44589: [Sema] Make deprecation fix-it replace all multi-parameter ObjC method slots.

[Volodymyr Sapsai via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL328807: [Sema] Make deprecation fix-it replace all 
multi-parameter ObjC method slots. (authored by vsapsai, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D44589?vs=139981&id=140286#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D44589

Files:
  cfe/trunk/include/clang/Basic/CharInfo.h
  cfe/trunk/include/clang/Sema/DelayedDiagnostic.h
  cfe/trunk/include/clang/Sema/Sema.h
  cfe/trunk/lib/Sema/DelayedDiagnostic.cpp
  cfe/trunk/lib/Sema/SemaDeclAttr.cpp
  cfe/trunk/lib/Sema/SemaExpr.cpp
  cfe/trunk/lib/Sema/SemaExprObjC.cpp
  cfe/trunk/test/SemaObjC/attr-deprecated-replacement-fixit.m
  cfe/trunk/unittests/Basic/CharInfoTest.cpp

Index: cfe/trunk/include/clang/Basic/CharInfo.h
===
--- cfe/trunk/include/clang/Basic/CharInfo.h
+++ cfe/trunk/include/clang/Basic/CharInfo.h
@@ -180,14 +180,15 @@
 
 /// Return true if this is a valid ASCII identifier.
 ///
-/// Note that this is a very simple check; it does not accept '$' or UCNs as
-/// valid identifier characters.
-LLVM_READONLY inline bool isValidIdentifier(StringRef S) {
-  if (S.empty() || !isIdentifierHead(S[0]))
+/// Note that this is a very simple check; it does not accept UCNs as valid
+/// identifier characters.
+LLVM_READONLY inline bool isValidIdentifier(StringRef S,
+bool AllowDollar = false) {
+  if (S.empty() || !isIdentifierHead(S[0], AllowDollar))
 return false;
 
   for (StringRef::iterator I = S.begin(), E = S.end(); I != E; ++I)
-if (!isIdentifierBody(*I))
+if (!isIdentifierBody(*I, AllowDollar))
   return false;
 
   return true;
Index: cfe/trunk/include/clang/Sema/DelayedDiagnostic.h
===
--- cfe/trunk/include/clang/Sema/DelayedDiagnostic.h
+++ cfe/trunk/include/clang/Sema/DelayedDiagnostic.h
@@ -31,6 +31,7 @@
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/Specifiers.h"
 #include "clang/Sema/Sema.h"
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Casting.h"
@@ -138,15 +139,14 @@
   void Destroy();
 
   static DelayedDiagnostic makeAvailability(AvailabilityResult AR,
-SourceLocation Loc,
+ArrayRef Locs,
 const NamedDecl *ReferringDecl,
 const NamedDecl *OffendingDecl,
 const ObjCInterfaceDecl *UnknownObjCClass,
 const ObjCPropertyDecl  *ObjCProperty,
 StringRef Msg,
 bool ObjCPropertyAccess);
 
-
   static DelayedDiagnostic makeAccess(SourceLocation Loc,
   const AccessedEntity &Entity) {
 DelayedDiagnostic DD;
@@ -194,6 +194,12 @@
 return StringRef(AvailabilityData.Message, AvailabilityData.MessageLen);
   }
 
+  ArrayRef getAvailabilitySelectorLocs() const {
+assert(Kind == Availability && "Not an availability diagnostic.");
+return llvm::makeArrayRef(AvailabilityData.SelectorLocs,
+  AvailabilityData.NumSelectorLocs);
+  }
+
   AvailabilityResult getAvailabilityResult() const {
 assert(Kind == Availability && "Not an availability diagnostic.");
 return AvailabilityData.AR;
@@ -238,6 +244,8 @@
 const ObjCPropertyDecl  *ObjCProperty;
 const char *Message;
 size_t MessageLen;
+SourceLocation *SelectorLocs;
+size_t NumSelectorLocs;
 AvailabilityResult AR;
 bool ObjCPropertyAccess;
   };
Index: cfe/trunk/include/clang/Sema/Sema.h
===
--- cfe/trunk/include/clang/Sema/Sema.h
+++ cfe/trunk/include/clang/Sema/Sema.h
@@ -3947,7 +3947,7 @@
 
   void redelayDiagnostics(sema::DelayedDiagnosticPool &pool);
 
-  void DiagnoseAvailabilityOfDecl(NamedDecl *D, SourceLocation Loc,
+  void DiagnoseAvailabilityOfDecl(NamedDecl *D, ArrayRef Locs,
   const ObjCInterfaceDecl *UnknownObjCClass,
   bool ObjCPropertyAccess,
   bool AvoidPartialAvailabilityChecks = false);
@@ -3962,7 +3962,7 @@
   // Expression Parsing Callbacks: SemaExpr.cpp.
 
   bool CanUseDecl(NamedDecl *D, bool TreatUnavailableAsInvalid);
-  bool DiagnoseUseOfDecl(NamedDecl *D, SourceLocation Loc,
+  bool DiagnoseUseOfDecl(NamedDecl *D, ArrayRef Locs,
  const ObjCInterfaceDecl *UnknownObjCClass = nullptr,
  bool ObjCPropertyAccess = false,
  bool AvoidPartialAvailabilityChec

[PATCH] D44882: [clangd] Implementation of workspace/symbol request

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

This is fantastic, really looking forward to using it.
I'm going to need to give up on vim at this rate, or do some serious work on 
ycm.

Most significant comments are around the new functions in SourceLocation, and 
whether we can keep the type-mapping restricted to the LSP layer.




Comment at: clangd/ClangdLSPServer.cpp:99
   
Params.capabilities.textDocument.completion.completionItem.snippetSupport;
+  if (Params.capabilities.workspace && Params.capabilities.workspace->symbol &&
+  Params.capabilities.workspace->symbol->symbolKind)

malaperle wrote:
> sammccall wrote:
> > This is the analogue to the one on `completion` that we currently ignore, 
> > and one on `symbol` corresponding to the `documentSymbol` call which isn't 
> > implemented yet.
> > 
> > I think the value of the extended types is low and it might be worth 
> > leaving out of the first version to keep complexity down.
> > (If we really want it, the supporting code (mapping tables) should probably 
> > be in a place where it can be reused by `documentSymbol` and can live next 
> > to the equivalent for `completion`...)
> I think having Struct and EnumMember is nice and for sure once Macros is 
> there we will want to use it. So would it be OK to move to FindSymbols.cpp 
> (the new common file for document/symbol and workspace/symbol)?
> I think having Struct and EnumMember is nice and for sure once Macros is 
> there we will want to use it.
OK, fair enough.

I think what bothers me about this option is how deep it goes - this isn't 
really a request option in any useful sense.
I'd suggest the "C++ layer" i.e. ClangdServer should always return 
full-fidelity results, and the "LSP layer" should deal with folding them.

In which case adjustKindToCapability should live somewhere like protocol.h and 
get called from this file - WDYT?



Comment at: clangd/ClangdServer.h:161
 
+  void workspaceSymbols(StringRef Query,
+const clangd::WorkspaceSymbolOptions &Opts,

nit: add a comment for this API, e.g. `/// Retrieve the top symbols from the 
workspace matching a query`



Comment at: clangd/FindSymbols.cpp:28
+  // All clients should support those.
+  if (Kind >= SymbolKind::File && Kind <= SymbolKind::Array)
+return Kind;

Can we rather have this condition checked when the client sets the supported 
symbols, and remove this special knowledge here?



Comment at: clangd/FindSymbols.cpp:32
+  if (supportedSymbolKinds &&
+  std::find(supportedSymbolKinds->begin(), supportedSymbolKinds->end(),
+Kind) != supportedSymbolKinds->end())

this seems gratuituously inefficient - I guess it doesn't matter that much, but 
can't we store the supported kinds as a std::bitset or so?



Comment at: clangd/FindSymbols.cpp:36
+
+  // Provide some fall backs for common kinds that are close enough.
+  if (Kind == SymbolKind::Struct)

nit: if we think this will be extended, a switch might be clearer



Comment at: clangd/FindSymbols.cpp:42
+
+  if (!supportedSymbolKinds) {
+// Provide some sensible default when all fails.

This code shouldn't need to handle this case. The LSP specifies the default set 
of supported types if it's not provided, so ClangdLSPServer should enure we 
always have a valid set



Comment at: clangd/FindSymbols.cpp:114
+  std::vector Result;
+  if (Query.empty() || !Index)
+return Result;

why Query.empty()?
In practice maybe showing results before you type is bad UX, but that seems up 
to the editor to decide.



Comment at: clangd/FindSymbols.cpp:117
+
+  auto Names = splitQualifiedName(Query);
+

nit: move this below the next "paragraph" where it's used?



Comment at: clangd/FindSymbols.cpp:130
+
+  // Global scope is represented by "" in FuzzyFind.
+  if (Names.first.startswith("::"))

nit: this is `Names.first.consume_front("::")`
Might be clearer as "FuzzyFind doesn't want leading :: qualifier" to avoid 
implication, that global namespace is special, particularly because...



Comment at: clangd/FindSymbols.cpp:130
+
+  // Global scope is represented by "" in FuzzyFind.
+  if (Names.first.startswith("::"))

sammccall wrote:
> nit: this is `Names.first.consume_front("::")`
> Might be clearer as "FuzzyFind doesn't want leading :: qualifier" to avoid 
> implication, that global namespace is special, particularly because...
actually the global namespace *is* special :-)

IMO If you type `Foo`, you should get results from any namespace, but if you 
type `::Foo` you should only get results from the global namespace.

So something like:
```
if (Names.first.consume_front("::") || !Names.first.empty())
  Req.Scopes = {Names.first};
```
thou

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 140288.
Charusso added a comment.

Fixed the unseen diff issues, removed redundant parentheses, the class got a 
private part.


https://reviews.llvm.org/D45050

Files:
  clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
  clang-tidy/bugprone/NotNullTerminatedResultCheck.h
  docs/ReleaseNotes.rst


Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -65,9 +65,6 @@
   Finds function calls where ``strlen`` or ``wcslen`` passed as argument and
   cause a not null-terminated result.
 
-- The 'misc-incorrect-roundings' check was renamed to 
`bugprone-incorrect-roundings
-  
`_
-
 - New module ``zircon`` for checks related to Fuchsia's Zircon kernel.
 
 - New `bugprone-throw-keyword-missing
Index: clang-tidy/bugprone/NotNullTerminatedResultCheck.h
===
--- clang-tidy/bugprone/NotNullTerminatedResultCheck.h
+++ clang-tidy/bugprone/NotNullTerminatedResultCheck.h
@@ -27,6 +27,7 @@
   : ClangTidyCheck(Name, Context) {}
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+private:
   void memAllocFuncFix(StringRef Name,
const ast_matchers::MatchFinder::MatchResult &Result);
   void memHandlerFuncFix(StringRef Name,
Index: clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
===
--- clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
+++ clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
@@ -1,3 +1,12 @@
+//===- NotNullTerminatedResultCheck.cpp - clang-tidy-*- C++ 
-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
 #include "NotNullTerminatedResultCheck.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
@@ -49,7 +58,7 @@
 const auto StrLenArgStr = exprToStr(
 Result.Nodes.getNodeAs("length-call")->getArg(0), Result);
 
-return (StrLenArgStr == LHSStr || StrLenArgStr == RHSStr);
+return StrLenArgStr == LHSStr || StrLenArgStr == RHSStr;
   }
   return false;
 }


Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -65,9 +65,6 @@
   Finds function calls where ``strlen`` or ``wcslen`` passed as argument and
   cause a not null-terminated result.
 
-- The 'misc-incorrect-roundings' check was renamed to `bugprone-incorrect-roundings
-  `_
-
 - New module ``zircon`` for checks related to Fuchsia's Zircon kernel.
 
 - New `bugprone-throw-keyword-missing
Index: clang-tidy/bugprone/NotNullTerminatedResultCheck.h
===
--- clang-tidy/bugprone/NotNullTerminatedResultCheck.h
+++ clang-tidy/bugprone/NotNullTerminatedResultCheck.h
@@ -27,6 +27,7 @@
   : ClangTidyCheck(Name, Context) {}
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+private:
   void memAllocFuncFix(StringRef Name,
const ast_matchers::MatchFinder::MatchResult &Result);
   void memHandlerFuncFix(StringRef Name,
Index: clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
===
--- clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
+++ clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
@@ -1,3 +1,12 @@
+//===- NotNullTerminatedResultCheck.cpp - clang-tidy-*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
 #include "NotNullTerminatedResultCheck.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
@@ -49,7 +58,7 @@
 const auto StrLenArgStr = exprToStr(
 Result.Nodes.getNodeAs("length-call")->getArg(0), Result);
 
-return (StrLenArgStr == LHSStr || StrLenArgStr == RHSStr);
+return StrLenArgStr == LHSStr || StrLenArgStr == RHSStr;
   }
   return false;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

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

xazax.hun added a comment.

you should always upload the whole diff, not just the last changes.


https://reviews.llvm.org/D45050



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

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tidy/bugprone/NotNullTerminatedResultCheck.h:30
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+private:
   void memAllocFuncFix(StringRef Name,

Please insert empty line above.


https://reviews.llvm.org/D45050



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

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 140294.
Charusso marked 5 inline comments as done.
Charusso added a comment.

Added the whole diff.


https://reviews.llvm.org/D45050

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
  clang-tidy/bugprone/NotNullTerminatedResultCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-not-null-terminated-result-strlen-before-cxx11.cpp
  test/clang-tidy/bugprone-not-null-terminated-result-strlen-cxx11.cpp
  test/clang-tidy/bugprone-not-null-terminated-result-wcslen-before-cxx11.cpp
  test/clang-tidy/bugprone-not-null-terminated-result-wcslen-cxx11.cpp

Index: test/clang-tidy/bugprone-not-null-terminated-result-wcslen-cxx11.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-not-null-terminated-result-wcslen-cxx11.cpp
@@ -0,0 +1,153 @@
+// RUN: %check_clang_tidy %s bugprone-not-null-terminated-result %t -- -- -std=c++11
+
+typedef unsigned int size_t;
+typedef int errno_t;
+size_t wcslen(const wchar_t *);
+wchar_t *wcschr(const wchar_t *, int);
+errno_t *wcsncpy_s(wchar_t *, const wchar_t *, size_t);
+
+void *alloca(size_t);
+void *calloc(size_t, size_t);
+void *malloc(size_t);
+void *realloc(void *, size_t);
+
+void *wmemcpy(void *, const void *, size_t);
+errno_t wmemcpy_s(void *, size_t, const void *, size_t);
+void *wmemchr(const void *, int, size_t);
+void *wmemmove(void *, const void *, size_t);
+errno_t wmemmove_s(void *, size_t, const void *, size_t);
+void *wmemset(void *, int, size_t);
+
+int wcsncmp(const wchar_t *, const wchar_t *, size_t);
+size_t wcsxfrm(wchar_t *, const wchar_t *, size_t);
+
+namespace std {
+using ::wcsncpy_s;
+using ::wmemcpy;
+using ::wcslen;
+}
+
+void bad_alloca(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)alloca(wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 'alloca' function's allocated memory cannot hold the null-terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: alloca(wcslen(src) + 1);
+}
+
+void good_alloca(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)alloca(wcslen(src) + 1);
+}
+
+void bad_calloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)calloc(wcslen(src), sizeof(wchar_t));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 'calloc' function's allocated memory cannot hold the null-terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: calloc((wcslen(src) + 1), sizeof(wchar_t));
+}
+
+void good_calloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)calloc((wcslen(src) + 1), sizeof(wchar_t));
+}
+
+void bad_malloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)malloc(wcslen(src) * 2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 'malloc' function's allocated memory cannot hold the null-terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: malloc((wcslen(src) * 2) + 1);
+}
+
+void good_malloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)malloc((wcslen(src) * 2) + 1);
+}
+
+void bad_realloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)realloc(dest, (wcslen(dest) + wcslen(src)));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 'realloc' function's allocated memory cannot hold the null-terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: realloc(dest, (wcslen(dest) + (wcslen(src) + 1)));
+}
+
+void good_realloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)realloc(dest, (wcslen(dest) + (wcslen(src) + 1)));
+}
+
+void bad_wmemcpy(wchar_t *dest, const wchar_t *src) {
+  std::wmemcpy(dest, src, std::wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'wmemcpy' function's result is not null-terminated [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: std::wcsncpy_s(dest, src, std::wcslen(src));
+}
+
+void good_wmemcpy_cxx11(wchar_t *dest, const wchar_t *src) {
+  std::wcsncpy_s(dest, src, std::wcslen(src));
+}
+
+void bad_wmemcpy_s(wchar_t *dest, const wchar_t *src) {
+  wmemcpy_s(dest, wcslen(dest), src, wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'wmemcpy_s' function's result is not null-terminated [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: wcsncpy_s(dest, src, wcslen(src));
+}
+
+void good_wmemcpy_s(wchar_t *dest, const wchar_t *src) {
+  wcsncpy_s(dest, src, wcslen(src));
+}
+
+void bad_wmemchr(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)wmemchr(src, '\0', wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 'wmemchr' function's result is not null-terminated [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: wcschr(src, '\0');
+}
+
+void good_wmemchr(wchar_t *dest, const wchar_t *src) {
+  dest = wcschr(src, '\0');
+}
+
+void bad_wmemmove(wchar_t *dest, const wchar_t *src) {
+  wmemmove(dest, src, wcslen(src));
+  // CHECK-MESSAGES

[PATCH] D44882: [clangd] Implementation of workspace/symbol request

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

malaperle added inline comments.



Comment at: clangd/FindSymbols.cpp:31
+
+  if (supportedSymbolKinds &&
+  std::find(supportedSymbolKinds->begin(), supportedSymbolKinds->end(),

MaskRay wrote:
> This std::find loop adds some overhead to each candidate... In my experience 
> the client usually doesn't care about the returned symbol kinds, they are 
> used to give a category name. You can always patch the upstream to add 
> missing categories.
> 
> This is one instance where LSP is over specified. nvm I don't have strong 
> opinion here
I have a client that throws an exception when the symbolkind is not known and 
the whole request fails, so I think it's worth checking. But if it's too slow I 
can look at making it faster. Unfortunately, I cannot patch any upstream 
project :)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44882



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

[PATCH] D44720: [clangd] Simplify fuzzy matcher (sequence alignment) by removing some condition checks.

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clangd/FuzzyMatch.cpp:96
 return None;
   return ScoreScale * std::min(PerfectBonus * PatN, std::max(0, Best));
 }

MaskRay wrote:
> sammccall wrote:
> > MaskRay wrote:
> > > I also don't understand why it clamps the value to zero here. Negative 
> > > values are also meaningful to me. Given that perfectBonus is only 3 it is 
> > > very easy to get a negative value here.
> > An important part of the contract of `match()` is that it returns a value 
> > in `[0,1]`.
> > We rely on this range to combine this with other scoring signals - we 
> > multiply this by a quality signal in code completion.
> > (Currently the quality signal is just derived from Sema, but the global 
> > index will provide the number of uses).
> > 
> > It would be possible to use a different squash function here, but I found 
> > max(kFloor,x) worked well for the examples I looked at - anything <= some 
> > floor value was "not really a useful match at all", and most of the 
> > variance below the floor seemed to be noise to me.
> > (Then I tuned the bonuses/penalties so the floor was at zero)
> We could try other criteria in the future. I believe the current one can be 
> improved because negative scores may be returned but the scoring shouldn't 
> return 0 for all the cases.
Sure, we can try other things, and to gather more data.
(To be clear though - with the data I *did* look at, including the scores <0 
did not add more information, only noise)



Comment at: clangd/FuzzyMatch.cpp:230
 void FuzzyMatcher::buildGraph() {
+  Scores[0][0][Miss] = Scores[0][0][Match] = {0, Miss};
   for (int W = 0; W < WordN; ++W) {

MaskRay wrote:
> sammccall wrote:
> > why this change?
> > this has also been moved from the cheaper constructor to the more expensive 
> > per-match call. (also the diagonal assignment added in the next loop)
> > 
> > Also, shouldn't [0][0][Match] be AwfulScore?
> > 
> "The more expensive per-match call" is just two value assignments.
> 
> I have removed the expensive table initialization in the constructor.
> 
> [0][0][*] can be any value.
> "The more expensive per-match call" is just two value assignments.
Oops, sorry - by "more expensive" I mean "called thousands of times more often".

> I have removed the expensive table initialization in the constructor.
I don't want to be rude, but I asked why you changed this, and you didn't 
answer. Unless there's a strong reason, I'd prefer to revert this change, as I 
find this harder to reason about.
(Roughly: in the old version of the code, any data that didn't need to change 
for the life of the object was initialized in the constructor. That way I 
didn't need to worry what was performance-critical and what wasn't - match() 
only did what was strictly necessary).

> [0][0][*] can be any value.
Can you please explain why?



Comment at: clangd/FuzzyMatch.cpp:325
+  int W = PatN;
+  for (int I = PatN; ++I <= WordN; )
+if (Scores[PatN][I][Match].Score > Scores[PatN][W][Match].Score)

MaskRay wrote:
> sammccall wrote:
> > nit: I -> P, move increment to the increment expression of the for loop?
> I -> P.
> 
> > move increment to the increment expression of the for loop?
> 
> Not sure about the coding standard here, but if you insist I'll have to 
> change it as you are the reviewer. If the loop variable was an iterator, `for 
> (It I = std::next(...); I != E; ++I)` would be uglier than `for (It I = ...; 
> ++I != E; )`
Uglier is subjective, but side-effects in the condition of a for-loop is 
sufficiently unusual and surprising that I'd prefer to avoid it in both cases.



Comment at: clangd/FuzzyMatch.cpp:340
+  A[WordN] = Miss;
+  for (int I = WordN, P = PatN; I > 0; I--) {
+if (I == W)

MaskRay wrote:
> sammccall wrote:
> > sammccall wrote:
> > > sammccall wrote:
> > > > W is the right name in this file for a variable iterating over word 
> > > > indices, please don't change this.
> > > > The new variable above could be EndW or so?
> > > As far as I can see, this loop is setting `A[W+1:...] = Miss` and 
> > > populating `A[0...W]` with the exsting logic.
> > > I think this would be clearer as two loops, currently there's a lot of 
> > > conditionals around Last that obscure what's actually happening.
> > You've shifted P (and the old W, new I) by 1. This does reduce the number 
> > of +1 and -1 in this function, but it's inconsistent with how these are 
> > used elsewhere: P should be the index into Pat of the character that we're 
> > considering.
> I don't understand the rationale not to use the shifted indices. The code 
> actually use `Scores[P][W][*]` to mean the optimal match of the first `P` 
> characters of the pattern with the first `W` characters of the word, not the 
> position of the character.
> 
> On the other hand, C++ reverse iterators use the shifted one for `for (I = 
> rend()

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp:21
+
+std::string exprToStr(const Expr *Expr,
+  const MatchFinder::MatchResult &Result) {

Please make it static. Same for other functions.



Comment at: clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp:56
+  FuncExpr->getArg(ArgPos)->IgnoreParenCasts())) {
+const auto LHSStr = exprToStr(BinOp->getLHS()->IgnoreParens(), Result);
+const auto RHSStr = exprToStr(BinOp->getRHS()->IgnoreParens(), Result);

Please don't use auto when type could not be deduced from statement itself 
(new, cast, iterator). Same for other places.



Comment at: docs/ReleaseNotes.rst:62
 
+- New `bugprone-not-null-terminated-result
+  
`_
 check

Please rebase for trunk, place check in alphabetical order in new checks list, 
use //:doc:// and proper link.



Comment at: docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst:4
+bugprone-not-null-terminated-result
+=
+

Is length same as length of title?



Comment at: docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst:6
+
+This check can be used to find function calls where ``strlen`` or ``wcslen``
+are passed as an argument and cause a not null-terminated result. Depending on

Please synchronize with statement in Release Notes.



Comment at: docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst:29
+
+In addition to issuing warnings, "Fix-it" rewrite all the necessary code
+depending on the version number. The upper code would be the following:

Please use fix-it. Same for other places.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45050



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

[PATCH] D44720: [clangd] Simplify fuzzy matcher (sequence alignment) by removing some condition checks.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 140297.
MaskRay added a comment.

missScore -> missPenalty


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44720

Files:
  clangd/FuzzyMatch.cpp
  clangd/FuzzyMatch.h

Index: clangd/FuzzyMatch.h
===
--- clangd/FuzzyMatch.h
+++ clangd/FuzzyMatch.h
@@ -58,8 +58,8 @@
   void buildGraph();
   void calculateRoles(const char *Text, CharRole *Out, int &Types, int N);
   bool allowMatch(int P, int W) const;
-  int skipPenalty(int W, Action Last) const;
-  int matchBonus(int P, int W, Action Last) const;
+  int missPenalty(int W, Action Last) const;
+  int matchScore(int P, int W, Action Last) const;
 
   // Pattern data is initialized by the constructor, then constant.
   char Pat[MaxPat]; // Pattern data
Index: clangd/FuzzyMatch.cpp
===
--- clangd/FuzzyMatch.cpp
+++ clangd/FuzzyMatch.cpp
@@ -58,6 +58,7 @@
 
 #include "FuzzyMatch.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/Format.h"
 
 namespace clang {
@@ -67,7 +68,6 @@
 constexpr int FuzzyMatcher::MaxPat;
 constexpr int FuzzyMatcher::MaxWord;
 
-static char lower(char C) { return C >= 'A' && C <= 'Z' ? C + ('a' - 'A') : C; }
 // A "negative infinity" score that won't overflow.
 // We use this to mark unreachable states and forbidden solutions.
 // Score field is 15 bits wide, min value is -2^14, we use half of that.
@@ -80,25 +80,18 @@
   ScoreScale(PatN ? float{1} / (PerfectBonus * PatN) : 0), WordN(0) {
   std::copy(Pattern.begin(), Pattern.begin() + PatN, Pat);
   for (int I = 0; I < PatN; ++I)
-LowPat[I] = lower(Pat[I]);
-  Scores[0][0][Miss] = {0, Miss};
-  Scores[0][0][Match] = {AwfulScore, Miss};
-  for (int P = 0; P <= PatN; ++P)
-for (int W = 0; W < P; ++W)
-  for (Action A : {Miss, Match})
-Scores[P][W][A] = {AwfulScore, Miss};
-  if (PatN > 0)
-calculateRoles(Pat, PatRole, PatTypeSet, PatN);
+LowPat[I] = toLower(Pat[I]);
+  calculateRoles(Pat, PatRole, PatTypeSet, PatN);
 }
 
 Optional FuzzyMatcher::match(StringRef Word) {
   if (!(WordContainsPattern = init(Word)))
 return None;
-  if (!PatN)
-return 1;
   buildGraph();
-  auto Best = std::max(Scores[PatN][WordN][Miss].Score,
-   Scores[PatN][WordN][Match].Score);
+  // Find the optimal prefix of Word to match Pattern.
+  int Best = AwfulScore;
+  for (int I = PatN; I <= WordN; I++)
+Best = std::max(Best, Scores[PatN][I][Match].Score);
   if (isAwful(Best))
 return None;
   return ScoreScale * std::min(PerfectBonus * PatN, std::max(0, Best));
@@ -179,7 +172,8 @@
 }
 void FuzzyMatcher::calculateRoles(const char *Text, CharRole *Out, int &TypeSet,
   int N) {
-  assert(N > 0);
+  if (!N)
+return;
   CharType Type = packedLookup(CharTypes, Text[0]);
   TypeSet = 1 << Type;
   // Types holds a sliding window of (Prev, Curr, Next) types.
@@ -206,10 +200,8 @@
   if (PatN > WordN)
 return false;
   std::copy(NewWord.begin(), NewWord.begin() + WordN, Word);
-  if (PatN == 0)
-return true;
   for (int I = 0; I < WordN; ++I)
-LowWord[I] = lower(Word[I]);
+LowWord[I] = toLower(Word[I]);
 
   // Cheap subsequence check.
   for (int W = 0, P = 0; P != PatN; ++W) {
@@ -236,31 +228,29 @@
 // This range is not strict: we can apply larger bonuses/penalties, or penalize
 // non-matched characters.
 void FuzzyMatcher::buildGraph() {
+  Scores[0][0][Miss] = Scores[0][0][Match] = {0, Miss};
   for (int W = 0; W < WordN; ++W) {
-Scores[0][W + 1][Miss] = {Scores[0][W][Miss].Score - skipPenalty(W, Miss),
+Scores[0][W + 1][Miss] = {Scores[0][W][Miss].Score - missPenalty(W, Miss),
   Miss};
 Scores[0][W + 1][Match] = {AwfulScore, Miss};
   }
   for (int P = 0; P < PatN; ++P) {
+Scores[P + 1][P][Miss] = Scores[P + 1][P][Match] = {AwfulScore, Miss};
 for (int W = P; W < WordN; ++W) {
   auto &Score = Scores[P + 1][W + 1], &PreMiss = Scores[P + 1][W];
 
-  auto MatchMissScore = PreMiss[Match].Score;
-  auto MissMissScore = PreMiss[Miss].Score;
-  if (P < PatN - 1) { // Skipping trailing characters is always free.
-MatchMissScore -= skipPenalty(W, Match);
-MissMissScore -= skipPenalty(W, Miss);
-  }
+  auto MatchMissScore = PreMiss[Match].Score - missPenalty(W, Match);
+  auto MissMissScore = PreMiss[Miss].Score - missPenalty(W, Miss);
   Score[Miss] = (MatchMissScore > MissMissScore)
 ? ScoreInfo{MatchMissScore, Match}
 : ScoreInfo{MissMissScore, Miss};
 
   if (!allowMatch(P, W)) {
 Score[Match] = {AwfulScore, Miss};
   } else {
 auto &PreMatch = Scores[P][W];
-auto MatchMatchScore = PreMatch[Match].Score + matchBonus(P, W, Match);
-auto MissMatchScore = PreMatch[Miss].Score + matchBonus(P, 

[PATCH] D45054: Set dso_local on cfi_slowpath

[Evgenii Stepanov via Phabricator via cfe-commits]

eugenis added a comment.

This looks wrong. cfi_slowpath is defined in libclang_rt.cfi, which is linked 
to the main executable. It is not always dso-local. On Android it is defined in 
libdl.so and is never dso-local.


https://reviews.llvm.org/D45054



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

[PATCH] D44720: [clangd] Simplify fuzzy matcher (sequence alignment) by removing some condition checks.

[Fangrui Song via Phabricator via cfe-commits]

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



Comment at: clangd/FuzzyMatch.cpp:230
 void FuzzyMatcher::buildGraph() {
+  Scores[0][0][Miss] = Scores[0][0][Match] = {0, Miss};
   for (int W = 0; W < WordN; ++W) {

sammccall wrote:
> MaskRay wrote:
> > sammccall wrote:
> > > why this change?
> > > this has also been moved from the cheaper constructor to the more 
> > > expensive per-match call. (also the diagonal assignment added in the next 
> > > loop)
> > > 
> > > Also, shouldn't [0][0][Match] be AwfulScore?
> > > 
> > "The more expensive per-match call" is just two value assignments.
> > 
> > I have removed the expensive table initialization in the constructor.
> > 
> > [0][0][*] can be any value.
> > "The more expensive per-match call" is just two value assignments.
> Oops, sorry - by "more expensive" I mean "called thousands of times more 
> often".
> 
> > I have removed the expensive table initialization in the constructor.
> I don't want to be rude, but I asked why you changed this, and you didn't 
> answer. Unless there's a strong reason, I'd prefer to revert this change, as 
> I find this harder to reason about.
> (Roughly: in the old version of the code, any data that didn't need to change 
> for the life of the object was initialized in the constructor. That way I 
> didn't need to worry what was performance-critical and what wasn't - match() 
> only did what was strictly necessary).
> 
> > [0][0][*] can be any value.
> Can you please explain why?
> Oops, sorry - by "more expensive" I mean "called thousands of times more 
> often".

It is true that `Scores[0][0][Miss] = Scores[0][0][Match] = {0, Miss};` is the 
cost incurred for each word.

But **it is not full table initialization**, it is just two variable 
assignments. And we will assign to other values of the first row 
`Scores[0][*][*]` in the following loop. The old scatters the table 
construction to **two places**, the constructor and this dynamic programming 
site.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44720



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

[PATCH] D44720: [clangd] Simplify fuzzy matcher (sequence alignment) by removing some condition checks.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clangd/FuzzyMatch.cpp:230
 void FuzzyMatcher::buildGraph() {
+  Scores[0][0][Miss] = Scores[0][0][Match] = {0, Miss};
   for (int W = 0; W < WordN; ++W) {

MaskRay wrote:
> sammccall wrote:
> > MaskRay wrote:
> > > sammccall wrote:
> > > > why this change?
> > > > this has also been moved from the cheaper constructor to the more 
> > > > expensive per-match call. (also the diagonal assignment added in the 
> > > > next loop)
> > > > 
> > > > Also, shouldn't [0][0][Match] be AwfulScore?
> > > > 
> > > "The more expensive per-match call" is just two value assignments.
> > > 
> > > I have removed the expensive table initialization in the constructor.
> > > 
> > > [0][0][*] can be any value.
> > > "The more expensive per-match call" is just two value assignments.
> > Oops, sorry - by "more expensive" I mean "called thousands of times more 
> > often".
> > 
> > > I have removed the expensive table initialization in the constructor.
> > I don't want to be rude, but I asked why you changed this, and you didn't 
> > answer. Unless there's a strong reason, I'd prefer to revert this change, 
> > as I find this harder to reason about.
> > (Roughly: in the old version of the code, any data that didn't need to 
> > change for the life of the object was initialized in the constructor. That 
> > way I didn't need to worry what was performance-critical and what wasn't - 
> > match() only did what was strictly necessary).
> > 
> > > [0][0][*] can be any value.
> > Can you please explain why?
> > Oops, sorry - by "more expensive" I mean "called thousands of times more 
> > often".
> 
> It is true that `Scores[0][0][Miss] = Scores[0][0][Match] = {0, Miss};` is 
> the cost incurred for each word.
> 
> But **it is not full table initialization**, it is just two variable 
> assignments. And we will assign to other values of the first row 
> `Scores[0][*][*]` in the following loop. The old scatters the table 
> construction to **two places**, the constructor and this dynamic programming 
> site.
> [0][0][*] can be any value.

Can you please explain why?

`Scores[0][0][*]` is the initial value which will be propagated to all other 
values in the table.
The relative difference of pairwise values in the table is a constant whatever 
initial value is chosen.

If you ignore the max clamp you used later, the initial value does not matter.




Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44720



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

[PATCH] D44720: [clangd] Simplify fuzzy matcher (sequence alignment) by removing some condition checks.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clangd/FuzzyMatch.cpp:340
+  A[WordN] = Miss;
+  for (int I = WordN, P = PatN; I > 0; I--) {
+if (I == W)

sammccall wrote:
> MaskRay wrote:
> > sammccall wrote:
> > > sammccall wrote:
> > > > sammccall wrote:
> > > > > W is the right name in this file for a variable iterating over word 
> > > > > indices, please don't change this.
> > > > > The new variable above could be EndW or so?
> > > > As far as I can see, this loop is setting `A[W+1:...] = Miss` and 
> > > > populating `A[0...W]` with the exsting logic.
> > > > I think this would be clearer as two loops, currently there's a lot of 
> > > > conditionals around Last that obscure what's actually happening.
> > > You've shifted P (and the old W, new I) by 1. This does reduce the number 
> > > of +1 and -1 in this function, but it's inconsistent with how these are 
> > > used elsewhere: P should be the index into Pat of the character that 
> > > we're considering.
> > I don't understand the rationale not to use the shifted indices. The code 
> > actually use `Scores[P][W][*]` to mean the optimal match of the first `P` 
> > characters of the pattern with the first `W` characters of the word, not 
> > the position of the character.
> > 
> > On the other hand, C++ reverse iterators use the shifted one for `for (I = 
> > rend(); I != rbegin(); ++I)`. The shifted one makes ending condition check 
> > easier.
> > I don't understand the rationale not to use the shifted indices
> The rationale is entirely consistency with the surrounding code. The 
> consistency helps avoid off-by-one errors when similar loops have different 
> conventions.
> 
> In this file, when looping over word or pattern dimensions, P and W 
> respectively are used for loop variables, and can be interpreted as indices 
> into Pat/Word.
> Here the interpretation would be "did we match or miss character Word[W]"
`Scores[P][W][*]` is interpreted as how good it is if we align the first `P` 
characters of the pattern with the first `W` characters of the word.

Note the code uses `number of characters` instead of the position.

Here the new interpretation would be "what we should do for the last character 
of the first W characters"


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44720



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

[PATCH] D44765: PR36643 Make clang-format support more generic TMarcos

[Teodor Petrov via Phabricator via cfe-commits]

obfuscated added inline comments.



Comment at: lib/Format/FormatTokenLexer.cpp:386
   String->HasUnescapedNewline = Macro->HasUnescapedNewline;
+  String->TMacroStringLiteral = true;
 

krasimir wrote:
> In the original code, TMacro detection was done as:
> ```
> (Text.startswith(Prefix = "_T(\"") && Text.endswith(Postfix = "\")"))
> ```
> In the new code the left part is saved in TMacroStringLiteral, and the right 
> part is checked in ContinuationIndenter. Could you keep them together in 
> `FormatTokenLexer`.
> @alexfh, why are we checking for the right check at all? What would be an 
> example where this is needed to disambiguate?
> 
Are you talking about the code in ContinuationIndenter::createBreakableToken?

I don't think I understand how the hole thing works.
Using the debugger I can see that this function is executed first and then 
createBreakableToken.
So we first detect the tmacro in FormatTokenLexer::tryMerge_TMacro and then use 
this information in the createBreakableToken to do something with it.

So when I get a TMacroStringLiteral==true in createBreakableToken I know that 
the token starts with a TMacro and there is no need to use some king of 
startswith calls. Probably the endswith call is redundant and I can do just a 
string search backwards...


https://reviews.llvm.org/D44765



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

[PATCH] D44764: [clangd] Use operator<< to prevent printers issues in Gtest

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clangd/Protocol.h:673
 json::Expr toJSON(const CompletionItem &);
+std::ostream &operator<<(std::ostream &, const CompletionItem &);
 

I think raw_ostream should work fine here, it's what we've done elsewhere.
Is there a reason to use std::ostream?

(llvm's gtest is specially modified so raw_ostream printers can be used)



Comment at: unittests/clangd/JSONExprTests.cpp:19
-void PrintTo(const Expr &E, std::ostream *OS) {
-  llvm::raw_os_ostream(*OS) << llvm::formatv("{0:2}", E);
-}

malaperle wrote:
> This one I couldn't change to operator<< because there was already one 
> defined with raw_ostream. But this just means losing indentation so perhaps 
> that's not too bad?
yeah, I think this is fine.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44764



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

[PATCH] D44764: [clangd] Use operator<< to prevent printers issues in Gtest

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

(Thanks for sorting this out BTW!)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44764



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

[PATCH] D44559: [Sema] Wrong width of result of mul operation

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

What would the design for that warning be?  C promotes all arithmetic on 
sub-int types to int, and if that's assigned back to a variable of the original 
type, there's a conversion.  But you seem to only want to warn about truncating 
the result of multiplication and not, say, addition or negation.  Is there a 
principle to this?  Just the likelihood of escaping the range of the original 
type?


https://reviews.llvm.org/D44559



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

[PATCH] D45056: [X86] Split up -march=icelake to -client & -server

[Gabor Buella via Phabricator via cfe-commits]

GBuella created this revision.
GBuella added reviewers: craig.topper, zvi, echristo.
Herald added a subscriber: cfe-commits.

Repository:
  rC Clang

https://reviews.llvm.org/D45056

Files:
  include/clang/Basic/X86Target.def
  lib/Basic/Targets/X86.cpp
  test/Driver/x86-march.c
  test/Frontend/x86-target-cpu.c
  test/Misc/target-invalid-cpu-note.c
  test/Preprocessor/predefined-arch-macros.c

Index: test/Preprocessor/predefined-arch-macros.c
===
--- test/Preprocessor/predefined-arch-macros.c
+++ test/Preprocessor/predefined-arch-macros.c
@@ -1055,7 +1055,7 @@
 // CHECK_CNL_M64: #define __x86_64 1
 // CHECK_CNL_M64: #define __x86_64__ 1
 
-// RUN: %clang -march=icelake -m32 -E -dM %s -o - 2>&1 \
+// RUN: %clang -march=icelake-client -m32 -E -dM %s -o - 2>&1 \
 // RUN: -target i386-unknown-linux \
 // RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_ICL_M32
 // CHECK_ICL_M32: #define __AES__ 1
@@ -1110,8 +1110,64 @@
 // CHECK_ICL_M32: #define __i386__ 1
 // CHECK_ICL_M32: #define __tune_corei7__ 1
 // CHECK_ICL_M32: #define i386 1
-//
-// RUN: %clang -march=icelake -m64 -E -dM %s -o - 2>&1 \
+
+// RUN: %clang -march=icelake-server -m32 -E -dM %s -o - 2>&1 \
+// RUN: -target i386-unknown-linux \
+// RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_ICX_M32
+// CHECK_ICX_M32: #define __AES__ 1
+// CHECK_ICX_M32: #define __AVX2__ 1
+// CHECK_ICX_M32: #define __AVX512BITALG__ 1
+// CHECK_ICX_M32: #define __AVX512BW__ 1
+// CHECK_ICX_M32: #define __AVX512CD__ 1
+// CHECK_ICX_M32: #define __AVX512DQ__ 1
+// CHECK_ICX_M32: #define __AVX512F__ 1
+// CHECK_ICX_M32: #define __AVX512IFMA__ 1
+// CHECK_ICX_M32: #define __AVX512VBMI2__ 1
+// CHECK_ICX_M32: #define __AVX512VBMI__ 1
+// CHECK_ICX_M32: #define __AVX512VL__ 1
+// CHECK_ICX_M32: #define __AVX512VNNI__ 1
+// CHECK_ICX_M32: #define __AVX512VPOPCNTDQ__ 1
+// CHECK_ICX_M32: #define __AVX__ 1
+// CHECK_ICX_M32: #define __BMI2__ 1
+// CHECK_ICX_M32: #define __BMI__ 1
+// CHECK_ICX_M32: #define __CLFLUSHOPT__ 1
+// CHECK_ICX_M32: #define __CLWB__ 1
+// CHECK_ICX_M32: #define __F16C__ 1
+// CHECK_ICX_M32: #define __FMA__ 1
+// CHECK_ICX_M32: #define __GFNI__ 1
+// CHECK_ICX_M32: #define __LZCNT__ 1
+// CHECK_ICX_M32: #define __MMX__ 1
+// CHECK_ICX_M32: #define __MPX__ 1
+// CHECK_ICX_M32: #define __PCLMUL__ 1
+// CHECK_ICX_M32: #define __PKU__ 1
+// CHECK_ICX_M32: #define __POPCNT__ 1
+// CHECK_ICX_M32: #define __PRFCHW__ 1
+// CHECK_ICX_M32: #define __RDPID__ 1
+// CHECK_ICX_M32: #define __RDRND__ 1
+// CHECK_ICX_M32: #define __RDSEED__ 1
+// CHECK_ICX_M32: #define __RTM__ 1
+// CHECK_ICX_M32: #define __SGX__ 1
+// CHECK_ICX_M32: #define __SHA__ 1
+// CHECK_ICX_M32: #define __SSE2__ 1
+// CHECK_ICX_M32: #define __SSE3__ 1
+// CHECK_ICX_M32: #define __SSE4_1__ 1
+// CHECK_ICX_M32: #define __SSE4_2__ 1
+// CHECK_ICX_M32: #define __SSE__ 1
+// CHECK_ICX_M32: #define __SSSE3__ 1
+// CHECK_ICX_M32: #define __VAES__ 1
+// CHECK_ICX_M32: #define __VPCLMULQDQ__ 1
+// CHECK_ICX_M32: #define __XSAVEC__ 1
+// CHECK_ICX_M32: #define __XSAVEOPT__ 1
+// CHECK_ICX_M32: #define __XSAVES__ 1
+// CHECK_ICX_M32: #define __XSAVE__ 1
+// CHECK_ICX_M32: #define __corei7 1
+// CHECK_ICX_M32: #define __corei7__ 1
+// CHECK_ICX_M32: #define __i386 1
+// CHECK_ICX_M32: #define __i386__ 1
+// CHECK_ICX_M32: #define __tune_corei7__ 1
+// CHECK_ICX_M32: #define i386 1
+
+// RUN: %clang -march=icelake-client -m64 -E -dM %s -o - 2>&1 \
 // RUN: -target i386-unknown-linux \
 // RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_ICL_M64
 // CHECK_ICL_M64: #define __AES__ 1
@@ -1168,6 +1224,63 @@
 // CHECK_ICL_M64: #define __x86_64 1
 // CHECK_ICL_M64: #define __x86_64__ 1
 
+// RUN: %clang -march=icelake-server -m64 -E -dM %s -o - 2>&1 \
+// RUN: -target i386-unknown-linux \
+// RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_ICX_M64
+// CHECK_ICX_M64: #define __AES__ 1
+// CHECK_ICX_M64: #define __AVX2__ 1
+// CHECK_ICX_M64: #define __AVX512BITALG__ 1
+// CHECK_ICX_M64: #define __AVX512BW__ 1
+// CHECK_ICX_M64: #define __AVX512CD__ 1
+// CHECK_ICX_M64: #define __AVX512DQ__ 1
+// CHECK_ICX_M64: #define __AVX512F__ 1
+// CHECK_ICX_M64: #define __AVX512IFMA__ 1
+// CHECK_ICX_M64: #define __AVX512VBMI2__ 1
+// CHECK_ICX_M64: #define __AVX512VBMI__ 1
+// CHECK_ICX_M64: #define __AVX512VL__ 1
+// CHECK_ICX_M64: #define __AVX512VNNI__ 1
+// CHECK_ICX_M64: #define __AVX512VPOPCNTDQ__ 1
+// CHECK_ICX_M64: #define __AVX__ 1
+// CHECK_ICX_M64: #define __BMI2__ 1
+// CHECK_ICX_M64: #define __BMI__ 1
+// CHECK_ICX_M64: #define __CLFLUSHOPT__ 1
+// CHECK_ICX_M64: #define __CLWB__ 1
+// CHECK_ICX_M64: #define __F16C__ 1
+// CHECK_ICX_M64: #define __FMA__ 1
+// CHECK_ICX_M64: #define __GFNI__ 1
+// CHECK_ICX_M64: #define __LZCNT__ 1
+// CHECK_ICX_M64: #define __MMX__ 1
+// CHECK_ICX_M64: #define __MPX__ 1
+// CHECK_ICX_M64: #define __PCLMUL__ 1
+// CHECK_ICX_M64: #define __PKU__ 1
+// CHECK_

[PATCH] D44093: [BUILTINS] structure pretty printer

[Paul Semel via Phabricator via cfe-commits]

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



Comment at: test/Sema/builtin-dump-struct.c:8
+  void *b;
+  int (*goodfunc)(const char *, ...);
+  int (*badfunc1)(const char *);

aaron.ballman wrote:
> Can you also add a test for: `int (*badfunc4)(char *, ...);` and `int 
> (*badfunc5)();`
Isn't `int (*func)()` is a valid prototype for a printf like function in C ?
I instead added `int (*func)(void)` to the test cases.



Comment at: test/Sema/builtin-dump-struct.c:15
+  __builtin_dump_struct(1);// expected-error {{too few arguments 
to function call, expected 2, have 1}}
+  __builtin_dump_struct(1, 2); // expected-error {{passing 'int' to 
parameter of incompatible type 'structure pointer type': type mismatch at 1st 
parameter ('int' vs 'structure pointer type')}}
+  __builtin_dump_struct(&a, 2);// expected-error {{passing 'int' to 
parameter of incompatible type 'int (*)(const char *, ...)': type mismatch at 
2nd parameter ('int' vs 'int (*)(const char *, ...)')}}

aaron.ballman wrote:
> Hrm, the `'structure pointer type'` in this diagnostic is unfortunate because 
> it's being quoted as though it were a real type -- you could drop the single 
> quotes. If you think the resulting diagnostic reads too strangely, perhaps we 
> will have to go back to a custom diagnostic after all.
I think it will be better to just put something like `structure pointer`, so 
that we understand the type we are talking about.
But this diagnostic seems great, still, what do you think about sticking with 
it ?



Comment at: test/Sema/builtin-dump-struct.c:17
+  __builtin_dump_struct(&a, 2);// expected-error {{passing 'int' to 
parameter of incompatible type 'int (*)(const char *, ...)': type mismatch at 
2nd parameter ('int' vs 'int (*)(const char *, ...)')}}
+  __builtin_dump_struct(b, &goodfunc); // expected-error {{passing 'void *' to 
parameter of incompatible type 'structure pointer type': type mismatch at 1st 
parameter ('void *' vs 'structure pointer type')}}
+  __builtin_dump_struct(&a, badfunc1); // expected-error {{passing 'int 
(*)(const char *)' to parameter of incompatible type 'int (*)(const char *, 
...)': type mismatch at 2nd parameter ('int (*)(const char *)' vs 'int 
(*)(const char *, ...)')}}

aaron.ballman wrote:
> Why `&goodfunc`?
Yes, we already have a test like this anyway :)


Repository:
  rC Clang

https://reviews.llvm.org/D44093



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

[PATCH] D45058: [X86] Disable SGX for Skylake Server

[Gabor Buella via Phabricator via cfe-commits]

GBuella created this revision.
GBuella added reviewers: craig.topper, zvi, echristo.
Herald added a subscriber: cfe-commits.

Repository:
  rC Clang

https://reviews.llvm.org/D45058

Files:
  lib/Basic/Targets/X86.cpp


Index: lib/Basic/Targets/X86.cpp
===
--- lib/Basic/Targets/X86.cpp
+++ lib/Basic/Targets/X86.cpp
@@ -182,7 +182,8 @@
 setFeatureEnabledImpl(Features, "xsavec", true);
 setFeatureEnabledImpl(Features, "xsaves", true);
 setFeatureEnabledImpl(Features, "mpx", true);
-setFeatureEnabledImpl(Features, "sgx", true);
+if (Kind != CK_SkylakeServer) // SKX inherits all SKL features, except SGX
+  setFeatureEnabledImpl(Features, "sgx", true);
 setFeatureEnabledImpl(Features, "clflushopt", true);
 setFeatureEnabledImpl(Features, "rtm", true);
 LLVM_FALLTHROUGH;


Index: lib/Basic/Targets/X86.cpp
===
--- lib/Basic/Targets/X86.cpp
+++ lib/Basic/Targets/X86.cpp
@@ -182,7 +182,8 @@
 setFeatureEnabledImpl(Features, "xsavec", true);
 setFeatureEnabledImpl(Features, "xsaves", true);
 setFeatureEnabledImpl(Features, "mpx", true);
-setFeatureEnabledImpl(Features, "sgx", true);
+if (Kind != CK_SkylakeServer) // SKX inherits all SKL features, except SGX
+  setFeatureEnabledImpl(Features, "sgx", true);
 setFeatureEnabledImpl(Features, "clflushopt", true);
 setFeatureEnabledImpl(Features, "rtm", true);
 LLVM_FALLTHROUGH;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44093: [BUILTINS] structure pretty printer

[Paul Semel via Phabricator via cfe-commits]

paulsemel updated this revision to Diff 140304.
paulsemel marked 3 inline comments as done.
paulsemel added a comment.

Added Aaron suggestions.
Fixed a bug when having nested anonymous unions and structures.


Repository:
  rC Clang

https://reviews.llvm.org/D44093

Files:
  include/clang/Basic/Builtins.def
  lib/CodeGen/CGBuiltin.cpp
  lib/Sema/SemaChecking.cpp
  test/CodeGen/dump-struct-builtin.c
  test/Sema/builtin-dump-struct.c

Index: test/Sema/builtin-dump-struct.c
===
--- /dev/null
+++ test/Sema/builtin-dump-struct.c
@@ -0,0 +1,40 @@
+// RUN: %clang_cc1 -triple i386-unknown-unknown -fsyntax-only -fno-spell-checking -verify %s
+
+void invalid_uses() {
+  struct A {
+  };
+  struct A a;
+  void *b;
+  int (*goodfunc)(const char *, ...);
+  int (*badfunc1)(const char *);
+  int (*badfunc2)(int, ...);
+  void (*badfunc3)(const char *, ...);
+  int (*badfunc4)(char *, ...);
+  int (*badfunc5)(void);
+
+  __builtin_dump_struct(); // expected-error {{too few arguments to function call, expected 2, have 0}}
+  __builtin_dump_struct(1);// expected-error {{too few arguments to function call, expected 2, have 1}}
+  __builtin_dump_struct(1, 2); // expected-error {{passing 'int' to parameter of incompatible type structure pointer: type mismatch at 1st parameter ('int' vs structure pointer)}}
+  __builtin_dump_struct(&a, 2);// expected-error {{passing 'int' to parameter of incompatible type 'int (*)(const char *, ...)': type mismatch at 2nd parameter ('int' vs 'int (*)(const char *, ...)')}}
+  __builtin_dump_struct(b, &goodfunc); // expected-error {{passing 'void *' to parameter of incompatible type structure pointer: type mismatch at 1st parameter ('void *' vs structure pointer)}}
+  __builtin_dump_struct(&a, badfunc1); // expected-error {{passing 'int (*)(const char *)' to parameter of incompatible type 'int (*)(const char *, ...)': type mismatch at 2nd parameter ('int (*)(const char *)' vs 'int (*)(const char *, ...)')}}
+  __builtin_dump_struct(&a, badfunc2); // expected-error {{passing 'int (*)(int, ...)' to parameter of incompatible type 'int (*)(const char *, ...)': type mismatch at 2nd parameter ('int (*)(int, ...)' vs 'int (*)(const char *, ...)')}}
+  __builtin_dump_struct(&a, badfunc3); // expected-error {{passing 'void (*)(const char *, ...)' to parameter of incompatible type 'int (*)(const char *, ...)': type mismatch at 2nd parameter ('void (*)(const char *, ...)' vs 'int (*)(const char *, ...)')}}
+  __builtin_dump_struct(&a, badfunc4); // expected-error {{passing 'int (*)(char *, ...)' to parameter of incompatible type 'int (*)(const char *, ...)': type mismatch at 2nd parameter ('int (*)(char *, ...)' vs 'int (*)(const char *, ...)')}}
+  __builtin_dump_struct(&a, badfunc5); // expected-error {{passing 'int (*)(void)' to parameter of incompatible type 'int (*)(const char *, ...)': type mismatch at 2nd parameter ('int (*)(void)' vs 'int (*)(const char *, ...)')}}
+  __builtin_dump_struct(a, goodfunc);  // expected-error {{passing 'struct A' to parameter of incompatible type structure pointer: type mismatch at 1st parameter ('struct A' vs structure pointer)}}
+}
+
+void valid_uses() {
+  struct A {
+  };
+  union B {
+  };
+
+  int (*goodfunc)(const char *, ...);
+  struct A a;
+  union B b;
+
+  __builtin_dump_struct(&a, goodfunc);
+  __builtin_dump_struct(&b, goodfunc);
+}
Index: test/CodeGen/dump-struct-builtin.c
===
--- /dev/null
+++ test/CodeGen/dump-struct-builtin.c
@@ -0,0 +1,387 @@
+// RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck %s
+
+#include "Inputs/stdio.h"
+
+void unit1() {
+  struct U1A {
+short a;
+  };
+
+  struct U1A a = {
+  .a = 12,
+  };
+
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[RES1:%[0-9]+]] = getelementptr inbounds %struct.U1A, %struct.U1A* %a, i32 0, i32 0
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[LOAD1:%[0-9]+]] = load i16, i16* [[RES1]],
+  // CHECK: call i32 (i8*, ...) @printf({{.*}}, i16 [[LOAD1]])
+  // CHECK: call i32 (i8*, ...) @printf(
+  __builtin_dump_struct(&a, &printf);
+}
+
+void unit2() {
+  struct U2A {
+unsigned short a;
+  };
+
+  struct U2A a = {
+  .a = 12,
+  };
+
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[RES1:%[0-9]+]] = getelementptr inbounds %struct.U2A, %struct.U2A* %a, i32 0, i32 0
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[LOAD1:%[0-9]+]] = load i16, i16* [[RES1]],
+  // CHECK: call i32 (i8*, ...) @printf({{.*}}, i16 [[LOAD1]])
+  // CHECK: call i32 (i8*, ...) @printf(
+  __builtin_dump_struct(&a, &printf);
+}
+
+void unit3() {
+  struct U3A {
+int a;
+  };
+
+  struct U3A a = {
+  .a = 12,
+  };
+
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[RES1:%[0-9]+]] = getelementptr inbounds %struct.U3A, %struct.U3A* %a, i32 0, i32 0
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[LOAD

[PATCH] D44720: [clangd] Simplify fuzzy matcher (sequence alignment) by removing some condition checks.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 140306.
MaskRay added a comment.

nit picking


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44720

Files:
  clangd/FuzzyMatch.cpp
  clangd/FuzzyMatch.h

Index: clangd/FuzzyMatch.h
===
--- clangd/FuzzyMatch.h
+++ clangd/FuzzyMatch.h
@@ -58,8 +58,8 @@
   void buildGraph();
   void calculateRoles(const char *Text, CharRole *Out, int &Types, int N);
   bool allowMatch(int P, int W) const;
-  int skipPenalty(int W, Action Last) const;
-  int matchBonus(int P, int W, Action Last) const;
+  int missPenalty(int W, Action Last) const;
+  int matchScore(int P, int W, Action Last) const;
 
   // Pattern data is initialized by the constructor, then constant.
   char Pat[MaxPat]; // Pattern data
Index: clangd/FuzzyMatch.cpp
===
--- clangd/FuzzyMatch.cpp
+++ clangd/FuzzyMatch.cpp
@@ -58,6 +58,7 @@
 
 #include "FuzzyMatch.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/Format.h"
 
 namespace clang {
@@ -67,7 +68,6 @@
 constexpr int FuzzyMatcher::MaxPat;
 constexpr int FuzzyMatcher::MaxWord;
 
-static char lower(char C) { return C >= 'A' && C <= 'Z' ? C + ('a' - 'A') : C; }
 // A "negative infinity" score that won't overflow.
 // We use this to mark unreachable states and forbidden solutions.
 // Score field is 15 bits wide, min value is -2^14, we use half of that.
@@ -80,25 +80,19 @@
   ScoreScale(PatN ? float{1} / (PerfectBonus * PatN) : 0), WordN(0) {
   std::copy(Pattern.begin(), Pattern.begin() + PatN, Pat);
   for (int I = 0; I < PatN; ++I)
-LowPat[I] = lower(Pat[I]);
-  Scores[0][0][Miss] = {0, Miss};
-  Scores[0][0][Match] = {AwfulScore, Miss};
-  for (int P = 0; P <= PatN; ++P)
-for (int W = 0; W < P; ++W)
-  for (Action A : {Miss, Match})
-Scores[P][W][A] = {AwfulScore, Miss};
-  if (PatN > 0)
-calculateRoles(Pat, PatRole, PatTypeSet, PatN);
+LowPat[I] = toLower(Pat[I]);
+  calculateRoles(Pat, PatRole, PatTypeSet, PatN);
 }
 
 Optional FuzzyMatcher::match(StringRef Word) {
   if (!(WordContainsPattern = init(Word)))
 return None;
-  if (!PatN)
-return 1;
   buildGraph();
-  auto Best = std::max(Scores[PatN][WordN][Miss].Score,
-   Scores[PatN][WordN][Match].Score);
+  // The pattern doesn't have to match the whole word (but the whole pattern
+  // must match). Find the optimal prefix of Word to match Pattern.
+  int Best = AwfulScore;
+  for (int I = PatN; I <= WordN; I++)
+Best = std::max(Best, Scores[PatN][I][Match].Score);
   if (isAwful(Best))
 return None;
   return ScoreScale * std::min(PerfectBonus * PatN, std::max(0, Best));
@@ -179,7 +173,8 @@
 }
 void FuzzyMatcher::calculateRoles(const char *Text, CharRole *Out, int &TypeSet,
   int N) {
-  assert(N > 0);
+  if (!N)
+return;
   CharType Type = packedLookup(CharTypes, Text[0]);
   TypeSet = 1 << Type;
   // Types holds a sliding window of (Prev, Curr, Next) types.
@@ -206,10 +201,8 @@
   if (PatN > WordN)
 return false;
   std::copy(NewWord.begin(), NewWord.begin() + WordN, Word);
-  if (PatN == 0)
-return true;
   for (int I = 0; I < WordN; ++I)
-LowWord[I] = lower(Word[I]);
+LowWord[I] = toLower(Word[I]);
 
   // Cheap subsequence check.
   for (int W = 0, P = 0; P != PatN; ++W) {
@@ -236,31 +229,29 @@
 // This range is not strict: we can apply larger bonuses/penalties, or penalize
 // non-matched characters.
 void FuzzyMatcher::buildGraph() {
+  Scores[0][0][Miss] = Scores[0][0][Match] = {0, Miss};
   for (int W = 0; W < WordN; ++W) {
-Scores[0][W + 1][Miss] = {Scores[0][W][Miss].Score - skipPenalty(W, Miss),
+Scores[0][W + 1][Miss] = {Scores[0][W][Miss].Score - missPenalty(W, Miss),
   Miss};
 Scores[0][W + 1][Match] = {AwfulScore, Miss};
   }
   for (int P = 0; P < PatN; ++P) {
+Scores[P + 1][P][Miss] = Scores[P + 1][P][Match] = {AwfulScore, Miss};
 for (int W = P; W < WordN; ++W) {
   auto &Score = Scores[P + 1][W + 1], &PreMiss = Scores[P + 1][W];
 
-  auto MatchMissScore = PreMiss[Match].Score;
-  auto MissMissScore = PreMiss[Miss].Score;
-  if (P < PatN - 1) { // Skipping trailing characters is always free.
-MatchMissScore -= skipPenalty(W, Match);
-MissMissScore -= skipPenalty(W, Miss);
-  }
+  auto MatchMissScore = PreMiss[Match].Score - missPenalty(W, Match);
+  auto MissMissScore = PreMiss[Miss].Score - missPenalty(W, Miss);
   Score[Miss] = (MatchMissScore > MissMissScore)
 ? ScoreInfo{MatchMissScore, Match}
 : ScoreInfo{MissMissScore, Miss};
 
   if (!allowMatch(P, W)) {
 Score[Match] = {AwfulScore, Miss};
   } else {
 auto &PreMatch = Scores[P][W];
-auto MatchMatchScore = PreMatch[Match].Score + matchBonus(P, W