[PATCH] D49754: Add -m(no-)spe, and e500 CPU definitions and support to clang

[John Paul Adrian Glaubitz via Phabricator via cfe-commits]

glaubitz added a comment.

In D49754#1323642 , @vit9696 wrote:

> Might the first crash from https://reviews.llvm.org/D49754#1183753 reproduce 
> for you or perhaps you have already bisected to trunk to figure out the 
> changest that fixes it?


Yes, it does on Debian unstable 32-bit PowerPC with the patches against LLVM-7, 
didn't test LLVM-8:

  root@kapitsa:/srv/llvm# clang-7 -cc1 -triple powerpc-gnu-linux-eabi -emit-obj 
-target-cpu ppc -target-feature +spe -O2  t.cpp
  t.cpp:23:1: warning: control reaches end of non-void function
  }
  ^
  Stack dump:
  0.  Program arguments: clang-7 -cc1 -triple powerpc-gnu-linux-eabi 
-emit-obj -target-cpu ppc -target-feature +spe -O2 t.cpp 
  1.   parser at end of file
  2.  Code generation
  3.  Running pass 'Function Pass Manager' on module 't.cpp'.
  4.  Running pass 'PowerPC DAG->DAG Pattern Instruction Selection' on 
function '@_ZN1c2fnERK1a'
  
/usr/lib/powerpc-linux-gnu/libLLVM-7.so.1(_ZN4llvm3sys15PrintStackTraceERNS_11raw_ostreamE+0x40)[0xcc684a4]
  /usr/lib/powerpc-linux-gnu/libLLVM-7.so.1(+0x81886c)[0xcc6886c]
  
/usr/lib/powerpc-linux-gnu/libLLVM-7.so.1(_ZN4llvm3sys17RunSignalHandlersEv+0xd8)[0xcc65b68]
  /usr/lib/powerpc-linux-gnu/libLLVM-7.so.1(+0x818a8c)[0xcc68a8c]
  linux-vdso32.so.1(__kernel_sigtramp32+0x0)[0x100424]
  /usr/lib/powerpc-linux-gnu/libLLVM-7.so.1(+0xe157dc)[0xd2657dc]
  /usr/lib/powerpc-linux-gnu/libLLVM-7.so.1(+0xdfdab0)[0xd24dab0]
  /usr/lib/powerpc-linux-gnu/libLLVM-7.so.1(+0xdf7224)[0xd247224]
  /usr/lib/powerpc-linux-gnu/libLLVM-7.so.1(+0xe10014)[0xd260014]
  
/usr/lib/powerpc-linux-gnu/libLLVM-7.so.1(_ZN4llvm12SelectionDAG13LegalizeTypesEv+0x244)[0xd265ba4]
  
/usr/lib/powerpc-linux-gnu/libLLVM-7.so.1(_ZN4llvm16SelectionDAGISel17CodeGenAndEmitDAGEv+0x238)[0xd36a39c]
  
/usr/lib/powerpc-linux-gnu/libLLVM-7.so.1(_ZN4llvm16SelectionDAGISel16SelectBasicBlockENS_14ilist_iteratorINS_12ilist_detail12node_optionsINS_11InstructionELb0ELb0EvEELb0ELb1EEES6_Rb+0x1fc)[0xd36a048]
  
/usr/lib/powerpc-linux-gnu/libLLVM-7.so.1(_ZN4llvm16SelectionDAGISel20SelectAllBasicBlocksERKNS_8FunctionE+0x21dc)[0xd368af0]
  
/usr/lib/powerpc-linux-gnu/libLLVM-7.so.1(_ZN4llvm16SelectionDAGISel20runOnMachineFunctionERNS_15MachineFunctionE+0x778)[0xd3659cc]
  /usr/lib/powerpc-linux-gnu/libLLVM-7.so.1(+0x24a5a00)[0xe8f5a00]
  
/usr/lib/powerpc-linux-gnu/libLLVM-7.so.1(_ZN4llvm19MachineFunctionPass13runOnFunctionERNS_8FunctionE+0xcc)[0xcf98948]
  
/usr/lib/powerpc-linux-gnu/libLLVM-7.so.1(_ZN4llvm13FPPassManager13runOnFunctionERNS_8FunctionE+0x1e8)[0xcdc0e18]
  
/usr/lib/powerpc-linux-gnu/libLLVM-7.so.1(_ZN4llvm13FPPassManager11runOnModuleERNS_6ModuleE+0x60)[0xcdc13d4]
  
/usr/lib/powerpc-linux-gnu/libLLVM-7.so.1(_ZN4llvm6legacy15PassManagerImpl3runERNS_6ModuleE+0x390)[0xcdc1960]
  
/usr/lib/powerpc-linux-gnu/libLLVM-7.so.1(_ZN4llvm6legacy11PassManager3runERNS_6ModuleE+0x14)[0xcdc220c]
  
clang-7(_ZN5clang17EmitBackendOutputERNS_17DiagnosticsEngineERKNS_19HeaderSearchOptionsERKNS_14CodeGenOptionsERKNS_13TargetOptionsERKNS_11LangOptionsERKN4llvm10DataLayoutEPNSE_6ModuleENS_13BackendActionESt10unique_ptrINSE_17raw_pwrite_streamESt14default_deleteISM_EE+0x32b4)[0x1023f644]
  clang-7[0x1074857c]
  clang-7(_ZN5clang8ParseASTERNS_4SemaEbb+0x2b4)[0x10c3da9c]
  clang-7(_ZN5clang17ASTFrontendAction13ExecuteActionEv+0xac)[0x106a2a84]
  clang-7(_ZN5clang13CodeGenAction13ExecuteActionEv+0x390)[0x107472f4]
  clang-7(_ZN5clang14FrontendAction7ExecuteEv+0x6c)[0x106a2408]
  
clang-7(_ZN5clang16CompilerInstance13ExecuteActionERNS_14FrontendActionE+0x638)[0x10659aa4]
  
clang-7(_ZN5clang25ExecuteCompilerInvocationEPNS_16CompilerInstanceE+0x748)[0x10743ba0]
  clang-7(_Z8cc1_mainN4llvm8ArrayRefIPKcEES2_Pv+0x6bc)[0x101ee2c4]
  clang-7(main+0x2920)[0x101ec550]
  /lib/powerpc-linux-gnu/libc.so.6(+0x22558)[0xbef2558]
  /lib/powerpc-linux-gnu/libc.so.6(__libc_start_main+0xe8)[0xbef2748]
  Segmentation fault
  root@kapitsa:/srv/llvm#


Repository:
  rC Clang

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

https://reviews.llvm.org/D49754



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

[PATCH] D55097: [constexpr][c++2a] Try-catch blocks in constexpr functions

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM aside from a few other small nits.




Comment at: lib/Sema/SemaDeclCXX.cpp:1904
+  case Stmt::CXXTryStmtClass:
+if (!Cxx2aLoc.isValid())
+  Cxx2aLoc = S->getBeginLoc();

`Cxx2aLoc.isInvalid()`



Comment at: lib/Sema/SemaDeclCXX.cpp:1956
+//
+// In C++2a lifts this restriction, as long as inner statements do also
+// apply to general constexpr rules.

This restriction is lifted in C++2a, as long as inner statements also apply the 
general constexpr rules.


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

https://reviews.llvm.org/D55097



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

[PATCH] D54450: Get the correct range of tokens for preprocessor conditions

[Miklos Vajna via Phabricator via cfe-commits]

vmiklos added a comment.

Next ping -- could you please review this one? Thanks! :-)


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

https://reviews.llvm.org/D54450



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

[PATCH] D54450: Get the correct range of tokens for preprocessor conditions

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

This seems to be correct, but the test coverage could be improved.
There are no tests with `()` e.g.




Comment at: lib/Lex/PPExpressions.cpp:154-156
 // Consume the ).
-Result.setEnd(PeekTok.getLocation());
 PP.LexNonComment(PeekTok);
+Result.setEnd(PeekTok.getLocation());

I'm not sure this is covered with the test?


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

https://reviews.llvm.org/D54450



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

[PATCH] D55475: Misc typos fixes in ./lib folder

[luzpaz via Phabricator via cfe-commits]

luzpaz created this revision.
Herald added subscribers: cfe-commits, jfb, whisperity, nhaehnle, jvesely, 
jholewinski.
Herald added a reviewer: teemperor.

Found via `codespell -q 3 -I ../clang-whitelist.txt -L 
uint,importd,crasher,gonna,cant,ue,ons,orign,ned`


Repository:
  rC Clang

https://reviews.llvm.org/D55475

Files:
  lib/ARCMigrate/FileRemapper.cpp
  lib/AST/ASTContext.cpp
  lib/AST/ASTStructuralEquivalence.cpp
  lib/AST/ExprConstant.cpp
  lib/AST/ODRHash.cpp
  lib/AST/RawCommentList.cpp
  lib/Analysis/CloneDetection.cpp
  lib/CodeGen/BackendUtil.cpp
  lib/CodeGen/CGBlocks.cpp
  lib/CodeGen/CGBuiltin.cpp
  lib/CodeGen/CGDebugInfo.h
  lib/CodeGen/CGObjC.cpp
  lib/CodeGen/CGOpenMPRuntime.h
  lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  lib/CodeGen/CGStmtOpenMP.cpp
  lib/Driver/Driver.cpp
  lib/Driver/ToolChain.cpp
  lib/Driver/ToolChains/AMDGPU.cpp
  lib/Driver/ToolChains/Darwin.cpp
  lib/Driver/ToolChains/Hexagon.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Lex/Lexer.cpp
  lib/Lex/PPDirectives.cpp
  lib/Lex/Preprocessor.cpp
  lib/Parse/Parser.cpp
  lib/Rewrite/RewriteRope.cpp
  lib/Sema/Sema.cpp
  lib/Sema/SemaChecking.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclCXX.cpp
  lib/Sema/SemaDeclObjC.cpp
  lib/Sema/SemaInit.cpp
  lib/Sema/SemaLookup.cpp
  lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
  lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
  lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp
  lib/StaticAnalyzer/Checkers/CloneChecker.cpp
  lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
  lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
  lib/StaticAnalyzer/Checkers/PointerArithChecker.cpp
  lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
  lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
  lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
  lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
  lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp
  lib/StaticAnalyzer/Core/BugReporter.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
  lib/StaticAnalyzer/Core/Store.cpp
  lib/StaticAnalyzer/Core/WorkList.cpp
  lib/StaticAnalyzer/README.txt

Index: lib/StaticAnalyzer/README.txt
===
--- lib/StaticAnalyzer/README.txt
+++ lib/StaticAnalyzer/README.txt
@@ -69,23 +69,23 @@
 
 = Notes about C++ =
 
-Since now constructors are seen before the variable that is constructed 
-in the CFG, we create a temporary object as the destination region that 
+Since now constructors are seen before the variable that is constructed
+in the CFG, we create a temporary object as the destination region that
 is constructed into. See ExprEngine::VisitCXXConstructExpr().
 
 In ExprEngine::processCallExit(), we always bind the object region to the
 evaluated CXXConstructExpr. Then in VisitDeclStmt(), we compute the
 corresponding lazy compound value if the variable is not a reference, and
 bind the variable region to the lazy compound value. If the variable
-is a reference, just use the object region as the initilizer value.
+is a reference, just use the object region as the initializer value.
 
 Before entering a C++ method (or ctor/dtor), the 'this' region is bound
-to the object region. In ctors, we synthesize 'this' region with  
+to the object region. In ctors, we synthesize 'this' region with
 CXXRecordDecl*, which means we do not use type qualifiers. In methods, we
-synthesize 'this' region with CXXMethodDecl*, which has getThisType() 
+synthesize 'this' region with CXXMethodDecl*, which has getThisType()
 taking type qualifiers into account. It does not matter we use qualified
 'this' region in one method and unqualified 'this' region in another
-method, because we only need to ensure the 'this' region is consistent 
+method, because we only need to ensure the 'this' region is consistent
 when we synthesize it and create it directly from CXXThisExpr in a single
 method call.
 
Index: lib/StaticAnalyzer/Core/WorkList.cpp
===
--- lib/StaticAnalyzer/Core/WorkList.cpp
+++ lib/StaticAnalyzer/Core/WorkList.cpp
@@ -152,7 +152,7 @@
 auto BE = N->getLocation().getAs();
 
 if (!BE) {
-  // Assume the choice of the order of the preceeding block entrance was
+  // Assume the choice of the order of the preceding block entrance was
   // correct.
   StackUnexplored.push_back(U);
 } else {
Index: lib/StaticAnalyzer/Core/Store.cpp
===
--- lib/StaticAnalyzer/Core/Store.cpp
+++ lib/StaticAnalyzer/Core/Store.cpp
@@ -88,7 +88,7 @@
   return R;
 
 // We don't know what to make of it.  Return a NULL region, which
-// will be interpretted as UnknownVal.
+// will be interpreted as UnknownVal.
 ret

[PATCH] D55270: [Sema] Further improvements to to static_assert diagnostics.

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

Quuxplusone added a subscriber: saar.raz.
Quuxplusone added inline comments.



Comment at: test/PCH/cxx-static_assert.cpp:17
 
-// expected-error@12 {{static_assert failed "N is not 2!"}}
+// expected-error@12 {{static_assert failed due to requirement '1 == 2' "N is 
not 2!"}}
 T<1> t1; // expected-note {{in instantiation of template class 'T<1>' 
requested here}}

aaron.ballman wrote:
> Quuxplusone wrote:
> > courbet wrote:
> > > aaron.ballman wrote:
> > > > I'm not certain how I feel about now printing the failure condition 
> > > > when there's an explicit message provided. From what I understand, a 
> > > > fair amount of code in the wild does `static_assert(some_condition, 
> > > > "some_condition")` because of older language modes where the message 
> > > > was not optional. I worry we're going to start seeing a lot of 
> > > > diagnostics like: `static_assert failed due to requirement '1 == 2' "N 
> > > > == 2"`, which seems a bit low-quality. See 
> > > > `DFAPacketizer::DFAPacketizer()` in LLVM as an example of something 
> > > > similar.
> > > > 
> > > > Given that the user entered a message, do we still want to show the 
> > > > requirement? Do we feel the same way if the requirement is fairly large?
> > > The issue is that `"N == 2"` is a useless error message. Actually, since 
> > > the  error message has to be a string literal, there is no way for the 
> > > user to print a debuggable output. So I really think we should print the 
> > > failed condition.
> > FWIW, I also don't agree with Aaron's concern.
> > 
> > I do think there is a lot of code in the wild whose string literal was 
> > "phoned in." After all, this is why C++17 allows us to omit the string 
> > literal: to avoid boilerplate like this.
> > 
> > static_assert(some-condition, "some-condition");
> > static_assert(some-condition, "some-condition was not satisfied");
> > static_assert(some-condition, "some-condition must be satisfied");
> > static_assert(some-condition, "");
> > 
> > But should Clang go _out of its way_ to suppress such "redundant" string 
> > literals? First of all, such suppression would be C++14-and-earlier: if a 
> > C++17-native program contains a string literal, we should maybe assume it's 
> > on purpose. Second, it's not clear how a machine could detect "redundant" 
> > literals with high fidelity.
> > 
> > static_assert(std::is_integral, "std::is_integral");
> > // clearly redundant
> > static_assert(std::is_integral, "T must be integral");
> > // redundant, but unlikely to be machine-detectable as such
> > 
> > static_assert((DFA_MAX_RESTERMS * DFA_MAX_RESOURCES) <= (8 * 
> > sizeof(DFAInput)),
> > "(DFA_MAX_RESTERMS * DFA_MAX_RESOURCES) > (8 * sizeof(DFAInput))");
> > // redundant, but unlikely to be machine-detectable as such
> > // thanks to the substitution of > for <=
> > 
> > static_assert((DFA_MAX_RESTERMS * DFA_MAX_RESOURCES) <= (8 * 
> > sizeof(DFAInput)),
> > "(DFA_MAX_RESTERMS * DFA_MAX_RESOURCES) too big for DFAInput");
> > // redundant, but unlikely to be machine-detectable as such
> > 
> > In any event, I agree with @courbet that Clang should print the expansion 
> > of the failed condition in any case.
> > 
> > Also: One might argue that if the `static_assert` fits completely on one 
> > source line, then we could omit the string-literal from our error message 
> > because the string literal will be shown anyway as part of the offending 
> > source line — but I believe IDE-users would complain about that, so, we 
> > shouldn't do that. And even then, we'd still want to print the failed 
> > condition!
> > 
> > Checking my understanding: The `static_assert` above (taken from 
> > `DFAPacketizer::DFAPacketizer()` in LLVM) would be unchanged by @courbet's 
> > patches, because none of its subexpressions are template-dependent. Right?
> > But should Clang go _out of its way_ to suppress such "redundant" string 
> > literals? 
> 
> I wasn't suggesting it should; I was suggesting that Clang should be 
> conservative and suppress printing the conditional when a message is present, 
> not when they look to be redundant enough.
> 
> > if a C++17-native program contains a string literal, we should maybe assume 
> > it's on purpose. 
> 
> This is the exact scenario I was envisioning.
> 
> It's a relatively weak preference, but I kind of prefer not displaying the 
> conditional information in the presence of a message (at least in C++17 and 
> above), especially as the conditional can be huge. I'm thinking of scenarios 
> where the user does something like:
> ```
> static_assert(condition1 && condition2 && (condition3 || condition4), "Simple 
> explanation");
> ```
> except that `condition` is replaced by `std::some_type_trait` in 
> various interesting ways.
> 
> I'm thinking of scenarios where the user does something like:
> 
> static_assert(condition1 && condition2 && (condit

[PATCH] D55476: [libunwind] [cmake] Rename append_if to avoid collision with LLVM

[Michał Górny via Phabricator via cfe-commits]

mgorny created this revision.
mgorny added reviewers: krytarowski, compnerd, phosek.
Herald added subscribers: libcxx-commits, jfb.

Rename the 'append_if' macro used in libunwind to 'unwind_append_if'.
Otherwise, when used in a combined LLVM+libunwind build, it overrides
the *incompatible* 'append_if' function from LLVM and breaks projects
following libunwind, e.g. OpenMP.


Repository:
  rUNW libunwind

https://reviews.llvm.org/D55476

Files:
  CMakeLists.txt
  src/CMakeLists.txt

Index: src/CMakeLists.txt
===
--- src/CMakeLists.txt
+++ src/CMakeLists.txt
@@ -4,7 +4,7 @@
 libunwind.cpp
 Unwind-EHABI.cpp
 Unwind-seh.cpp)
-append_if(LIBUNWIND_CXX_SOURCES APPLE Unwind_AppleExtras.cpp)
+unwind_append_if(LIBUNWIND_CXX_SOURCES APPLE Unwind_AppleExtras.cpp)
 
 set(LIBUNWIND_C_SOURCES
 UnwindLevel1.c
@@ -36,7 +36,7 @@
 ${CMAKE_CURRENT_SOURCE_DIR}/../include/libunwind.h
 ${CMAKE_CURRENT_SOURCE_DIR}/../include/unwind.h)
 
-append_if(LIBUNWIND_HEADERS APPLE
+unwind_append_if(LIBUNWIND_HEADERS APPLE
   "${CMAKE_CURRENT_SOURCE_DIR}/../include/mach-o/compact_unwind_encoding.h")
 
 if (MSVC_IDE)
@@ -52,25 +52,25 @@
 
 # Generate library list.
 set(libraries)
-append_if(libraries LIBUNWIND_HAS_C_LIB c)
+unwind_append_if(libraries LIBUNWIND_HAS_C_LIB c)
 if (LIBUNWIND_USE_COMPILER_RT)
   list(APPEND libraries "${LIBUNWIND_BUILTINS_LIBRARY}")
 else()
-  append_if(libraries LIBUNWIND_HAS_GCC_S_LIB gcc_s)
-  append_if(libraries LIBUNWIND_HAS_GCC_LIB gcc)
+  unwind_append_if(libraries LIBUNWIND_HAS_GCC_S_LIB gcc_s)
+  unwind_append_if(libraries LIBUNWIND_HAS_GCC_LIB gcc)
 endif()
-append_if(libraries LIBUNWIND_HAS_DL_LIB dl)
+unwind_append_if(libraries LIBUNWIND_HAS_DL_LIB dl)
 if (LIBUNWIND_ENABLE_THREADS)
-  append_if(libraries LIBUNWIND_HAS_PTHREAD_LIB pthread)
+  unwind_append_if(libraries LIBUNWIND_HAS_PTHREAD_LIB pthread)
 endif()
 
 # Setup flags.
-append_if(LIBUNWIND_CXX_FLAGS LIBUNWIND_HAS_NO_RTTI_FLAG -fno-rtti)
+unwind_append_if(LIBUNWIND_CXX_FLAGS LIBUNWIND_HAS_NO_RTTI_FLAG -fno-rtti)
 
-append_if(LIBUNWIND_LINK_FLAGS LIBUNWIND_HAS_NODEFAULTLIBS_FLAG -nodefaultlibs)
+unwind_append_if(LIBUNWIND_LINK_FLAGS LIBUNWIND_HAS_NODEFAULTLIBS_FLAG -nodefaultlibs)
 
 # MINGW_LIBRARIES is defined in config-ix.cmake
-append_if(libraries MINGW "${MINGW_LIBRARIES}")
+unwind_append_if(libraries MINGW "${MINGW_LIBRARIES}")
 
 if (LIBUNWIND_HAS_NO_EXCEPTIONS_FLAG AND LIBUNWIND_HAS_FUNWIND_TABLES)
   list(APPEND LIBUNWIND_COMPILE_FLAGS -fno-exceptions)
Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -204,7 +204,7 @@
 set(LIBUNWIND_LINK_FLAGS "")
 
 # Get required flags.
-macro(append_if list condition var)
+macro(unwind_append_if list condition var)
   if (${condition})
 list(APPEND ${list} ${var})
   endif()
@@ -242,48 +242,48 @@
 # Setup Compiler Flags
 #===
 
-append_if(LIBUNWIND_COMPILE_FLAGS LIBUNWIND_HAS_WERROR_FLAG -Werror=return-type)
+unwind_append_if(LIBUNWIND_COMPILE_FLAGS LIBUNWIND_HAS_WERROR_FLAG -Werror=return-type)
 
 # Get warning flags
-append_if(LIBUNWIND_COMPILE_FLAGS LIBUNWIND_HAS_W_FLAG -W)
-append_if(LIBUNWIND_COMPILE_FLAGS LIBUNWIND_HAS_WALL_FLAG -Wall)
-append_if(LIBUNWIND_COMPILE_FLAGS LIBUNWIND_HAS_WCHAR_SUBSCRIPTS_FLAG -Wchar-subscripts)
-append_if(LIBUNWIND_COMPILE_FLAGS LIBUNWIND_HAS_WCONVERSION_FLAG -Wconversion)
-append_if(LIBUNWIND_COMPILE_FLAGS LIBUNWIND_HAS_WMISMATCHED_TAGS_FLAG -Wmismatched-tags)
-append_if(LIBUNWIND_COMPILE_FLAGS LIBUNWIND_HAS_WMISSING_BRACES_FLAG -Wmissing-braces)
-append_if(LIBUNWIND_COMPILE_FLAGS LIBUNWIND_HAS_WNEWLINE_EOF_FLAG -Wnewline-eof)
-append_if(LIBUNWIND_COMPILE_FLAGS LIBUNWIND_HAS_WNO_UNUSED_FUNCTION_FLAG -Wno-unused-function)
-append_if(LIBUNWIND_COMPILE_FLAGS LIBUNWIND_HAS_WSHADOW_FLAG -Wshadow)
-append_if(LIBUNWIND_COMPILE_FLAGS LIBUNWIND_HAS_WSHORTEN_64_TO_32_FLAG -Wshorten-64-to-32)
-append_if(LIBUNWIND_COMPILE_FLAGS LIBUNWIND_HAS_WSIGN_COMPARE_FLAG -Wsign-compare)
-append_if(LIBUNWIND_COMPILE_FLAGS LIBUNWIND_HAS_WSIGN_CONVERSION_FLAG -Wsign-conversion)
-append_if(LIBUNWIND_COMPILE_FLAGS LIBUNWIND_HAS_WSTRICT_ALIASING_FLAG -Wstrict-aliasing=2)
-append_if(LIBUNWIND_COMPILE_FLAGS LIBUNWIND_HAS_WSTRICT_OVERFLOW_FLAG -Wstrict-overflow=4)
-append_if(LIBUNWIND_COMPILE_FLAGS LIBUNWIND_HAS_WUNUSED_PARAMETER_FLAG -Wunused-parameter)
-append_if(LIBUNWIND_COMPILE_FLAGS LIBUNWIND_HAS_WUNUSED_VARIABLE_FLAG -Wunused-variable)
-append_if(LIBUNWIND_COMPILE_FLAGS LIBUNWIND_HAS_WWRITE_STRINGS_FLAG -Wwrite-strings)
-append_if(LIBUNWIND_COMPILE_FLAGS LIBUNWIND_HAS_WUNDEF_FLAG -Wundef)
+unwind_append_if(LIBUNWIND_COMPILE_FLAGS LIBUNWIND_HAS_W_FLAG -W)
+unwind_append_if(LIBUNWIND_COMPILE_FLAGS LIBUNWIND_HAS_WALL_FLAG -Wall)
+unwind_append_if(LIBUNWIND_COMPILE_FLAGS LIBUNWIND_HAS_WCHAR_SUBSCRIPTS_FLAG -Wchar-subscripts)
+unwind_app

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

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

Quuxplusone added inline comments.



Comment at: libcxx/include/memory:1645
 
-template 
+template 
 _LIBCPP_INLINE_VISIBILITY

ldionne wrote:
> Coming at it from a slightly different angle, I would think this is what we 
> want:
> 
> ```
> templateclass _RawSourceTp = typename remove_const<_SourceTp>::type,
>   class _RawDestTp = typename remove_const<_DestTp>::type>
> _LIBCPP_INLINE_VISIBILITY static typename enable_if<
>// 
> We can use memcpy instead of a loop with construct if...
> is_trivially_move_constructible<_DestTp>::value && // 
> - the Dest is trivially move constructible, and
> is_same<_RawSourceTp, _RawDestTp>::value &&// 
> - both types are the same modulo constness, and either
> (__is_default_allocator::value ||  // 
>   + the allocator is the default allocator (and we know `construct` is just 
> placement-new), or
>  !__has_construct::value), // 
>   + the allocator does not provide a custom `construct` method (so we'd fall 
> back to placement-new)
> void>::type
> __construct_range_forward(allocator_type&, _SourceTp* __begin1, _SourceTp* 
> __end1, _DestTp*& __begin2)
> {
> ptrdiff_t _Np = __end1 - __begin1;
> if (_Np > 0)
> {
> _VSTD::memcpy(const_cast<_RawDestTp*>(__begin2), __begin1, _Np * 
> sizeof(_DestTp));
> __begin2 += _Np;
> }
> }
> ```
> 
> And then we should have
> 
> ```
> template 
> struct __is_default_allocator : false_type { };
> 
> template 
> struct __is_default_allocator<_VSTD::allocator<_Tp> > : true_type { };
> ```
> 
> Does this make sense?
> 
> Also, I'm not sure I understand why we use `const_cast` on the destination 
> type. It seems like we should instead enforce that it is non-const? But this 
> is a pre-existing thing in the code, this doesn't affect this review.
> 
I agree that it is wrong to express the check in terms of 
`is_same>`; it should be expressed in terms of a 
trait which is satisfied by `std::allocator`-for-any-T. @ldionne expressed 
it in terms of `__is_default_allocator`. I continue to ask that it be 
expressed in terms of `__has_trivial_construct`, where 
libc++ specializes `__has_trivial_construct, ...>` if need 
be.

Orthogonally, the condition `__has_construct` is wrong because it has an extra `const`. It is conceivable 
— though of course implausible/pathological — for `construct(T*, T&)` to exist 
and do something different from `construct(T*, const T&)`.



Comment at: 
libcxx/test/std/containers/sequences/vector/vector.cons/construct_iter_iter.pass.cpp:196
   test_ctor_under_alloc();
+  test_ctor_with_different_value_type();
 }

I suggest that interesting test cases include "array of `int` to vector of 
`unsigned int`" (trivial, but unimplemented in this patch) and "array of 
`iostream*` to vector of `ostream*`" (non-trivial because each pointer must be 
adjusted).


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

https://reviews.llvm.org/D48342



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

[PATCH] D45898: [SemaCXX] Mark destructor as referenced

[Orivej Desh via Phabricator via cfe-commits]

orivej added a comment.

Actually, `arc-list-init-destruct.mm` crashes Clang 7 with the same backtrace 
as this test case, and Clang trunk generates similar assembly (to the the point 
that I could essentially copy the `// CHECK` comments from the .mm test to a 
.cpp test), so I'm not sure if adding a .cpp test is valuable…


Repository:
  rC Clang

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

https://reviews.llvm.org/D45898



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

[PATCH] D45898: [SemaCXX] Mark destructor as referenced

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Yeah, if we weren't exercising the code in our test suite, that would be one 
thing, but I think a language-mode-specific test probably isn't too valuable.


Repository:
  rC Clang

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

https://reviews.llvm.org/D45898



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

r348709 - SourceManager: insert(make_pair(..)) -> try_emplace. NFC

[Fangrui Song via cfe-commits]

Author: maskray
Date: Sat Dec  8 17:46:01 2018
New Revision: 348709

URL: http://llvm.org/viewvc/llvm-project?rev=348709&view=rev
Log:
SourceManager: insert(make_pair(..)) -> try_emplace. NFC

Modified:
cfe/trunk/lib/Basic/SourceManager.cpp

Modified: cfe/trunk/lib/Basic/SourceManager.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/SourceManager.cpp?rev=348709&r1=348708&r2=348709&view=diff
==
--- cfe/trunk/lib/Basic/SourceManager.cpp (original)
+++ cfe/trunk/lib/Basic/SourceManager.cpp Sat Dec  8 17:46:01 2018
@@ -195,8 +195,7 @@ llvm::MemoryBuffer *ContentCache::getBuf
 }
 
 unsigned LineTableInfo::getLineTableFilenameID(StringRef Name) {
-  auto IterBool =
-  FilenameIDs.insert(std::make_pair(Name, FilenamesByID.size()));
+  auto IterBool = FilenameIDs.try_emplace(Name, FilenamesByID.size());
   if (IterBool.second)
 FilenamesByID.push_back(&*IterBool.first);
   return IterBool.first->second;
@@ -1965,9 +1964,7 @@ SourceManager::getDecomposedIncludedLoc(
   // Uses IncludedLocMap to retrieve/cache the decomposed loc.
 
   using DecompTy = std::pair;
-  using MapTy = llvm::DenseMap;
-  std::pair
-InsertOp = IncludedLocMap.insert(std::make_pair(FID, DecompTy()));
+  auto InsertOp = IncludedLocMap.try_emplace(FID);
   DecompTy &DecompLoc = InsertOp.first->second;
   if (!InsertOp.second)
 return DecompLoc; // already in map.


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

[PATCH] D47196: [Time-report ](2): Recursive timers in Clang

[Trass3r via Phabricator via cfe-commits]

Trass3r added a comment.

Is this still in progress?


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

https://reviews.llvm.org/D47196



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

[PATCH] D55482: [clang-tidy] Improve google-objc-function-naming diagnostics 📙

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore created this revision.
stephanemoore added reviewers: benhamilton, aaron.ballman.
Herald added subscribers: cfe-commits, xazax.hun.

The diagnostics from google-objc-function-naming check will be more
actionable if they provide a brief description of the requirements from
the Google Objective-C style guide. The more descriptive diagnostics may
help clarify that functions in the global namespace must have an
appropriate prefix followed by Pascal case (engineers working previously
with static functions might not immediately understand the different
requirements of static and non-static functions).

Test Notes:
Verified against the clang-tidy tests.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D55482

Files:
  clang-tidy/google/FunctionNamingCheck.cpp
  test/clang-tidy/google-objc-function-naming.m
  test/clang-tidy/google-objc-function-naming.mm

Index: test/clang-tidy/google-objc-function-naming.mm
===
--- test/clang-tidy/google-objc-function-naming.mm
+++ test/clang-tidy/google-objc-function-naming.mm
@@ -1,16 +1,19 @@
 // RUN: %check_clang_tidy %s google-objc-function-naming %t
 
 void printSomething() {}
-// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'printSomething' not
-// using function naming conventions described by Google Objective-C style guide
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: global function name
+// 'printSomething' must have an appropriate prefix followed by Pascal case as
+// required by Google Objective-C style guide
 
 void PrintSomething() {}
-// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'PrintSomething' not
-// using function naming conventions described by Google Objective-C style guide
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: global function name
+// 'PrintSomething' must have an appropriate prefix followed by Pascal case as
+// required by Google Objective-C style guide
 
 void ABCBad_Name() {}
-// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'ABCBad_Name' not
-// using function naming conventions described by Google Objective-C style guide
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: global function name 'ABCBad_Name'
+// must have an appropriate prefix followed by Pascal case as required by Google
+// Objective-C style guide
 
 namespace {
 
Index: test/clang-tidy/google-objc-function-naming.m
===
--- test/clang-tidy/google-objc-function-naming.m
+++ test/clang-tidy/google-objc-function-naming.m
@@ -3,28 +3,35 @@
 typedef _Bool bool;
 
 static bool ispositive(int a) { return a > 0; }
-// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'ispositive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: static function name 'ispositive'
+// must be in Pascal case as required by Google Objective-C style guide
 // CHECK-FIXES: static bool Ispositive(int a) { return a > 0; }
 
 static bool is_positive(int a) { return a > 0; }
-// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'is_positive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: static function name 'is_positive'
+// must be in Pascal case as required by Google Objective-C style guide
 // CHECK-FIXES: static bool IsPositive(int a) { return a > 0; }
 
 static bool isPositive(int a) { return a > 0; }
-// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'isPositive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: static function name 'isPositive'
+// must be in Pascal case as required by Google Objective-C style guide
 // CHECK-FIXES: static bool IsPositive(int a) { return a > 0; }
 
 static bool Is_Positive(int a) { return a > 0; }
-// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'Is_Positive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: static function name 'Is_Positive'
+// must be in Pascal case as required by Google Objective-C style guide
 // CHECK-FIXES: static bool IsPositive(int a) { return a > 0; }
 
 static bool IsPositive(int a) { return a > 0; }
 
 bool ispalindrome(const char *str);
-// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'ispalindrome' not using function naming conventions described by Google Objective-C style guide
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: global function name 'ispalindrome'
+// must have an appropriate prefix followed by Pascal case as required by Google
+// Objective-C style guide
 
 static const char *md5(const char *str) { return 0; }
-// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: function name 'md5' not using function naming conventions described by Google Objective-C style guide
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning

[PATCH] D55483: Introduce the callback attribute and emit !callback metadata

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert created this revision.
jdoerfert added reviewers: hfinkel, reames, fhahn, ABataev.
Herald added subscribers: jfb, bollu.

  With commit rX (currently https://reviews.llvm.org/D54498), LLVM
  gained the ability to apply existing optimizations on indirections
  through callbacks. This is based on an abstraction that hides the
  middle man as described in rX and the llvm::AbstractCallSite class.
  
  This commit enables clang to emit !callback metadata that is
  understood by LLVM. It does so in three different cases:
1) For known broker functions declarations that are directly
   emitted, e.g., __kmpc_fork_call for the OpenMP pragma parallel.
2) For known broker functions that are identified by their name and
   source location through the builtin mechanism, e.g.,
   pthread_create from the POSIX thread API.
3) For user annotated functions that carry the "callback(idx, ...)"
   attribute. The attribute has to include the index of the callback
   callee and how the passed arguments can be identified (as many as
   the callback callee has).
  
  For additional information, also consider the commit message and
  discussion for the LLVM patch: https://reviews.llvm.org/D54498

NOTE: This is only committed after https://reviews.llvm.org/D54498 and
the commit message will be modified accordingly.


Repository:
  rC Clang

https://reviews.llvm.org/D55483

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/Builtins.def
  include/clang/Basic/Builtins.h
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Basic/Builtins.cpp
  lib/CodeGen/CGOpenMPRuntime.cpp
  lib/CodeGen/CodeGenModule.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/CodeGen/callback_annotated.c
  test/CodeGen/callback_openmp.c
  test/CodeGen/callback_pthread_create.c

Index: test/CodeGen/callback_pthread_create.c
===
--- /dev/null
+++ test/CodeGen/callback_pthread_create.c
@@ -0,0 +1,31 @@
+// RUN: %clang -O1 %s -S -c -emit-llvm -o - | FileCheck %s
+// RUN: %clang -O1 %s -S -c -emit-llvm -o - | opt -ipconstprop -S | FileCheck --check-prefix=IPCP %s
+
+// CHECK: declare !callback ![[cid:[0-9]+]] dso_local i32 @pthread_create
+// CHECK: ![[cid]] = !{i1 false, i64 3, i64 4}
+
+#include 
+
+const int GlobalVar = 0;
+
+static void *callee0(void *payload) {
+// IPCP:  define internal i8* @callee0
+// IPCP-NEXT:   entry:
+// IPCP-NEXT: ret i8* null
+  return payload;
+}
+
+static void *callee1(void *payload) {
+// IPCP:  define internal i8* @callee1
+// IPCP-NEXT:   entry:
+// IPCP-NEXT: ret i8* bitcast (i32* @GlobalVar to i8*)
+  return payload;
+}
+
+void foo() {
+  pthread_t MyFirstThread;
+  pthread_create(&MyFirstThread, NULL, callee0, NULL);
+
+  pthread_t MySecondThread;
+  pthread_create(&MySecondThread, NULL, callee1, (void *)&GlobalVar);
+}
Index: test/CodeGen/callback_openmp.c
===
--- /dev/null
+++ test/CodeGen/callback_openmp.c
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -triple i386-unknown-unknown -fopenmp -O1 %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple i386-unknown-unknown -fopenmp -O1 %s -emit-llvm -o - | opt -ipconstprop -S | FileCheck --check-prefix=IPCP %s
+
+// CHECK: declare !callback ![[cid:[0-9]+]] void @__kmpc_fork_call
+// CHECK: declare !callback ![[cid]] void @__kmpc_fork_teams
+// CHECK: ![[cid]] = !{i1 true, i64 3, i64 0, i64 0}
+
+void work1(int, int);
+void work2(int, int);
+void work12(int, int);
+
+void foo(int q) {
+  int p = 2;
+
+  #pragma omp parallel firstprivate(q, p)
+  work1(p, q);
+// IPCP: call void @work1(i32 2, i32 %{{[._a-zA-Z0-9]*}})
+
+  #pragma omp parallel for firstprivate(p, q)
+  for (int i = 0; i < q; i++)
+work2(i, p);
+// IPCP: call void @work2(i32 %{{[._a-zA-Z0-9]*}}, i32 2)
+
+  #pragma omp target teams firstprivate(p)
+  work12(p, p);
+// IPCP: call void @work12(i32 2, i32 2)
+}
Index: test/CodeGen/callback_annotated.c
===
--- /dev/null
+++ test/CodeGen/callback_annotated.c
@@ -0,0 +1,70 @@
+// RUN: %clang_cc1 -triple i386-unknown-unknown -fopenmp -O1 %s -emit-llvm -o - | FileCheck %s --check-prefix=RUN1
+// RUN: %clang_cc1 -triple i386-unknown-unknown -fopenmp -O1 %s -emit-llvm -o - | FileCheck %s --check-prefix=RUN2
+// RUN: %clang_cc1 -triple i386-unknown-unknown -fopenmp -O1 %s -emit-llvm -o - | opt -ipconstprop -S | FileCheck --check-prefix=IPCP %s
+
+// RUN1-DAG: @broker0({{[^#]*#[0-9]+}} !callback ![[cid0:[0-9]+]]
+__attribute__((callback (1, 2)))
+void* broker0(void* (*callee)(void *), void *payload) {
+  return callee(payload);
+}
+
+// RUN1-DAG: @broker1({{[^#]*#[0-9]+}} !callback ![[cid1:[0-9]+]]
+__attribute__((callback (2, 1)))
+void* broker1(void *payload, void* (*callee)(void *)) {
+  return broker0(callee, payload);
+}
+
+// RUN1-DAG: declare !callback ![[cid2:[0-9]+]] i8* @broker2
+__attribute_

[PATCH] D55484: ComputeLineNumbers: delete SSE2 vectorization

[Fangrui Song via Phabricator via cfe-commits]

MaskRay created this revision.
MaskRay added a reviewer: bkramer.
Herald added a subscriber: cfe-commits.

SSE2 vectorization was added in 2012, but it is 2018 now and I can't
observe any performance boost with the existing _mm_movemask_epi8 or the 
following SSE4.2 (compiling with -msse4.2):

  __m128i C = _mm_setr_epi8('\r','\n',0,0,0,0,0,0,0,0,0,0,0,0,0,0);
  _mm_cmpestri(C, 2, Chunk, 16, _SIDD_UBYTE_OPS | _SIDD_CMP_EQUAL_ANY | 
_SIDD_POSITIVE_POLARITY | _SIDD_LEAST_SIGNIFICANT)

Delete the vectorization to simplify the code.

Also don't check the line ending sequence \n\r


Repository:
  rC Clang

https://reviews.llvm.org/D55484

Files:
  lib/Basic/SourceManager.cpp


Index: lib/Basic/SourceManager.cpp
===
--- lib/Basic/SourceManager.cpp
+++ lib/Basic/SourceManager.cpp
@@ -1216,65 +1216,22 @@
 
   const unsigned char *Buf = (const unsigned char *)Buffer->getBufferStart();
   const unsigned char *End = (const unsigned char *)Buffer->getBufferEnd();
-  unsigned Offs = 0;
+  unsigned I = 0;
   while (true) {
 // Skip over the contents of the line.
-const unsigned char *NextBuf = (const unsigned char *)Buf;
-
-#ifdef __SSE2__
-// Try to skip to the next newline using SSE instructions. This is very
-// performance sensitive for programs with lots of diagnostics and in -E
-// mode.
-__m128i CRs = _mm_set1_epi8('\r');
-__m128i LFs = _mm_set1_epi8('\n');
-
-// First fix up the alignment to 16 bytes.
-while (((uintptr_t)NextBuf & 0xF) != 0) {
-  if (*NextBuf == '\n' || *NextBuf == '\r' || *NextBuf == '\0')
-goto FoundSpecialChar;
-  ++NextBuf;
-}
-
-// Scan 16 byte chunks for '\r' and '\n'. Ignore '\0'.
-while (NextBuf+16 <= End) {
-  const __m128i Chunk = *(const __m128i*)NextBuf;
-  __m128i Cmp = _mm_or_si128(_mm_cmpeq_epi8(Chunk, CRs),
- _mm_cmpeq_epi8(Chunk, LFs));
-  unsigned Mask = _mm_movemask_epi8(Cmp);
-
-  // If we found a newline, adjust the pointer and jump to the handling 
code.
-  if (Mask != 0) {
-NextBuf += llvm::countTrailingZeros(Mask);
-goto FoundSpecialChar;
-  }
-  NextBuf += 16;
-}
-#endif
-
-while (*NextBuf != '\n' && *NextBuf != '\r' && *NextBuf != '\0')
-  ++NextBuf;
-
-#ifdef __SSE2__
-FoundSpecialChar:
-#endif
-Offs += NextBuf-Buf;
-Buf = NextBuf;
-
-if (Buf[0] == '\n' || Buf[0] == '\r') {
-  // If this is \n\r or \r\n, skip both characters.
-  if ((Buf[1] == '\n' || Buf[1] == '\r') && Buf[0] != Buf[1]) {
-++Offs;
-++Buf;
-  }
-  ++Offs;
-  ++Buf;
-  LineOffsets.push_back(Offs);
+while (Buf[I] != '\n' && Buf[I] != '\r' && Buf[I] != '\0')
+  ++I;
+
+if (Buf[I] == '\n' || Buf[I] == '\r') {
+  // If this is \r\n, skip both characters.
+  if (Buf[I] == '\r' && Buf[I+1] == '\n')
+++I;
+  ++I;
+  LineOffsets.push_back(I);
 } else {
-  // Otherwise, this is a null.  If end of file, exit.
-  if (Buf == End) break;
-  // Otherwise, skip the null.
-  ++Offs;
-  ++Buf;
+  // Otherwise, this is a NUL. If end of file, exit.
+  if (Buf+I == End) break;
+  ++I;
 }
   }
 


Index: lib/Basic/SourceManager.cpp
===
--- lib/Basic/SourceManager.cpp
+++ lib/Basic/SourceManager.cpp
@@ -1216,65 +1216,22 @@
 
   const unsigned char *Buf = (const unsigned char *)Buffer->getBufferStart();
   const unsigned char *End = (const unsigned char *)Buffer->getBufferEnd();
-  unsigned Offs = 0;
+  unsigned I = 0;
   while (true) {
 // Skip over the contents of the line.
-const unsigned char *NextBuf = (const unsigned char *)Buf;
-
-#ifdef __SSE2__
-// Try to skip to the next newline using SSE instructions. This is very
-// performance sensitive for programs with lots of diagnostics and in -E
-// mode.
-__m128i CRs = _mm_set1_epi8('\r');
-__m128i LFs = _mm_set1_epi8('\n');
-
-// First fix up the alignment to 16 bytes.
-while (((uintptr_t)NextBuf & 0xF) != 0) {
-  if (*NextBuf == '\n' || *NextBuf == '\r' || *NextBuf == '\0')
-goto FoundSpecialChar;
-  ++NextBuf;
-}
-
-// Scan 16 byte chunks for '\r' and '\n'. Ignore '\0'.
-while (NextBuf+16 <= End) {
-  const __m128i Chunk = *(const __m128i*)NextBuf;
-  __m128i Cmp = _mm_or_si128(_mm_cmpeq_epi8(Chunk, CRs),
- _mm_cmpeq_epi8(Chunk, LFs));
-  unsigned Mask = _mm_movemask_epi8(Cmp);
-
-  // If we found a newline, adjust the pointer and jump to the handling code.
-  if (Mask != 0) {
-NextBuf += llvm::countTrailingZeros(Mask);
-goto FoundSpecialChar;
-  }
-  NextBuf += 16;
-}
-#endif
-
-while (*NextBuf != '\n' && *NextBuf != '\r' && *NextBuf != '\0')
-  ++NextBuf;
-
-#ifdef __SSE2__
-FoundSpecialChar:
-#endif
-Offs += NextB

[PATCH] D55307: [analyzer] MoveChecker Pt.6: Suppress the warning for the few move-safe STL classes.

[Aleksei Sidorin via Phabricator via cfe-commits]

a_sidorin accepted this revision.
a_sidorin added a comment.

I think the change is fine, just a minor stylish remark.




Comment at: test/Analysis/Inputs/system-header-simulator-cxx.h:782
+namespace std {
+  template // TODO: Implement the stub for deleter.
+  class unique_ptr {

Nit: our coding rules require a space before template lbrace.


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

https://reviews.llvm.org/D55307



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

[PATCH] D45898: [SemaCXX] Mark destructor as referenced

[Orivej Desh via Phabricator via cfe-commits]

orivej added a comment.
Herald added a subscriber: jkorous.

The committed test does not crash Clang 7, but the following test does, yet it 
compiles without any warnings by the current Clang trunk thanks to this fix.

  struct A { ~A(); };
  struct B : A {};
  struct C { C();  B b; };
  struct D { C c, d; };
  D f() { return {}; }


Repository:
  rC Clang

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

https://reviews.llvm.org/D45898



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

[PATCH] D45898: [SemaCXX] Mark destructor as referenced

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Always worth adding more tests.  Mind writing that one up as a commit?


Repository:
  rC Clang

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

https://reviews.llvm.org/D45898



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