Re: r258504 - Change of UserLabelPrefix default value from "_" to ""
Hi James, > I reverted this change with r258894, as it breaks (at least) sparc-rtems. Can you send me a reproducer, please? Right now I'm at a dead end -- no tests are broken, and no buildbots are reported any fails. Yet you claim that my patch breaks one target -- and there is zero info how to verify this. > Clearly this area of the code was not sufficiently covered by the testsuite. Not sure I understand what you mean -- it's actually covered quite extensively in test/Preprocessor/init.c. There is a line for generic SPARC as well: // SPARC:#define __USER_LABEL_PREFIX__ _ and I added initialization of UserLabelPrefix to "_" in SparcTargetInfo constructor specifically to accommodate this test. Perhaps you might want to add tests for sparc-rterms target as well? Yours, Andrey On Wed, Jan 27, 2016 at 4:10 AM, James Y Knight wrote: > I reverted this change with r258894, as it breaks (at least) sparc-rtems. > Clearly this area of the code was not sufficiently covered by the testsuite. > > On Fri, Jan 22, 2016 at 10:24 AM, Andrey Bokhanko via cfe-commits > wrote: >> >> Author: asbokhan >> Date: Fri Jan 22 09:24:34 2016 >> New Revision: 258504 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=258504&view=rev >> Log: >> Change of UserLabelPrefix default value from "_" to "" >> >> Differential Revision: http://reviews.llvm.org/D16295 >> >> Modified: >> cfe/trunk/lib/Basic/TargetInfo.cpp >> cfe/trunk/lib/Basic/Targets.cpp >> >> Modified: cfe/trunk/lib/Basic/TargetInfo.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/TargetInfo.cpp?rev=258504&r1=258503&r2=258504&view=diff >> >> == >> --- cfe/trunk/lib/Basic/TargetInfo.cpp (original) >> +++ cfe/trunk/lib/Basic/TargetInfo.cpp Fri Jan 22 09:24:34 2016 >> @@ -72,7 +72,7 @@ TargetInfo::TargetInfo(const llvm::Tripl >>DoubleFormat = &llvm::APFloat::IEEEdouble; >>LongDoubleFormat = &llvm::APFloat::IEEEdouble; >>DataLayoutString = nullptr; >> - UserLabelPrefix = "_"; >> + UserLabelPrefix = ""; >>MCountName = "mcount"; >>RegParmMax = 0; >>SSERegParmMax = 0; >> >> Modified: cfe/trunk/lib/Basic/Targets.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets.cpp?rev=258504&r1=258503&r2=258504&view=diff >> >> == >> --- cfe/trunk/lib/Basic/Targets.cpp (original) >> +++ cfe/trunk/lib/Basic/Targets.cpp Fri Jan 22 09:24:34 2016 >> @@ -102,9 +102,7 @@ protected: >> >> public: >>CloudABITargetInfo(const llvm::Triple &Triple) >> - : OSTargetInfo(Triple) { >> -this->UserLabelPrefix = ""; >> - } >> + : OSTargetInfo(Triple) {} >> }; >> >> static void getDarwinDefines(MacroBuilder &Builder, const LangOptions >> &Opts, >> @@ -242,6 +240,7 @@ public: >>this->TLSSupported = !Triple.isOSVersionLT(2); >> >> this->MCountName = "\01mcount"; >> +this->UserLabelPrefix = "_"; >>} >> >>std::string isValidSectionSpecifier(StringRef SR) const override { >> @@ -284,8 +283,6 @@ protected: >> public: >>DragonFlyBSDTargetInfo(const llvm::Triple &Triple) >>: OSTargetInfo(Triple) { >> -this->UserLabelPrefix = ""; >> - >> switch (Triple.getArch()) { >> default: >> case llvm::Triple::x86: >> @@ -327,8 +324,6 @@ protected: >>} >> public: >>FreeBSDTargetInfo(const llvm::Triple &Triple) : >> OSTargetInfo(Triple) { >> -this->UserLabelPrefix = ""; >> - >> switch (Triple.getArch()) { >> default: >> case llvm::Triple::x86: >> @@ -368,9 +363,7 @@ protected: >>} >> public: >>KFreeBSDTargetInfo(const llvm::Triple &Triple) >> - : OSTargetInfo(Triple) { >> -this->UserLabelPrefix = ""; >> - } >> + : OSTargetInfo(Triple) {} >> }; >> >> // Minix Target >> @@ -392,9 +385,7 @@ protected: >> DefineStd(Builder, "unix", Opts); >>} >> public: >> - MinixTargetInfo(const llvm::Triple &Triple) : >> OSTargetInfo(Triple) { >> -this->UserLabelPrefix = ""; >> - } >> + MinixTargetInfo(const llvm::Triple &Triple) : >> OSTargetInfo(Triple) {} >> }; >> >> // Linux target >> @@ -467,7 +458,6 @@ protected: >>} >> public: >>NetBSDTargetInfo(const llvm::Triple &Triple) : >> OSTargetInfo(Triple) { >> -this->UserLabelPrefix = ""; >> this->MCountName = "_mcount"; >>} >> }; >> @@ -488,7 +478,6 @@ protected: >>} >> public: >>OpenBSDTargetInfo(const llvm::Triple &Triple) : >> OSTargetInfo(Triple) { >> -this->UserLabelPrefix = ""; >> this->TLSSupported = false; >> >>switch (Triple.getArch()) { >> @@ -536,7 +525,6 @@ protected: >>} >> public: >>BitrigTargetInfo(const llvm::Triple &Triple) : >> OSTargetInfo(Triple) { >> -this->UserLabelPrefix = ""; >> this->MCountName = "__mcount"; >>} >> }; >> @@ -554,9 +542,7 @@ protected: >> Builder.defineMacro("__ELF__"); >>} >> public: >> - PSPTa
Re: [PATCH] D16408: [libcxx] Additional 'REQUIRE' directives for tests that require en_US.UTF-8.
dsanders added a comment. Thanks > @dsanders: What platform are you on that doesn't have en_US.UTF-8? If your > serious about testing libc++ you should install the locale. (but I understand > that's not always possible). It's Debian Jessie but it was configured for the Europe/London region during installation so I had en_GB.UTF-8 instead. I've installed en_US.UTF-8 (and the other missing locales like ru_RU.UTF-8) on all bar one of the machines I build releases on. The last machine lacks en_US.UTF-8 (but has the others) so I can test this patch before I commit it. After the commit I'll enable it. http://reviews.llvm.org/D16408 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r258918 - Update for LLVM change.
Author: d0k Date: Wed Jan 27 04:01:30 2016 New Revision: 258918 URL: http://llvm.org/viewvc/llvm-project?rev=258918&view=rev Log: Update for LLVM change. Modified: cfe/trunk/lib/Parse/ParseStmtAsm.cpp cfe/trunk/tools/driver/cc1as_main.cpp Modified: cfe/trunk/lib/Parse/ParseStmtAsm.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseStmtAsm.cpp?rev=258918&r1=258917&r2=258918&view=diff == --- cfe/trunk/lib/Parse/ParseStmtAsm.cpp (original) +++ cfe/trunk/lib/Parse/ParseStmtAsm.cpp Wed Jan 27 04:01:30 2016 @@ -23,10 +23,10 @@ #include "llvm/MC/MCInstrInfo.h" #include "llvm/MC/MCObjectFileInfo.h" #include "llvm/MC/MCParser/MCAsmParser.h" +#include "llvm/MC/MCParser/MCTargetAsmParser.h" #include "llvm/MC/MCRegisterInfo.h" #include "llvm/MC/MCStreamer.h" #include "llvm/MC/MCSubtargetInfo.h" -#include "llvm/MC/MCTargetAsmParser.h" #include "llvm/MC/MCTargetOptions.h" #include "llvm/Support/SourceMgr.h" #include "llvm/Support/TargetRegistry.h" Modified: cfe/trunk/tools/driver/cc1as_main.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/driver/cc1as_main.cpp?rev=258918&r1=258917&r2=258918&view=diff == --- cfe/trunk/tools/driver/cc1as_main.cpp (original) +++ cfe/trunk/tools/driver/cc1as_main.cpp Wed Jan 27 04:01:30 2016 @@ -30,10 +30,10 @@ #include "llvm/MC/MCInstrInfo.h" #include "llvm/MC/MCObjectFileInfo.h" #include "llvm/MC/MCParser/MCAsmParser.h" +#include "llvm/MC/MCParser/MCTargetAsmParser.h" #include "llvm/MC/MCRegisterInfo.h" #include "llvm/MC/MCStreamer.h" #include "llvm/MC/MCSubtargetInfo.h" -#include "llvm/MC/MCTargetAsmParser.h" #include "llvm/MC/MCTargetOptions.h" #include "llvm/Option/Arg.h" #include "llvm/Option/ArgList.h" ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libcxx] r258920 - [libcxx] Additional 'REQUIRE' directives for tests that require en_US.UTF-8.
Author: dsanders Date: Wed Jan 27 04:45:07 2016 New Revision: 258920 URL: http://llvm.org/viewvc/llvm-project?rev=258920&view=rev Log: [libcxx] Additional 'REQUIRE' directives for tests that require en_US.UTF-8. Summary: These are the tests that didn't fail in the release candidate because they were covered by another 'REQUIRES' directive. Reviewers: mclow.lists, hans, bcraig, EricWF Subscribers: EricWF, dim, cfe-commits Differential Revision: http://reviews.llvm.org/D16408 Modified: libcxx/trunk/test/std/localization/locale.categories/category.collate/locale.collate.byname/compare.pass.cpp libcxx/trunk/test/std/localization/locale.categories/category.ctype/locale.ctype.byname/tolower_1.pass.cpp libcxx/trunk/test/std/localization/locale.categories/category.ctype/locale.ctype.byname/tolower_many.pass.cpp libcxx/trunk/test/std/localization/locale.categories/category.ctype/locale.ctype.byname/toupper_1.pass.cpp libcxx/trunk/test/std/localization/locale.categories/category.ctype/locale.ctype.byname/toupper_many.pass.cpp libcxx/trunk/test/std/localization/locale.categories/category.ctype/locale.ctype.byname/widen_1.pass.cpp libcxx/trunk/test/std/localization/locale.categories/category.ctype/locale.ctype.byname/widen_many.pass.cpp Modified: libcxx/trunk/test/std/localization/locale.categories/category.collate/locale.collate.byname/compare.pass.cpp URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/localization/locale.categories/category.collate/locale.collate.byname/compare.pass.cpp?rev=258920&r1=258919&r2=258920&view=diff == --- libcxx/trunk/test/std/localization/locale.categories/category.collate/locale.collate.byname/compare.pass.cpp (original) +++ libcxx/trunk/test/std/localization/locale.categories/category.collate/locale.collate.byname/compare.pass.cpp Wed Jan 27 04:45:07 2016 @@ -7,6 +7,8 @@ // //===--===// +// REQUIRES: locale.en_US.UTF-8 + // // template class collate_byname Modified: libcxx/trunk/test/std/localization/locale.categories/category.ctype/locale.ctype.byname/tolower_1.pass.cpp URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/localization/locale.categories/category.ctype/locale.ctype.byname/tolower_1.pass.cpp?rev=258920&r1=258919&r2=258920&view=diff == --- libcxx/trunk/test/std/localization/locale.categories/category.ctype/locale.ctype.byname/tolower_1.pass.cpp (original) +++ libcxx/trunk/test/std/localization/locale.categories/category.ctype/locale.ctype.byname/tolower_1.pass.cpp Wed Jan 27 04:45:07 2016 @@ -7,6 +7,8 @@ // //===--===// +// REQUIRES: locale.en_US.UTF-8 + // // template class ctype_byname; Modified: libcxx/trunk/test/std/localization/locale.categories/category.ctype/locale.ctype.byname/tolower_many.pass.cpp URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/localization/locale.categories/category.ctype/locale.ctype.byname/tolower_many.pass.cpp?rev=258920&r1=258919&r2=258920&view=diff == --- libcxx/trunk/test/std/localization/locale.categories/category.ctype/locale.ctype.byname/tolower_many.pass.cpp (original) +++ libcxx/trunk/test/std/localization/locale.categories/category.ctype/locale.ctype.byname/tolower_many.pass.cpp Wed Jan 27 04:45:07 2016 @@ -7,6 +7,8 @@ // //===--===// +// REQUIRES: locale.en_US.UTF-8 + // // template class ctype_byname; Modified: libcxx/trunk/test/std/localization/locale.categories/category.ctype/locale.ctype.byname/toupper_1.pass.cpp URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/localization/locale.categories/category.ctype/locale.ctype.byname/toupper_1.pass.cpp?rev=258920&r1=258919&r2=258920&view=diff == --- libcxx/trunk/test/std/localization/locale.categories/category.ctype/locale.ctype.byname/toupper_1.pass.cpp (original) +++ libcxx/trunk/test/std/localization/locale.categories/category.ctype/locale.ctype.byname/toupper_1.pass.cpp Wed Jan 27 04:45:07 2016 @@ -7,6 +7,8 @@ // //===--===// +// REQUIRES: locale.en_US.UTF-8 + // // template class ctype_byname; Modified: libcxx/trunk/test/std/localization/locale.categories/category.ctype/locale.ctype.byname/toupper_many.pass.cpp URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/localization/locale.categories/category.ctype/locale.ctype.byname/toupper_many.pass.cpp?rev=258920&r1=258919&r2=258920&view=diff =
Re: [PATCH] D16584: [libcxx] Work around for clang calling GAS after having already failed.
dsanders added a comment. Thanks. Given that you're the patch author should I wait for someone else to LGTM too? http://reviews.llvm.org/D16584 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15120: Add support for __float128 type to be used by targets that support it
nemanjai added a comment. Thank you for the discussion on rank. I'm glad we have an agreement on the rank of the two types. However, considering __float128 already has a higher rank in this patch, is there anything else that you would like me to change here before the patch is approved? Do you suggest that we need to allow operations (or at least assignments) between the two types and take away the diagnostics that are part of this patch? Repository: rL LLVM http://reviews.llvm.org/D15120 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16298: Improve test coverage of -Wdouble-promotion
rob.lougher added a comment. Ping. http://reviews.llvm.org/D16298 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r258925 - [clang-tidy] Use relative URL for redirection.
Author: alexfh Date: Wed Jan 27 05:37:12 2016 New Revision: 258925 URL: http://llvm.org/viewvc/llvm-project?rev=258925&view=rev Log: [clang-tidy] Use relative URL for redirection. Modified: clang-tools-extra/trunk/docs/clang-tidy.rst Modified: clang-tools-extra/trunk/docs/clang-tidy.rst URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy.rst?rev=258925&r1=258924&r2=258925&view=diff == --- clang-tools-extra/trunk/docs/clang-tidy.rst (original) +++ clang-tools-extra/trunk/docs/clang-tidy.rst Wed Jan 27 05:37:12 2016 @@ -1,6 +1,6 @@ :orphan: .. meta:: - :http-equiv=refresh: 0;URL='http://clang.llvm.org/extra/clang-tidy/' + :http-equiv=refresh: 0;URL='clang-tidy/' clang-tidy documentation has moved here: http://clang.llvm.org/extra/clang-tidy/ ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r258924 - Add clang-tools-extra documentation to the CMake build.
Author: alexfh Date: Wed Jan 27 05:37:08 2016 New Revision: 258924 URL: http://llvm.org/viewvc/llvm-project?rev=258924&view=rev Log: Add clang-tools-extra documentation to the CMake build. Added: clang-tools-extra/trunk/docs/CMakeLists.txt clang-tools-extra/trunk/docs/doxygen.cfg.in clang-tools-extra/trunk/docs/doxygen.intro Modified: clang-tools-extra/trunk/CMakeLists.txt Modified: clang-tools-extra/trunk/CMakeLists.txt URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/CMakeLists.txt?rev=258924&r1=258923&r2=258924&view=diff == --- clang-tools-extra/trunk/CMakeLists.txt (original) +++ clang-tools-extra/trunk/CMakeLists.txt Wed Jan 27 05:37:08 2016 @@ -15,3 +15,10 @@ if(CLANG_ENABLE_STATIC_ANALYZER AND CLAN add_subdirectory(test) add_subdirectory(unittests) endif() + +option(CLANG_TOOLS_EXTRA_INCLUDE_DOCS "Generate build targets for the Clang Extra Tools docs." + ${LLVM_INCLUDE_DOCS}) +if( CLANG_TOOLS_EXTRA_INCLUDE_DOCS ) + add_subdirectory(docs) +endif() + Added: clang-tools-extra/trunk/docs/CMakeLists.txt URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/CMakeLists.txt?rev=258924&view=auto == --- clang-tools-extra/trunk/docs/CMakeLists.txt (added) +++ clang-tools-extra/trunk/docs/CMakeLists.txt Wed Jan 27 05:37:08 2016 @@ -0,0 +1,102 @@ +if (DOXYGEN_FOUND) + if (LLVM_ENABLE_DOXYGEN) +set(abs_srcdir ${CMAKE_CURRENT_SOURCE_DIR}) +set(abs_builddir ${CMAKE_CURRENT_BINARY_DIR}) + +if (HAVE_DOT) + set(DOT ${LLVM_PATH_DOT}) +endif() + +if (LLVM_DOXYGEN_EXTERNAL_SEARCH) + set(enable_searchengine "YES") + set(searchengine_url "${LLVM_DOXYGEN_SEARCHENGINE_URL}") + set(enable_server_based_search "YES") + set(enable_external_search "YES") + set(extra_search_mappings "${LLVM_DOXYGEN_SEARCH_MAPPINGS}") +else() + set(enable_searchengine "NO") + set(searchengine_url "") + set(enable_server_based_search "NO") + set(enable_external_search "NO") + set(extra_search_mappings "") +endif() + +# If asked, configure doxygen for the creation of a Qt Compressed Help file. +if (LLVM_ENABLE_DOXYGEN_QT_HELP) + set(CLANG_TOOLS_DOXYGEN_QCH_FILENAME "org.llvm.clang.qch" CACHE STRING +"Filename of the Qt Compressed help file") + set(CLANG_TOOLS_DOXYGEN_QHP_NAMESPACE "org.llvm.clang" CACHE STRING +"Namespace under which the intermediate Qt Help Project file lives") + set(CLANG_TOOLS_DOXYGEN_QHP_CUST_FILTER_NAME "Clang ${CLANG_VERSION}" CACHE STRING +"See http://qt-project.org/doc/qt-4.8/qthelpproject.html#custom-filters";) + set(CLANG_TOOLS_DOXYGEN_QHP_CUST_FILTER_ATTRS "Clang,${CLANG_VERSION}" CACHE STRING +"See http://qt-project.org/doc/qt-4.8/qthelpproject.html#filter-attributes";) + set(clang_tools_doxygen_generate_qhp "YES") + set(clang_tools_doxygen_qch_filename "${CLANG_DOXYGEN_QCH_FILENAME}") + set(clang_tools_doxygen_qhp_namespace "${CLANG_DOXYGEN_QHP_NAMESPACE}") + set(clang_tools_doxygen_qhelpgenerator_path "${LLVM_DOXYGEN_QHELPGENERATOR_PATH}") + set(clang_tools_doxygen_qhp_cust_filter_name "${CLANG_DOXYGEN_QHP_CUST_FILTER_NAME}") + set(clang_tools_doxygen_qhp_cust_filter_attrs "${CLANG_DOXYGEN_QHP_CUST_FILTER_ATTRS}") +else() + set(clang_tools_doxygen_generate_qhp "NO") + set(clang_tools_doxygen_qch_filename "") + set(clang_tools_doxygen_qhp_namespace "") + set(clang_tools_doxygen_qhelpgenerator_path "") + set(clang_tools_doxygen_qhp_cust_filter_name "") + set(clang_tools_doxygen_qhp_cust_filter_attrs "") +endif() + +option(LLVM_DOXYGEN_SVG + "Use svg instead of png files for doxygen graphs." OFF) +if (LLVM_DOXYGEN_SVG) + set(DOT_IMAGE_FORMAT "svg") +else() + set(DOT_IMAGE_FORMAT "png") +endif() + +configure_file(${CMAKE_CURRENT_SOURCE_DIR}/doxygen.cfg.in + ${CMAKE_CURRENT_BINARY_DIR}/doxygen.cfg @ONLY) + +set(abs_top_srcdir) +set(abs_top_builddir) +set(DOT) +set(enable_searchengine) +set(searchengine_url) +set(enable_server_based_search) +set(enable_external_search) +set(extra_search_mappings) +set(clang_tools_doxygen_generate_qhp) +set(clang_tools_doxygen_qch_filename) +set(clang_tools_doxygen_qhp_namespace) +set(clang_tools_doxygen_qhelpgenerator_path) +set(clang_tools_doxygen_qhp_cust_filter_name) +set(clang_tools_doxygen_qhp_cust_filter_attrs) +set(DOT_IMAGE_FORMAT) + +add_custom_target(doxygen-clang-tools + COMMAND ${DOXYGEN_EXECUTABLE} ${CMAKE_CURRENT_BINARY_DIR}/doxygen.cfg + WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} + COMMENT "Generating clang doxygen documentation." VERBATIM) + +if (LLVM_BUILD_DOCS) + add_dependencies(doxygen doxygen-clang-tools) +endif() +
[clang-tools-extra] r258926 - [clang-tidy] Fix documentation.
Author: alexfh Date: Wed Jan 27 05:37:19 2016 New Revision: 258926 URL: http://llvm.org/viewvc/llvm-project?rev=258926&view=rev Log: [clang-tidy] Fix documentation. Fixed broken links to cppcoreguidelines (anchors specified in the .md file should be used, not automatic anchors generated by github). Fixed formatting, array_view -> span, fixed sphinx errors in misc-definitions-in-headers.rst. Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-array-to-pointer-decay.rst clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-constant-array-index.rst clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-pointer-arithmetic.rst clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-pro-type-const-cast.rst clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-pro-type-cstyle-cast.rst clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-pro-type-reinterpret-cast.rst clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-pro-type-static-cast-downcast.rst clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-pro-type-union-access.rst clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-pro-type-vararg.rst clang-tools-extra/trunk/docs/clang-tidy/checks/misc-definitions-in-headers.rst Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-array-to-pointer-decay.rst URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-array-to-pointer-decay.rst?rev=258926&r1=258925&r2=258926&view=diff == --- clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-array-to-pointer-decay.rst (original) +++ clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-array-to-pointer-decay.rst Wed Jan 27 05:37:19 2016 @@ -5,7 +5,8 @@ cppcoreguidelines-pro-bounds-array-to-po This check flags all array to pointer decays. -Pointers should not be used as arrays. array_view is a bounds-checked, safe alternative to using pointers to access arrays. +Pointers should not be used as arrays. ``span`` is a bounds-checked, safe +alternative to using pointers to access arrays. This rule is part of the "Bounds safety" profile of the C++ Core Guidelines, see -https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#-bounds3-no-array-to-pointer-decay +https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Pro-bounds-decay. Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-constant-array-index.rst URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-constant-array-index.rst?rev=258926&r1=258925&r2=258926&view=diff == --- clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-constant-array-index.rst (original) +++ clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-constant-array-index.rst Wed Jan 27 05:37:19 2016 @@ -4,13 +4,13 @@ cppcoreguidelines-pro-bounds-constant-ar = This check flags all array subscript expressions on static arrays and -std::arrays that either do not have a constant integer expression index or -are out of bounds (for std::array). For out-of-bounds checking of static +``std::arrays`` that either do not have a constant integer expression index or +are out of bounds (for ``std::array``). For out-of-bounds checking of static arrays, see the clang-diagnostic-array-bounds check. The check can generate fixes after the option -cppcoreguidelines-pro-bounds-constant-array-index.GslHeader has been -set to the name of the include file that contains gsl::at(), e.g. "gsl/gsl.h". +``cppcoreguidelines-pro-bounds-constant-array-index.GslHeader`` has been set to +the name of the include file that contains ``gsl::at()``, e.g. ``"gsl/gsl.h"``. This rule is part of the "Bounds safety" profile of the C++ Core Guidelines, see -https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#-bounds2-only-index-into-arrays-using-constant-expressions. +https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Pro-bounds-arrayindex. Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-pointer-arithmetic.rst URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-pointer-arithmetic.rst?rev=258926&r1=258925&r2=258926&view=diff == --- clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-pointer-arithme
Re: [PATCH] D15721: [Sema] Fix ICE on casting a vector of bools to a vector of T
Anastasia accepted this revision. Anastasia added a comment. This revision is now accepted and ready to land. Seems sensible. LGTM! http://reviews.llvm.org/D15721 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16219: PR8901: attribute "mode" rejected for enums and dependent types
d.zobnin.bugzilla added a comment. Friendly ping, please take a look. http://reviews.llvm.org/D16219 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16351: [FIX] Bug 25404 - Crash on typedef in OpenCL 2.0
ichesnokov marked an inline comment as done. Comment at: lib/Sema/SemaDecl.cpp:2039 @@ +2038,3 @@ + if (Old->isImplicit() || New->isImplicit()) { + return; + } asl wrote: > Please follow the LLVM's coding style guidelines. E.g. *never* use tabs. It is not tab, it is triple space http://reviews.llvm.org/D16351 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r258307 - [OPENMP 4.0] Fix for codegen of 'cancel' directive within 'sections' directive.
Tested the attached patch which contains a back port of the net changes from both r258307 and Author: abataev Date: Fri Jan 22 02:56:50 2016 New Revision: 258495 URL: http://llvm.org/viewvc/llvm-project?rev=258495&view=rev Log: [OPENMP] Generalize codegen for 'sections'-based directive. If 'sections' directive has only one sub-section, the code for 'single'-based directive was emitted. Removed this codegen, because it causes crashes in different cases. Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp cfe/trunk/test/OpenMP/cancel_codegen.cpp cfe/trunk/test/OpenMP/cancellation_point_codegen.cpp cfe/trunk/test/OpenMP/parallel_sections_codegen.cpp cfe/trunk/test/OpenMP/sections_codegen.cpp cfe/trunk/test/OpenMP/sections_firstprivate_codegen.cpp cfe/trunk/test/OpenMP/sections_lastprivate_codegen.cpp cfe/trunk/test/OpenMP/sections_private_codegen.cpp cfe/trunk/test/OpenMP/sections_reduction_codegen.cpp on x86_64-apple-darwin15 without regressions in the cfe or libomp test suites. Jack ps Also verified that no regressions occur in the OpenMP3.1_Validation test suite. On Tue, Jan 26, 2016 at 2:23 PM, Hans Wennborg wrote: > Did that fix land, and should it be merged to 3.8? > > On Thu, Jan 21, 2016 at 7:03 PM, Alexey Bataev wrote: >> Later today I will post another fix, that will fix all 'sections' >> related troubles, including this one. So I don't think it is necessary >> to merge it >> >> Best regards, >> Alexey Bataev >> = >> Software Engineer >> Intel Compiler Team >> >> 22.01.2016 0:10, Hans Wennborg пишет: >>> Jack suggested (https://llvm.org/bugs/show_bug.cgi?id=26059#c7) that >>> this should be merged to 3.8. >>> >>> Alexey, you're the code owner here. OK for merging? If yes, do you >>> want to go ahead and merge with utils/release/merge.sh? >>> >>> On Wed, Jan 20, 2016 at 4:29 AM, Alexey Bataev via cfe-commits >>> wrote: Author: abataev Date: Wed Jan 20 06:29:47 2016 New Revision: 258307 URL: http://llvm.org/viewvc/llvm-project?rev=258307&view=rev Log: [OPENMP 4.0] Fix for codegen of 'cancel' directive within 'sections' directive. Allow to emit code for 'cancel' directive within 'sections' directive with single sub-section. Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp cfe/trunk/test/OpenMP/cancel_codegen.cpp Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=258307&r1=258306&r2=258307&view=diff == --- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original) +++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Wed Jan 20 06:29:47 2016 @@ -3685,8 +3685,6 @@ void CGOpenMPRuntime::emitCancelCall(Cod // kmp_int32 cncl_kind); if (auto *OMPRegionInfo = dyn_cast_or_null(CGF.CapturedStmtInfo)) { -if (OMPRegionInfo->getDirectiveKind() == OMPD_single) - return; auto &&ThenGen = [this, Loc, CancelRegion, OMPRegionInfo](CodeGenFunction &CGF) { llvm::Value *Args[] = { Modified: cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp?rev=258307&r1=258306&r2=258307&view=diff == --- cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp (original) +++ cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp Wed Jan 20 06:29:47 2016 @@ -1786,7 +1786,11 @@ CodeGenFunction::EmitSections(const OMPE CGF.EmitOMPPrivateClause(S, SingleScope); (void)SingleScope.Privatize(); +auto Exit = CGF.getJumpDestInCurrentScope("omp.sections.exit"); +CGF.BreakContinueStack.push_back(BreakContinue(Exit, Exit)); CGF.EmitStmt(Stmt); +CGF.EmitBlock(Exit.getBlock()); +CGF.BreakContinueStack.pop_back(); }; CGM.getOpenMPRuntime().emitSingleRegion(*this, CodeGen, S.getLocStart(), llvm::None, llvm::None, llvm::None, @@ -2647,7 +2651,8 @@ CodeGenFunction::getOMPCancelDestination if (Kind == OMPD_parallel || Kind == OMPD_task) return ReturnBlock; assert(Kind == OMPD_for || Kind == OMPD_section || Kind == OMPD_sections || - Kind == OMPD_parallel_sections || Kind == OMPD_parallel_for); + Kind == OMPD_parallel_sections || Kind == OMPD_parallel_for || + Kind == OMPD_single); return BreakContinueStack.back().BreakBlock; } Modified: cfe/trunk/test/OpenMP/cancel_codegen.cpp URL: http://llvm.org/viewvc/ll
Re: r258504 - Change of UserLabelPrefix default value from "_" to ""
Imagine there's a 2d table of values for UserLabelPrefix, each row for CPU and column for OS. The value of many if those cells was changed by this commit, because you stopped painting columns as "". That is, originally, the default entry was "_", then cpu rows were filled, and then the os columns were painted on top of that. Now, the Sparc row gets painted "_", but the rtems column isn't painted on top anymore. So, Sparc+rtems gets the wrong value. As to test cases, yes, I agree, I will add some more preprocessor tests for other OS/CPU combinations. On Jan 27, 2016 3:12 AM, "Andrey Bokhanko" wrote: > Hi James, > > > I reverted this change with r258894, as it breaks (at least) sparc-rtems. > > Can you send me a reproducer, please? > > Right now I'm at a dead end -- no tests are broken, and no buildbots > are reported any fails. Yet you claim that my patch breaks one target > -- and there is zero info how to verify this. > > > Clearly this area of the code was not sufficiently covered by the > testsuite. > > Not sure I understand what you mean -- it's actually covered quite > extensively in test/Preprocessor/init.c. There is a line for generic > SPARC as well: > > // SPARC:#define __USER_LABEL_PREFIX__ _ > > and I added initialization of UserLabelPrefix to "_" in > SparcTargetInfo constructor specifically to accommodate this test. > > Perhaps you might want to add tests for sparc-rterms target as well? > > Yours, > Andrey > > > On Wed, Jan 27, 2016 at 4:10 AM, James Y Knight > wrote: > > I reverted this change with r258894, as it breaks (at least) sparc-rtems. > > Clearly this area of the code was not sufficiently covered by the > testsuite. > > > > On Fri, Jan 22, 2016 at 10:24 AM, Andrey Bokhanko via cfe-commits > > wrote: > >> > >> Author: asbokhan > >> Date: Fri Jan 22 09:24:34 2016 > >> New Revision: 258504 > >> > >> URL: http://llvm.org/viewvc/llvm-project?rev=258504&view=rev > >> Log: > >> Change of UserLabelPrefix default value from "_" to "" > >> > >> Differential Revision: http://reviews.llvm.org/D16295 > >> > >> Modified: > >> cfe/trunk/lib/Basic/TargetInfo.cpp > >> cfe/trunk/lib/Basic/Targets.cpp > >> > >> Modified: cfe/trunk/lib/Basic/TargetInfo.cpp > >> URL: > >> > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/TargetInfo.cpp?rev=258504&r1=258503&r2=258504&view=diff > >> > >> > == > >> --- cfe/trunk/lib/Basic/TargetInfo.cpp (original) > >> +++ cfe/trunk/lib/Basic/TargetInfo.cpp Fri Jan 22 09:24:34 2016 > >> @@ -72,7 +72,7 @@ TargetInfo::TargetInfo(const llvm::Tripl > >>DoubleFormat = &llvm::APFloat::IEEEdouble; > >>LongDoubleFormat = &llvm::APFloat::IEEEdouble; > >>DataLayoutString = nullptr; > >> - UserLabelPrefix = "_"; > >> + UserLabelPrefix = ""; > >>MCountName = "mcount"; > >>RegParmMax = 0; > >>SSERegParmMax = 0; > >> > >> Modified: cfe/trunk/lib/Basic/Targets.cpp > >> URL: > >> > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets.cpp?rev=258504&r1=258503&r2=258504&view=diff > >> > >> > == > >> --- cfe/trunk/lib/Basic/Targets.cpp (original) > >> +++ cfe/trunk/lib/Basic/Targets.cpp Fri Jan 22 09:24:34 2016 > >> @@ -102,9 +102,7 @@ protected: > >> > >> public: > >>CloudABITargetInfo(const llvm::Triple &Triple) > >> - : OSTargetInfo(Triple) { > >> -this->UserLabelPrefix = ""; > >> - } > >> + : OSTargetInfo(Triple) {} > >> }; > >> > >> static void getDarwinDefines(MacroBuilder &Builder, const LangOptions > >> &Opts, > >> @@ -242,6 +240,7 @@ public: > >>this->TLSSupported = !Triple.isOSVersionLT(2); > >> > >> this->MCountName = "\01mcount"; > >> +this->UserLabelPrefix = "_"; > >>} > >> > >>std::string isValidSectionSpecifier(StringRef SR) const override { > >> @@ -284,8 +283,6 @@ protected: > >> public: > >>DragonFlyBSDTargetInfo(const llvm::Triple &Triple) > >>: OSTargetInfo(Triple) { > >> -this->UserLabelPrefix = ""; > >> - > >> switch (Triple.getArch()) { > >> default: > >> case llvm::Triple::x86: > >> @@ -327,8 +324,6 @@ protected: > >>} > >> public: > >>FreeBSDTargetInfo(const llvm::Triple &Triple) : > >> OSTargetInfo(Triple) { > >> -this->UserLabelPrefix = ""; > >> - > >> switch (Triple.getArch()) { > >> default: > >> case llvm::Triple::x86: > >> @@ -368,9 +363,7 @@ protected: > >>} > >> public: > >>KFreeBSDTargetInfo(const llvm::Triple &Triple) > >> - : OSTargetInfo(Triple) { > >> -this->UserLabelPrefix = ""; > >> - } > >> + : OSTargetInfo(Triple) {} > >> }; > >> > >> // Minix Target > >> @@ -392,9 +385,7 @@ protected: > >> DefineStd(Builder, "unix", Opts); > >>} > >> public: > >> - MinixTargetInfo(const llvm::Triple &Triple) : > >> OSTargetInfo(Triple) { > >> -this->UserLabelPrefix = ""; > >> - } > >> + MinixTargetInfo(const ll
[PATCH] D16626: [x86] Correct setting of WIntType for MCU target
andreybokhanko created this revision. andreybokhanko added reviewers: rnk, mkuper, rafael. andreybokhanko added a subscriber: cfe-commits. Correct setting of WIntType for MCU target http://reviews.llvm.org/D16626 Files: lib/Basic/Targets.cpp test/Preprocessor/elfiamcu-predefines.c Index: lib/Basic/Targets.cpp === --- lib/Basic/Targets.cpp +++ lib/Basic/Targets.cpp @@ -3885,6 +3885,7 @@ LongDoubleWidth = 64; LongDoubleFormat = &llvm::APFloat::IEEEdouble; UserLabelPrefix = ""; +WIntType = UnsignedInt; } CallingConvCheckResult checkCallingConvention(CallingConv CC) const override { Index: test/Preprocessor/elfiamcu-predefines.c === --- test/Preprocessor/elfiamcu-predefines.c +++ test/Preprocessor/elfiamcu-predefines.c @@ -1,6 +1,7 @@ // RUN: %clang_cc1 -E -dM -triple i586-intel-elfiamcu | FileCheck %s // CHECK: #define __USER_LABEL_PREFIX__ {{$}} +// CHECK: #define __WINT_TYPE__ unsigned int // CHECK: #define __iamcu // CHECK: #define __iamcu__ Index: lib/Basic/Targets.cpp === --- lib/Basic/Targets.cpp +++ lib/Basic/Targets.cpp @@ -3885,6 +3885,7 @@ LongDoubleWidth = 64; LongDoubleFormat = &llvm::APFloat::IEEEdouble; UserLabelPrefix = ""; +WIntType = UnsignedInt; } CallingConvCheckResult checkCallingConvention(CallingConv CC) const override { Index: test/Preprocessor/elfiamcu-predefines.c === --- test/Preprocessor/elfiamcu-predefines.c +++ test/Preprocessor/elfiamcu-predefines.c @@ -1,6 +1,7 @@ // RUN: %clang_cc1 -E -dM -triple i586-intel-elfiamcu | FileCheck %s // CHECK: #define __USER_LABEL_PREFIX__ {{$}} +// CHECK: #define __WINT_TYPE__ unsigned int // CHECK: #define __iamcu // CHECK: #define __iamcu__ ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16012: Carry raw string literal information through to the AST StringLiteral representation
aaron.ballman added a comment. In http://reviews.llvm.org/D16012#331219, @aaron.ballman wrote: > Ping Ping http://reviews.llvm.org/D16012 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16012: Carry raw string literal information through to the AST StringLiteral representation
What's the benefit of storing this? You can get the same effect by re-lexing. We don't guarantee that the pretty printed version of the AST comprises the same sequence of tokens in general. On 27 Jan 2016 3:03 p.m., "Aaron Ballman via cfe-commits" < cfe-commits@lists.llvm.org> wrote: > aaron.ballman added a comment. > > In http://reviews.llvm.org/D16012#331219, @aaron.ballman wrote: > > > Ping > > > Ping > > > http://reviews.llvm.org/D16012 > > > > ___ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16591: Add backend dignostic printer for unsupported features
olista01 removed rL LLVM as the repository for this revision. olista01 updated this revision to Diff 46127. olista01 added a comment. Added a test for the new diagnostic printer. http://reviews.llvm.org/D16591 Files: include/clang/Basic/DiagnosticFrontendKinds.td lib/CodeGen/CodeGenAction.cpp test/CodeGen/backend-unsupported-error.ll test/Frontend/optimization-remark-analysis.c test/Misc/backend-optimization-failure-nodbg.cpp Index: test/Misc/backend-optimization-failure-nodbg.cpp === --- test/Misc/backend-optimization-failure-nodbg.cpp +++ test/Misc/backend-optimization-failure-nodbg.cpp @@ -4,7 +4,7 @@ // Test verifies optimization failures generated by the backend are handled // correctly by clang. LLVM tests verify all of the failure conditions. -void test_switch(int *A, int *B, int Length) { +void test_switch(int *A, int *B, int Length) { /* expected-warning {{loop not vectorized: failed explicitly specified loop vectorization}} */ #pragma clang loop vectorize(enable) unroll(disable) for (int i = 0; i < Length; i++) { switch (A[i]) { @@ -18,4 +18,4 @@ B[i] = 3; } } -/* expected-warning {{loop not vectorized: failed explicitly specified loop vectorization}} */ } +} Index: test/Frontend/optimization-remark-analysis.c === --- test/Frontend/optimization-remark-analysis.c +++ test/Frontend/optimization-remark-analysis.c @@ -1,8 +1,8 @@ // RUN: %clang -O1 -fvectorize -target x86_64-unknown-unknown -emit-llvm -Rpass-analysis -S %s -o - 2>&1 | FileCheck %s --check-prefix=RPASS // RUN: %clang -O1 -fvectorize -target x86_64-unknown-unknown -emit-llvm -S %s -o - 2>&1 | FileCheck %s -// RPASS: {{.*}}:21:1: remark: loop not vectorized: loop contains a switch statement -// CHECK-NOT: {{.*}}:21:1: remark: loop not vectorized: loop contains a switch statement +// RPASS: {{.*}}:7:8: remark: loop not vectorized: loop contains a switch statement +// CHECK-NOT: {{.*}}:7:8: remark: loop not vectorized: loop contains a switch statement double foo(int N, int *Array) { double v = 0.0; Index: test/CodeGen/backend-unsupported-error.ll === --- /dev/null +++ test/CodeGen/backend-unsupported-error.ll @@ -0,0 +1,45 @@ +; RUN: not %clang_cc1 -triple r600-unknown-unknown -S -o - %s 2>&1 | FileCheck %s +; REQUIRES: amdgpu-registered-target + +; This is to check that backend errors for unsupported features are formatted correctly + +; CHECK: error: test.c:2:20: in function bar i32 (): unsupported call to function foo.2 + +target triple = "r600-unknown-unknown" + +; Function Attrs: nounwind uwtable +define i32 @bar() #0 !dbg !4 { +entry: + %call = call i32 @foo(), !dbg !12 + ret i32 %call, !dbg !13 +} + +; Function Attrs: nounwind uwtable +define i32 @foo() #0 !dbg !8 { +entry: + %call = call i32 @bar(), !dbg !14 + ret i32 %call, !dbg !15 +} + +attributes #0 = { nounwind uwtable "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "stack-protector-buffer-size"="8" "unsafe-fp-math"="false" "use-soft-float"="false" } + +!llvm.dbg.cu = !{!0} +!llvm.module.flags = !{!9, !10} +!llvm.ident = !{!11} + +!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 3.9.0", isOptimized: false, runtimeVersion: 0, emissionKind: 1, enums: !2, subprograms: !3) +!1 = !DIFile(filename: "test.c", directory: "") +!2 = !{} +!3 = !{!4, !8} +!4 = distinct !DISubprogram(name: "bar", scope: !1, file: !1, line: 2, type: !5, isLocal: false, isDefinition: true, scopeLine: 2, isOptimized: false, variables: !2) +!5 = !DISubroutineType(types: !6) +!6 = !{!7} +!7 = !DIBasicType(name: "int", size: 32, align: 32, encoding: DW_ATE_signed) +!8 = distinct !DISubprogram(name: "foo", scope: !1, file: !1, line: 3, type: !5, isLocal: false, isDefinition: true, scopeLine: 3, flags: DIFlagPrototyped, isOptimized: false, variables: !2) +!9 = !{i32 2, !"Dwarf Version", i32 4} +!10 = !{i32 2, !"Debug Info Version", i32 3} +!11 = !{!"clang version 3.9.0"} +!12 = !DILocation(line: 2, column: 20, scope: !4) +!13 = !DILocation(line: 2, column: 13, scope: !4) +!14 = !DILocation(line: 3, column: 20, scope: !8) +!15 = !DILocation(line: 3, column: 13, scope: !8) Index: lib/CodeGen/CodeGenAction.cpp === --- lib/CodeGen/CodeGenAction.cpp +++ lib/CodeGen/CodeGenAction.cpp @@ -238,6 +238,13 @@ ((BackendConsumer *)Context)->DiagnosticHandlerImpl(DI); } +/// Get the best possible source location to represent a diagnostic that +/// may have associated debug info. +const FullSourceLoc +getBestLocationFromDebugLoc(const llvm::DiagnosticInfoWithDebugLocBase &D, +bool &BadDebugInfo, Str
Re: [PATCH] D16012: Carry raw string literal information through to the AST StringLiteral representation
On Wed, Jan 27, 2016 at 9:08 AM, Richard Smith wrote: > What's the benefit of storing this? You can get the same effect by > re-lexing. We don't guarantee that the pretty printed version of the AST > comprises the same sequence of tokens in general. So we don't lose information the user wrote. For instance, this can be used by http://reviews.llvm.org/D16529 or other clang-tidy checks. ~Aaron > > On 27 Jan 2016 3:03 p.m., "Aaron Ballman via cfe-commits" > wrote: >> >> aaron.ballman added a comment. >> >> In http://reviews.llvm.org/D16012#331219, @aaron.ballman wrote: >> >> > Ping >> >> >> Ping >> >> >> http://reviews.llvm.org/D16012 >> >> >> >> ___ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15120: Add support for __float128 type to be used by targets that support it
hubert.reinterpretcast added a comment. In http://reviews.llvm.org/D15120#337144, @nemanjai wrote: > Do you suggest that we need to allow operations (or at least assignments) > between the two types and take away the diagnostics that are part of this > patch? My overriding concern at this time is to allow (or not) the operations consistently (that is, not making it platform-specific). The net result is that we will allow operations between the two types; however, we will encounter a further issue to discuss on what the common type should be when the representations are the same (the TS prefers `long double`). As for the diagnostics, the `__float128 Repository: rL LLVM http://reviews.llvm.org/D15120 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15120: Add support for __float128 type to be used by targets that support it
hubert.reinterpretcast added a comment. In http://reviews.llvm.org/D15120#337144, @nemanjai wrote: > Do you suggest that we need to allow operations (or at least assignments) > between the two types and take away the diagnostics that are part of this > patch? My overriding concern at this time is to allow (or not) the operations consistently (that is, not making it platform-specific). The net result is that we will allow operations between the two types; however, we will encounter a further issue to discuss (@rjmccall @rsmith; your input requested) regarding what the common type should be when the representations are the same (the TS prefers the standard floating type, which is `long double` here). As for the diagnostics, the IEEEquad to PPCDoubleDouble conversion still requires checks for narrowing. Repository: rL LLVM http://reviews.llvm.org/D15120 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D16628: clang-cl: support __cdecl-on-struct anachronism
sberg created this revision. sberg added a reviewer: rnk. sberg added a subscriber: cfe-commits. The Microsoft compiler emits warning C4229: anachronism used : modifiers on data are ignored for struct {} __cdecl s; but ICU's gendict can generate such (and does when building LibreOffice), so accepting this in clang-cl too would be useful. http://reviews.llvm.org/D16628 Files: lib/Parse/ParseDeclCXX.cpp test/Parser/ms-anachronism.c Index: test/Parser/ms-anachronism.c === --- /dev/null +++ test/Parser/ms-anachronism.c @@ -0,0 +1,3 @@ +// RUN: %clang_cc1 -triple i686-windows-msvc -fms-extensions -fsyntax-only -verify %s + +struct {} __cdecl s; // expected-warning {{'__cdecl' only applies to function types; type here is 'struct}} Index: lib/Parse/ParseDeclCXX.cpp === --- lib/Parse/ParseDeclCXX.cpp +++ lib/Parse/ParseDeclCXX.cpp @@ -1103,6 +1103,11 @@ return true; case tok::colon: return CouldBeBitfield; // enum E { ... } : 2; + // Microsoft compatibility + case tok::kw___cdecl: // struct foo {...} __cdecl x; // C4229 +if (!getLangOpts().MicrosoftExt) + break; +// fall through // Type qualifiers case tok::kw_const: // struct foo {...} const x; case tok::kw_volatile:// struct foo {...} volatile x; Index: test/Parser/ms-anachronism.c === --- /dev/null +++ test/Parser/ms-anachronism.c @@ -0,0 +1,3 @@ +// RUN: %clang_cc1 -triple i686-windows-msvc -fms-extensions -fsyntax-only -verify %s + +struct {} __cdecl s; // expected-warning {{'__cdecl' only applies to function types; type here is 'struct}} Index: lib/Parse/ParseDeclCXX.cpp === --- lib/Parse/ParseDeclCXX.cpp +++ lib/Parse/ParseDeclCXX.cpp @@ -1103,6 +1103,11 @@ return true; case tok::colon: return CouldBeBitfield; // enum E { ... } : 2; + // Microsoft compatibility + case tok::kw___cdecl: // struct foo {...} __cdecl x; // C4229 +if (!getLangOpts().MicrosoftExt) + break; +// fall through // Type qualifiers case tok::kw_const: // struct foo {...} const x; case tok::kw_volatile:// struct foo {...} volatile x; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r258504 - Change of UserLabelPrefix default value from "_" to ""
Yes, I understand this -- but the whole idea of my patch is to change the blanket default, on the whole table. Rafael believes this is the right thing to do (actually, he is the one who requested the change), and judging by feedback on PR26255, he is right. Please let me know when you'll add the tests for targets not yet covered, as I'd like to fix them and get my patch back. Yours, Andrey On Wed, Jan 27, 2016 at 4:42 PM, James Y Knight wrote: > Imagine there's a 2d table of values for UserLabelPrefix, each row for CPU > and column for OS. The value of many if those cells was changed by this > commit, because you stopped painting columns as "". > > That is, originally, the default entry was "_", then cpu rows were filled, > and then the os columns were painted on top of that. > > Now, the Sparc row gets painted "_", but the rtems column isn't painted on > top anymore. So, Sparc+rtems gets the wrong value. > > As to test cases, yes, I agree, I will add some more preprocessor tests for > other OS/CPU combinations. > > On Jan 27, 2016 3:12 AM, "Andrey Bokhanko" wrote: >> >> Hi James, >> >> > I reverted this change with r258894, as it breaks (at least) >> > sparc-rtems. >> >> Can you send me a reproducer, please? >> >> Right now I'm at a dead end -- no tests are broken, and no buildbots >> are reported any fails. Yet you claim that my patch breaks one target >> -- and there is zero info how to verify this. >> >> > Clearly this area of the code was not sufficiently covered by the >> > testsuite. >> >> Not sure I understand what you mean -- it's actually covered quite >> extensively in test/Preprocessor/init.c. There is a line for generic >> SPARC as well: >> >> // SPARC:#define __USER_LABEL_PREFIX__ _ >> >> and I added initialization of UserLabelPrefix to "_" in >> SparcTargetInfo constructor specifically to accommodate this test. >> >> Perhaps you might want to add tests for sparc-rterms target as well? >> >> Yours, >> Andrey >> >> >> On Wed, Jan 27, 2016 at 4:10 AM, James Y Knight >> wrote: >> > I reverted this change with r258894, as it breaks (at least) >> > sparc-rtems. >> > Clearly this area of the code was not sufficiently covered by the >> > testsuite. >> > >> > On Fri, Jan 22, 2016 at 10:24 AM, Andrey Bokhanko via cfe-commits >> > wrote: >> >> >> >> Author: asbokhan >> >> Date: Fri Jan 22 09:24:34 2016 >> >> New Revision: 258504 >> >> >> >> URL: http://llvm.org/viewvc/llvm-project?rev=258504&view=rev >> >> Log: >> >> Change of UserLabelPrefix default value from "_" to "" >> >> >> >> Differential Revision: http://reviews.llvm.org/D16295 >> >> >> >> Modified: >> >> cfe/trunk/lib/Basic/TargetInfo.cpp >> >> cfe/trunk/lib/Basic/Targets.cpp >> >> >> >> Modified: cfe/trunk/lib/Basic/TargetInfo.cpp >> >> URL: >> >> >> >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/TargetInfo.cpp?rev=258504&r1=258503&r2=258504&view=diff >> >> >> >> >> >> == >> >> --- cfe/trunk/lib/Basic/TargetInfo.cpp (original) >> >> +++ cfe/trunk/lib/Basic/TargetInfo.cpp Fri Jan 22 09:24:34 2016 >> >> @@ -72,7 +72,7 @@ TargetInfo::TargetInfo(const llvm::Tripl >> >>DoubleFormat = &llvm::APFloat::IEEEdouble; >> >>LongDoubleFormat = &llvm::APFloat::IEEEdouble; >> >>DataLayoutString = nullptr; >> >> - UserLabelPrefix = "_"; >> >> + UserLabelPrefix = ""; >> >>MCountName = "mcount"; >> >>RegParmMax = 0; >> >>SSERegParmMax = 0; >> >> >> >> Modified: cfe/trunk/lib/Basic/Targets.cpp >> >> URL: >> >> >> >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets.cpp?rev=258504&r1=258503&r2=258504&view=diff >> >> >> >> >> >> == >> >> --- cfe/trunk/lib/Basic/Targets.cpp (original) >> >> +++ cfe/trunk/lib/Basic/Targets.cpp Fri Jan 22 09:24:34 2016 >> >> @@ -102,9 +102,7 @@ protected: >> >> >> >> public: >> >>CloudABITargetInfo(const llvm::Triple &Triple) >> >> - : OSTargetInfo(Triple) { >> >> -this->UserLabelPrefix = ""; >> >> - } >> >> + : OSTargetInfo(Triple) {} >> >> }; >> >> >> >> static void getDarwinDefines(MacroBuilder &Builder, const LangOptions >> >> &Opts, >> >> @@ -242,6 +240,7 @@ public: >> >>this->TLSSupported = !Triple.isOSVersionLT(2); >> >> >> >> this->MCountName = "\01mcount"; >> >> +this->UserLabelPrefix = "_"; >> >>} >> >> >> >>std::string isValidSectionSpecifier(StringRef SR) const override { >> >> @@ -284,8 +283,6 @@ protected: >> >> public: >> >>DragonFlyBSDTargetInfo(const llvm::Triple &Triple) >> >>: OSTargetInfo(Triple) { >> >> -this->UserLabelPrefix = ""; >> >> - >> >> switch (Triple.getArch()) { >> >> default: >> >> case llvm::Triple::x86: >> >> @@ -327,8 +324,6 @@ protected: >> >>} >> >> public: >> >>FreeBSDTargetInfo(const llvm::Triple &Triple) : >> >> OSTargetInfo(Triple) { >> >> -this->UserLabelPrefix = ""; >> >> - >> >>
Re: [PATCH] D15920: [CMake] Add option to switch default C++ stdlib
Hahnfeld added a reviewer: mcrosier. Hahnfeld added a comment. Any comments on this change? http://reviews.llvm.org/D15920 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16219: PR8901: attribute "mode" rejected for enums and dependent types
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Aside from one nitpick about the diagnostic wording, this LGTM. Comment at: include/clang/Basic/Attr.td:882 @@ -881,3 +881,3 @@ let Spellings = [GCC<"mode">]; - let Subjects = SubjectList<[Var, TypedefName, Field], ErrorDiag, - "ExpectedVariableFieldOrTypedef">; + let Subjects = SubjectList<[Var, Enum, TypedefName, Field], ErrorDiag, + "ExpectedVariableEnumFieldOrTypedef">; One of these days I may have to come up with a more general solution for this sort of subject list. :-( Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2856 @@ -2855,1 +2855,3 @@ +def err_enum_mode_vector_type : Error< + "mode %0 is not supported for enumeral types">; def warn_attribute_nonnull_no_pointers : Warning< s/enumeral/enumeration http://reviews.llvm.org/D16219 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libclc] r258932 - Add _CLC_V_V_VP_VECTORIZE macro
Author: tstellar Date: Wed Jan 27 08:52:07 2016 New Revision: 258932 URL: http://llvm.org/viewvc/llvm-project?rev=258932&view=rev Log: Add _CLC_V_V_VP_VECTORIZE macro Patch by: Pavel Ondračka Modified: libclc/trunk/generic/lib/clcmacro.h Modified: libclc/trunk/generic/lib/clcmacro.h URL: http://llvm.org/viewvc/llvm-project/libclc/trunk/generic/lib/clcmacro.h?rev=258932&r1=258931&r2=258932&view=diff == --- libclc/trunk/generic/lib/clcmacro.h (original) +++ libclc/trunk/generic/lib/clcmacro.h Wed Jan 27 08:52:07 2016 @@ -109,6 +109,28 @@ } \ \ +#define _CLC_V_V_VP_VECTORIZE(DECLSPEC, RET_TYPE, FUNCTION, ARG1_TYPE, ARG2_TYPE) \ + DECLSPEC RET_TYPE##2 FUNCTION(ARG1_TYPE##2 x, ARG2_TYPE##2 *y) { \ +return (RET_TYPE##2)(FUNCTION(x.x, (ARG2_TYPE*)y), FUNCTION(x.y, (ARG2_TYPE*)y+1)); \ + } \ +\ + DECLSPEC RET_TYPE##3 FUNCTION(ARG1_TYPE##3 x, ARG2_TYPE##3 *y) { \ +return (RET_TYPE##3)(FUNCTION(x.x, (ARG2_TYPE*)y), FUNCTION(x.y, (ARG2_TYPE*)y+1), \ + FUNCTION(x.z, (ARG2_TYPE*)y+2)); \ + } \ +\ + DECLSPEC RET_TYPE##4 FUNCTION(ARG1_TYPE##4 x, ARG2_TYPE##4 *y) { \ +return (RET_TYPE##4)(FUNCTION(x.lo, (ARG2_TYPE##2*)y), FUNCTION(x.hi, (ARG2_TYPE##2*)((ARG2_TYPE*)y+2))); \ + } \ +\ + DECLSPEC RET_TYPE##8 FUNCTION(ARG1_TYPE##8 x, ARG2_TYPE##8 *y) { \ +return (RET_TYPE##8)(FUNCTION(x.lo, (ARG2_TYPE##4*)y), FUNCTION(x.hi, (ARG2_TYPE##4*)((ARG2_TYPE*)y+4))); \ + } \ +\ + DECLSPEC RET_TYPE##16 FUNCTION(ARG1_TYPE##16 x, ARG2_TYPE##16 *y) { \ +return (RET_TYPE##16)(FUNCTION(x.lo, (ARG2_TYPE##8*)y), FUNCTION(x.hi, (ARG2_TYPE##8*)((ARG2_TYPE*)y+8))); \ + } + #define _CLC_DEFINE_BINARY_BUILTIN(RET_TYPE, FUNCTION, BUILTIN, ARG1_TYPE, ARG2_TYPE) \ _CLC_DEF _CLC_OVERLOAD RET_TYPE FUNCTION(ARG1_TYPE x, ARG2_TYPE y) { \ return BUILTIN(x, y); \ ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libclc] r258933 - Implement modf math builtin
Author: tstellar Date: Wed Jan 27 08:52:10 2016 New Revision: 258933 URL: http://llvm.org/viewvc/llvm-project?rev=258933&view=rev Log: Implement modf math builtin V2: use the reference implementation as suggested by Matt Arsenault Patch By: Pavel Ondračka Added: libclc/trunk/generic/include/clc/math/modf.h libclc/trunk/generic/include/clc/math/modf.inc libclc/trunk/generic/lib/math/modf.cl libclc/trunk/generic/lib/math/modf.inc Modified: libclc/trunk/generic/include/clc/clc.h libclc/trunk/generic/lib/SOURCES Modified: libclc/trunk/generic/include/clc/clc.h URL: http://llvm.org/viewvc/llvm-project/libclc/trunk/generic/include/clc/clc.h?rev=258933&r1=258932&r2=258933&view=diff == --- libclc/trunk/generic/include/clc/clc.h (original) +++ libclc/trunk/generic/include/clc/clc.h Wed Jan 27 08:52:10 2016 @@ -67,6 +67,7 @@ #include #include #include +#include #include #include #include Added: libclc/trunk/generic/include/clc/math/modf.h URL: http://llvm.org/viewvc/llvm-project/libclc/trunk/generic/include/clc/math/modf.h?rev=258933&view=auto == --- libclc/trunk/generic/include/clc/math/modf.h (added) +++ libclc/trunk/generic/include/clc/math/modf.h Wed Jan 27 08:52:10 2016 @@ -0,0 +1,24 @@ +/* + * Copyright (c) 2014 Advanced Micro Devices, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +#define __CLC_BODY +#include Added: libclc/trunk/generic/include/clc/math/modf.inc URL: http://llvm.org/viewvc/llvm-project/libclc/trunk/generic/include/clc/math/modf.inc?rev=258933&view=auto == --- libclc/trunk/generic/include/clc/math/modf.inc (added) +++ libclc/trunk/generic/include/clc/math/modf.inc Wed Jan 27 08:52:10 2016 @@ -0,0 +1,25 @@ +/* + * Copyright (c) 2014, 2015 Advanced Micro Devices, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +_CLC_OVERLOAD _CLC_DECL __CLC_GENTYPE modf(__CLC_GENTYPE x, global __CLC_GENTYPE *iptr); +_CLC_OVERLOAD _CLC_DECL __CLC_GENTYPE modf(__CLC_GENTYPE x, local __CLC_GENTYPE *iptr); +_CLC_OVERLOAD _CLC_DECL __CLC_GENTYPE modf(__CLC_GENTYPE x, private __CLC_GENTYPE *iptr); Modified: libclc/trunk/generic/lib/SOURCES URL: http://llvm.org/viewvc/llvm-project/libclc/trunk/generic/lib/SOURCES?rev=258933&r1=258932&r2=258933&view=diff == --- libclc/trunk/generic/lib/SOURCES (original) +++ libclc/trunk/generic/lib/SOURCES Wed Jan 27 08:52:10 2016 @@ -96,6 +96,7 @@ math/log10.cl math/log1p.cl math/log2.cl math/mad.cl +math/modf.cl math/native_log.cl math/native_log2.cl math/tables.cl Added: libclc/trunk/generic/lib/math/modf.cl URL: http://llvm.org/viewvc/llvm-proj
Re: [PATCH] D16529: [clang-tidy] Add modernize-raw-string-literal check
alexfh added inline comments. Comment at: clang-tidy/modernize/RawStringLiteralCheck.cpp:27 @@ +26,3 @@ + // we only transform ASCII string literals + if (!Literal->isAscii()) +return false; Why can't the check convert non-ascii string literals to corresponding raw string literals? Comment at: clang-tidy/modernize/RawStringLiteralCheck.cpp:57 @@ +56,3 @@ + + // already a raw string literal if R comes before " + if (Text.find_first_of("R") < Text.find_first_of(R"(")")) nit: Capitalization, punctuation. Same for other comments in the file. Comment at: clang-tidy/modernize/RawStringLiteralCheck.cpp:58 @@ +57,3 @@ + // already a raw string literal if R comes before " + if (Text.find_first_of("R") < Text.find_first_of(R"(")")) +return false; aaron.ballman wrote: > I think it might make more sense to block on D16012 (which will hopefully be > reviewed relatively soon), and then just use the StringLiteral object to > specify whether it's a raw string literal or not. Also, why `find_first_of` instead of just `find(char)`? It will allow you to get rid of the `R"(")"` awfulness ;) Comment at: clang-tidy/modernize/RawStringLiteralCheck.cpp:61 @@ +60,3 @@ + + const bool HasBackSlash = Text.find(R"(\\)") != StringRef::npos; + const bool HasNewLine = Text.find(R"(\n)") != StringRef::npos; I'd remove these variables and either just combine all these to a single condition (which would make the code benefit from short-circuiting) or just use a regular expression, which can be more effective than N lookups in a row (not sure for which N though). Another benefit of the regular expression is reduction of the number of `R"(`s in the code: `R"(\\[n'"?\\])"` ;). Comment at: clang-tidy/modernize/RawStringLiteralCheck.cpp:77 @@ +76,3 @@ + std::string Delimiter; + for (int Counter = 0; containsDelimiter(Bytes, Delimiter); ++Counter) { +if (Counter == 0) { Why don't you try an empty delimiter? It will cover a majority of cases. Also, even though the function is used only when generating fix-it hints, it still should be more effective than it is now. Most of the heap allocations and copies caused by the usage of std::ostringstream, std::string and std::string::operator+ can be avoided, e.g. like this: static const StringRef Delimiters[2][] = {{"", "R\"(", ")\""}, {"lit", "R\"lit(", ")lit\""}, {"str", "R\"str(", ")str\""}, /* add more different ones to make it unlikely to meet all of these in a single literal in the wild */}; for (const auto &Delim : Delimiters) { if (Bytes.find(Delim[2]) != StringRef::npos) return (Delim[1] + Bytes + Delim[2]).str(); } // Give up gracefully on literals having all our delimiters. Comment at: clang-tidy/modernize/RawStringLiteralCheck.cpp:107 @@ +106,3 @@ + if (const auto *Literal = Result.Nodes.getNodeAs("lit")) +preferRawStringLiterals(Result, Literal); +} I don't see a reason to have a separate function, I'd just inline it here. Comment at: clang-tidy/modernize/RawStringLiteralCheck.cpp:113 @@ +112,3 @@ + if (containsEscapedCharacters(Result, Literal)) { +SourceRange ReplacementRange = Literal->getSourceRange(); +CharSourceRange CharRange = Lexer::makeFileCharRange( I'd just use the expression instead of the variable. Comment at: clang-tidy/modernize/RawStringLiteralCheck.cpp:117 @@ +116,3 @@ +Result.Context->getLangOpts()); +StringRef Replacement = asRawStringLiteral(Literal); +diag(Literal->getLocStart(), `Replacement` will point to deleted memory right after this line. http://reviews.llvm.org/D16529 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D16630: PR23057: Fix assertion `Val && "isa<> used on a null pointer"' on invalid for-range expression
d.zobnin.bugzilla created this revision. d.zobnin.bugzilla added reviewers: kcc, rsmith. d.zobnin.bugzilla added a subscriber: cfe-commits. Fix the issue discovered by fuzzing (https://llvm.org/bugs/show_bug.cgi?id=23057#c18) by handling nullptr in Sema::ActOnCXXForRangeDecl and correct delayed typos in for-range expression before calling Sema::ActOnCXXForRangeStmt (fixes PR26288 as well). http://reviews.llvm.org/D16630 Files: lib/Parse/ParseStmt.cpp lib/Sema/SemaDecl.cpp test/CXX/dcl.dcl/dcl.spec/dcl.type/p3-0x.cpp test/Parser/cxx-invalid-for-range.cpp Index: lib/Parse/ParseStmt.cpp === --- lib/Parse/ParseStmt.cpp +++ lib/Parse/ParseStmt.cpp @@ -1716,9 +1716,11 @@ StmtResult ForEachStmt; if (ForRange) { +ExprResult CorrectedRange = +Actions.CorrectDelayedTyposInExpr(ForRangeInit.RangeExpr.get()); ForRangeStmt = Actions.ActOnCXXForRangeStmt( getCurScope(), ForLoc, CoawaitLoc, FirstPart.get(), -ForRangeInit.ColonLoc, ForRangeInit.RangeExpr.get(), +ForRangeInit.ColonLoc, CorrectedRange.get(), T.getCloseLocation(), Sema::BFRK_Build); // Similarly, we need to do the semantic analysis for a for-range Index: lib/Sema/SemaDecl.cpp === --- lib/Sema/SemaDecl.cpp +++ lib/Sema/SemaDecl.cpp @@ -9901,6 +9901,10 @@ } void Sema::ActOnCXXForRangeDecl(Decl *D) { + // If there is no declaration, there was an error parsing it. Ignore it. + if (!D) +return; + VarDecl *VD = dyn_cast(D); if (!VD) { Diag(D->getLocation(), diag::err_for_range_decl_must_be_var); Index: test/CXX/dcl.dcl/dcl.spec/dcl.type/p3-0x.cpp === --- test/CXX/dcl.dcl/dcl.spec/dcl.type/p3-0x.cpp +++ test/CXX/dcl.dcl/dcl.spec/dcl.type/p3-0x.cpp @@ -18,6 +18,9 @@ for (struct S { S(int) {} } s : arr) { // expected-error {{types may not be defined in a for range declaration}} } + for (struct S { S(int) {} } s : Undeclared); // expected-error{{types may not be defined in a for range declaration}} + // expected-error@-1{{use of undeclared identifier 'Undeclared'}} + new struct T {}; // expected-error {{'T' cannot be defined in a type specifier}} new struct A {}; // expected-error {{'A' cannot be defined in a type specifier}} Index: test/Parser/cxx-invalid-for-range.cpp === --- test/Parser/cxx-invalid-for-range.cpp +++ test/Parser/cxx-invalid-for-range.cpp @@ -0,0 +1,18 @@ +// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s + +// From PR23057 comment #18 (https://llvm.org/bugs/show_bug.cgi?id=23057#c18). + +namespace N { + int X[10]; // expected-note{{declared here +} + +void f1() { + for (auto operator new : X); // expected-error{{'operator new' cannot be the name of a variable or data member}} + // expected-error@-1{{use of undeclared identifier 'X'; did you mean 'N::X'?}} +} + +void f2() { + for (a operator== :) // expected-error{{'operator==' cannot be the name of a variable or data member}} + // expected-error@-1{{expected expression}} + // expected-error@-2{{unknown type name 'a'}} +} // expected-error{{expected statement}} Index: lib/Parse/ParseStmt.cpp === --- lib/Parse/ParseStmt.cpp +++ lib/Parse/ParseStmt.cpp @@ -1716,9 +1716,11 @@ StmtResult ForEachStmt; if (ForRange) { +ExprResult CorrectedRange = +Actions.CorrectDelayedTyposInExpr(ForRangeInit.RangeExpr.get()); ForRangeStmt = Actions.ActOnCXXForRangeStmt( getCurScope(), ForLoc, CoawaitLoc, FirstPart.get(), -ForRangeInit.ColonLoc, ForRangeInit.RangeExpr.get(), +ForRangeInit.ColonLoc, CorrectedRange.get(), T.getCloseLocation(), Sema::BFRK_Build); // Similarly, we need to do the semantic analysis for a for-range Index: lib/Sema/SemaDecl.cpp === --- lib/Sema/SemaDecl.cpp +++ lib/Sema/SemaDecl.cpp @@ -9901,6 +9901,10 @@ } void Sema::ActOnCXXForRangeDecl(Decl *D) { + // If there is no declaration, there was an error parsing it. Ignore it. + if (!D) +return; + VarDecl *VD = dyn_cast(D); if (!VD) { Diag(D->getLocation(), diag::err_for_range_decl_must_be_var); Index: test/CXX/dcl.dcl/dcl.spec/dcl.type/p3-0x.cpp === --- test/CXX/dcl.dcl/dcl.spec/dcl.type/p3-0x.cpp +++ test/CXX/dcl.dcl/dcl.spec/dcl.type/p3-0x.cpp @@ -18,6 +18,9 @@ for (struct S { S(int) {} } s : arr) { // expected-error {{types may not be defined in a for range declaration}} } + for (struct S { S(int) {} } s : Undeclared); // expected-error{{types may not be define
Re: [PATCH] D16607: Implementation of PS4 ABI, round 1
probinson added a comment. Subscribed the reviewers of http://reviews.llvm.org/D14980, which had fixed the alignment-attribute layout bug. http://reviews.llvm.org/D16607 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D16632: clang-cl: Take dllexport from original function decl into account
sberg created this revision. sberg added a reviewer: rnk. sberg added a subscriber: cfe-commits. ...in cases where a member function is redeclared as a friend of a nested class. (LibreOffice happens to get tripped up by this.) http://reviews.llvm.org/D16632 Files: lib/CodeGen/CodeGenModule.cpp test/CodeGenCXX/dllexport.cpp Index: test/CodeGenCXX/dllexport.cpp === --- test/CodeGenCXX/dllexport.cpp +++ test/CodeGenCXX/dllexport.cpp @@ -264,6 +264,16 @@ __declspec(dllexport) void friend1() {} void friend2() {} +// MSC-DAG: define dllexport void @"\01?func@Befriended@@SAXXZ"() +// GNU-DAG: define dllexport void @_ZN10Befriended4funcEv() +struct __declspec(dllexport) Befriended { + static void func(); + struct Befriending { +friend void Befriended::func(); + }; +}; +void Befriended::func() {} + // Implicit declarations can be redeclared with dllexport. // MSC-DAG: define dllexport noalias i8* @"\01??2@{{YAPAXI|YAPEAX_K}}@Z"( // GNU-DAG: define dllexport noalias i8* @_Znw{{[yj]}}( Index: lib/CodeGen/CodeGenModule.cpp === --- lib/CodeGen/CodeGenModule.cpp +++ lib/CodeGen/CodeGenModule.cpp @@ -765,7 +765,8 @@ if (FD->hasAttr()) F->setDLLStorageClass(llvm::GlobalVariable::DLLImportStorageClass); - else if (FD->hasAttr()) + else if (FD->hasAttr() || + FD->getCanonicalDecl()->hasAttr()) F->setDLLStorageClass(llvm::GlobalVariable::DLLExportStorageClass); else F->setDLLStorageClass(llvm::GlobalVariable::DefaultStorageClass); Index: test/CodeGenCXX/dllexport.cpp === --- test/CodeGenCXX/dllexport.cpp +++ test/CodeGenCXX/dllexport.cpp @@ -264,6 +264,16 @@ __declspec(dllexport) void friend1() {} void friend2() {} +// MSC-DAG: define dllexport void @"\01?func@Befriended@@SAXXZ"() +// GNU-DAG: define dllexport void @_ZN10Befriended4funcEv() +struct __declspec(dllexport) Befriended { + static void func(); + struct Befriending { +friend void Befriended::func(); + }; +}; +void Befriended::func() {} + // Implicit declarations can be redeclared with dllexport. // MSC-DAG: define dllexport noalias i8* @"\01??2@{{YAPAXI|YAPEAX_K}}@Z"( // GNU-DAG: define dllexport noalias i8* @_Znw{{[yj]}}( Index: lib/CodeGen/CodeGenModule.cpp === --- lib/CodeGen/CodeGenModule.cpp +++ lib/CodeGen/CodeGenModule.cpp @@ -765,7 +765,8 @@ if (FD->hasAttr()) F->setDLLStorageClass(llvm::GlobalVariable::DLLImportStorageClass); - else if (FD->hasAttr()) + else if (FD->hasAttr() || + FD->getCanonicalDecl()->hasAttr()) F->setDLLStorageClass(llvm::GlobalVariable::DLLExportStorageClass); else F->setDLLStorageClass(llvm::GlobalVariable::DefaultStorageClass); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r258935 - Adding back in a test that I inadvertently removed in r258862
Author: cbieneman Date: Wed Jan 27 09:51:56 2016 New Revision: 258935 URL: http://llvm.org/viewvc/llvm-project?rev=258935&view=rev Log: Adding back in a test that I inadvertently removed in r258862 Added: cfe/trunk/test/SemaObjC/atomoic-property-synnthesis-rules.m Added: cfe/trunk/test/SemaObjC/atomoic-property-synnthesis-rules.m URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/atomoic-property-synnthesis-rules.m?rev=258935&view=auto == --- cfe/trunk/test/SemaObjC/atomoic-property-synnthesis-rules.m (added) +++ cfe/trunk/test/SemaObjC/atomoic-property-synnthesis-rules.m Wed Jan 27 09:51:56 2016 @@ -0,0 +1,377 @@ +// RUN: %clang_cc1 -fsyntax-only -verify -Wno-objc-root-class %s + +/* + Conditions for warning: + 1. the property is atomic + 2. the current @implementation contains an @synthesize for the property + 3. the current @implementation contains a hand-written setter XOR getter + 4. the property is read-write + + Cases marked WARN should warn one the following: + warning: Atomic property 'x' has a synthesized setter and a + manually-implemented getter, which may break atomicity. + warning: Atomic property 'x' has a synthesized getter and a + manually-implemented setter, which may break atomicity. + + Cases not marked WARN only satisfy the indicated subset + of the conditions required to warn. + + There should be 8 warnings. +*/ + +@interface Foo +{ +/* 12 4 */int GetSet; +/* WARN */int Get; +/* WARN */int Set; +/* 12 4 */int None; +/* 2 4 */int GetSet_Nonatomic; +/* 234 */int Get_Nonatomic; +/* 234 */int Set_Nonatomic; +/* 2 4 */int None_Nonatomic; + +/* 12 */int GetSet_ReadOnly; +/* 123 */int Get_ReadOnly; +/* 123 */int Set_ReadOnly; +/* 12 */int None_ReadOnly; +/* 2 */int GetSet_Nonatomic_ReadOnly; +/* 23 */int Get_Nonatomic_ReadOnly; +/* 23 */int Set_Nonatomic_ReadOnly; +/* 2 */int None_Nonatomic_ReadOnly; + +/* 12 4 */int GetSet_ReadWriteInExt; +/* WARN */int Get_ReadWriteInExt; +/* WARN */int Set_ReadWriteInExt; +/* 12 4 */int None_ReadWriteInExt; +/* 2 4 */int GetSet_Nonatomic_ReadWriteInExt; +/* 234 */int Get_Nonatomic_ReadWriteInExt; +/* 234 */int Set_Nonatomic_ReadWriteInExt; +/* 2 4 */int None_Nonatomic_ReadWriteInExt; + + +/* 12 4 */int GetSet_LateSynthesize; +/* WARN */int Get_LateSynthesize; +/* WARN */int Set_LateSynthesize; +/* 12 4 */int None_LateSynthesize; +/* 2 4 */int GetSet_Nonatomic_LateSynthesize; +/* 234 */int Get_Nonatomic_LateSynthesize; +/* 234 */int Set_Nonatomic_LateSynthesize; +/* 2 4 */int None_Nonatomic_LateSynthesize; + +/* 12 */int GetSet_ReadOnly_LateSynthesize; +/* 123 */int Get_ReadOnly_LateSynthesize; +/* 123 */int Set_ReadOnly_LateSynthesize; +/* 12 */int None_ReadOnly_LateSynthesize; +/* 2 */int GetSet_Nonatomic_ReadOnly_LateSynthesize; +/* 23 */int Get_Nonatomic_ReadOnly_LateSynthesize; +/* 23 */int Set_Nonatomic_ReadOnly_LateSynthesize; +/* 2 */int None_Nonatomic_ReadOnly_LateSynthesize; + +/* 12 4 */int GetSet_ReadWriteInExt_LateSynthesize; +/* WARN */int Get_ReadWriteInExt_LateSynthesize; +/* WARN */int Set_ReadWriteInExt_LateSynthesize; +/* 12 4 */int None_ReadWriteInExt_LateSynthesize; +/* 2 4 */int GetSet_Nonatomic_ReadWriteInExt_LateSynthesize; +/* 234 */int Get_Nonatomic_ReadWriteInExt_LateSynthesize; +/* 234 */int Set_Nonatomic_ReadWriteInExt_LateSynthesize; +/* 2 4 */int None_Nonatomic_ReadWriteInExt_LateSynthesize; + + +/* 1 4 */int GetSet_NoSynthesize; +/* 1 34 */int Get_NoSynthesize; +/* 1 34 */int Set_NoSynthesize; +/* 1 4 */int None_NoSynthesize; +/*4 */int GetSet_Nonatomic_NoSynthesize; +/* 34 */int Get_Nonatomic_NoSynthesize; +/* 34 */int Set_Nonatomic_NoSynthesize; +/*4 */int None_Nonatomic_NoSynthesize; + +/* 1*/int GetSet_ReadOnly_NoSynthesize; +/* 1 3 */int Get_ReadOnly_NoSynthesize; +/* 1 3 */int Set_ReadOnly_NoSynthesize; +/* 1*/int None_ReadOnly_NoSynthesize; +/* */int GetSet_Nonatomic_ReadOnly_NoSynthesize; +/* 3 */int Get_Nonatomic_ReadOnly_NoSynthesize; +/* 3 */int Set_Nonatomic_ReadOnly_NoSynthesize; +/* */int None_Nonatomic_ReadOnly_NoSynthesize; + +/* 1 4 */int GetSet_ReadWriteInExt_NoSynthesize; +/* 1 34 */int Get_ReadWriteInExt_NoSynthesize; +/* 1 34 */int Set_ReadWriteInExt_NoSynthesize; +/* 1 4 */int None_ReadWriteInExt_NoSynthesize; +/*4 */int GetSet_Nonatomic_ReadWriteInExt
Re: [PATCH] D16632: clang-cl: Take dllexport from original function decl into account
rnk added a subscriber: rnk. rnk added a comment. Hans knows all about dllexport now. http://reviews.llvm.org/D16632 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15267: For MS ABI, emit dllexport friend functions defined inline in class
rnk added inline comments. Comment at: include/clang/AST/ASTConsumer.h:58-64 @@ -57,5 +57,9 @@ /// \brief This callback is invoked each time an inline method definition is /// completed. virtual void HandleInlineMethodDefinition(CXXMethodDecl *D) {} + /// \brief This callback is invoked each time an inline friend function + /// definition is completed. + virtual void HandleInlineFriendFunctionDefinition(FunctionDecl *D) {} + I'm pretty sure we can relax HandleInlineMethodDefinition to take a FunctionDecl and then we don't need the extra AST consumer callback. Comment at: lib/Parse/ParseCXXInlineMethods.cpp:568-569 @@ -567,2 +567,4 @@ Actions.ActOnFinishInlineMethodDef(MD); + else if (auto *FD = dyn_cast_or_null(LM.D)) +Actions.ActOnFinishInlineFriendFunctionDef(FD); } I'd check for the friend specification here rather than asserting later. There probably are or will eventually be ways to sneak a non-friend, non-method FunctionDecl into a class context. http://reviews.llvm.org/D15267 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14653: [libcxx] Introduce the mechanism for fixing -fno-exceptions test failures.
mclow.lists added inline comments. Comment at: include/__config:847 @@ +846,3 @@ +} +#define _LIBCPP_THROW(E, MSG) __libcxx_noexceptions_report(MSG) +#else // !_LIBCPP_NO_EXCEPTIONS I don't care for having to specify something twice. (E, MSG). Maybe "stringify" E and make that the message. Comment at: test/support/noexcept.h:23 @@ +22,3 @@ +// tests use multiple catch statements, in those cases we have to use the +// _LIBCPP_NO_EXCEPTIONS macro and exclude the additional catch statements. +#ifndef _LIBCPP_NO_EXCEPTIONS I don't care for this; I think that "implementing a mechanism for throwing exceptions in the test suite for when we've disabled exceptions" seems like something that we'll have to revisit time and time again. I wonder if it would be better to just split some tests into multiple tests (some parts that test exception handling, some that don't), and then XFAIL: no-exceptions the ones that test exception handling. http://reviews.llvm.org/D14653 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15267: For MS ABI, emit dllexport friend functions defined inline in class
sberg added inline comments. Comment at: include/clang/AST/ASTConsumer.h:58-64 @@ -57,5 +57,9 @@ /// \brief This callback is invoked each time an inline method definition is /// completed. virtual void HandleInlineMethodDefinition(CXXMethodDecl *D) {} + /// \brief This callback is invoked each time an inline friend function + /// definition is completed. + virtual void HandleInlineFriendFunctionDefinition(FunctionDecl *D) {} + rnk wrote: > I'm pretty sure we can relax HandleInlineMethodDefinition to take a > FunctionDecl and then we don't need the extra AST consumer callback. ...and then also rename it from HandleInlineMethodDefinition to, say,HandleInlineFunctionDefinition? Or better stick with the (then no longer accurate) name? http://reviews.llvm.org/D15267 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16628: clang-cl: support __cdecl-on-struct anachronism
rnk added inline comments. Comment at: lib/Parse/ParseDeclCXX.cpp:1108-1110 @@ -1106,1 +1107,5 @@ + case tok::kw___cdecl: // struct foo {...} __cdecl x; // C4229 +if (!getLangOpts().MicrosoftExt) + break; +// fall through // Type qualifiers I think this would be clearer as: // We will diagnose __cdecl on non-function declarations later, so claim that is valid // after a type specifier. return getLangOpts().MicrosoftExt; Unpacking what the fallthrough does is a little tricky. http://reviews.llvm.org/D16628 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16628: clang-cl: support __cdecl-on-struct anachronism
sberg added inline comments. Comment at: lib/Parse/ParseDeclCXX.cpp:1107 @@ +1106,3 @@ + // Microsoft compatibility + case tok::kw___cdecl: // struct foo {...} __cdecl x; // C4229 +if (!getLangOpts().MicrosoftExt) majnemer wrote: > What about __fastcall, etc. To be honest, I have no idea what the full set of keywords is that would need to be taken care of here (so hoped I would get away with just the one I happened to come across ;) http://reviews.llvm.org/D16628 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16628: clang-cl: support __cdecl-on-struct anachronism
majnemer added a subscriber: majnemer. Comment at: lib/Parse/ParseDeclCXX.cpp:1107 @@ +1106,3 @@ + // Microsoft compatibility + case tok::kw___cdecl: // struct foo {...} __cdecl x; // C4229 +if (!getLangOpts().MicrosoftExt) What about __fastcall, etc. http://reviews.llvm.org/D16628 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16591: Add backend dignostic printer for unsupported features
ast accepted this revision. ast added a comment. This revision is now accepted and ready to land. thanks http://reviews.llvm.org/D16591 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15120: Add support for __float128 type to be used by targets that support it
rjmccall added a comment. I think it's not unlikely that float128_t will see future standardization (as an optional extension, of course), and it would be strange for an operation between two types to not consistently yield the type of higher rank. I could see an argument that we should not treat float128_t as a distinct type from long double on targets where they have the same implementation. That might avoid the need for special-case manglings. Repository: rL LLVM http://reviews.llvm.org/D15120 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15267: For MS ABI, emit dllexport friend functions defined inline in class
rnk added inline comments. Comment at: include/clang/AST/ASTConsumer.h:58-64 @@ -57,5 +57,9 @@ /// \brief This callback is invoked each time an inline method definition is /// completed. virtual void HandleInlineMethodDefinition(CXXMethodDecl *D) {} + /// \brief This callback is invoked each time an inline friend function + /// definition is completed. + virtual void HandleInlineFriendFunctionDefinition(FunctionDecl *D) {} + sberg wrote: > rnk wrote: > > I'm pretty sure we can relax HandleInlineMethodDefinition to take a > > FunctionDecl and then we don't need the extra AST consumer callback. > ...and then also rename it from HandleInlineMethodDefinition to, > say,HandleInlineFunctionDefinition? Or better stick with the (then no longer > accurate) name? Renaming would be good if you're up for it. I have a feeling that nobody outside of Clang is overriding this ASTConsumer callback. It's purpose is very specific to dllexport. http://reviews.llvm.org/D15267 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16607: Implementation of PS4 ABI, round 1
rjmccall added a comment. Seems reasonable. http://reviews.llvm.org/D16607 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16628: clang-cl: support __cdecl-on-struct anachronism
majnemer added inline comments. Comment at: lib/Parse/ParseDeclCXX.cpp:1107 @@ +1106,3 @@ + // Microsoft compatibility + case tok::kw___cdecl: // struct foo {...} __cdecl x; // C4229 +if (!getLangOpts().MicrosoftExt) sberg wrote: > majnemer wrote: > > What about __fastcall, etc. > To be honest, I have no idea what the full set of keywords is that would need > to be taken care of here (so hoped I would get away with just the one I > happened to come across ;) The complete list is https://msdn.microsoft.com/en-us/library/984x0h58.aspx#mainBody http://reviews.llvm.org/D16628 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r258950 - Add backend dignostic printer for unsupported features
Author: olista01 Date: Wed Jan 27 11:30:28 2016 New Revision: 258950 URL: http://llvm.org/viewvc/llvm-project?rev=258950&view=rev Log: Add backend dignostic printer for unsupported features The related LLVM patch adds a backend diagnostic type for reporting unsupported features, this adds a printer for them to clang. In the case where debug location information is not available, I've changed the printer to report the location as the first line of the function, rather than the closing brace, as the latter does not give the user any information. This also affects optimisation remarks. Differential Revision: http://reviews.llvm.org/D16591 Added: cfe/trunk/test/CodeGen/backend-unsupported-error.ll Modified: cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td cfe/trunk/lib/CodeGen/CodeGenAction.cpp cfe/trunk/test/Frontend/optimization-remark-analysis.c cfe/trunk/test/Misc/backend-optimization-failure-nodbg.cpp Modified: cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td?rev=258950&r1=258949&r2=258950&view=diff == --- cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td Wed Jan 27 11:30:28 2016 @@ -58,8 +58,10 @@ def remark_fe_backend_optimization_remar BackendInfo, InGroup; def warn_fe_backend_optimization_failure : Warning<"%0">, BackendInfo, InGroup, DefaultWarn; -def note_fe_backend_optimization_remark_invalid_loc : Note<"could " - "not determine the original source location for %0:%1:%2">; +def note_fe_backend_invalid_loc : Note<"could " + "not determine the original source location for %0:%1:%2">, BackendInfo; + +def err_fe_backend_unsupported : Error<"%0">, BackendInfo; def remark_sanitize_address_insert_extra_padding_accepted : Remark< "-fsanitize-address-field-padding applied to %0">, ShowInSystemHeader, Modified: cfe/trunk/lib/CodeGen/CodeGenAction.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenAction.cpp?rev=258950&r1=258949&r2=258950&view=diff == --- cfe/trunk/lib/CodeGen/CodeGenAction.cpp (original) +++ cfe/trunk/lib/CodeGen/CodeGenAction.cpp Wed Jan 27 11:30:28 2016 @@ -242,6 +242,13 @@ namespace clang { ((BackendConsumer *)Context)->DiagnosticHandlerImpl(DI); } +/// Get the best possible source location to represent a diagnostic that +/// may have associated debug info. +const FullSourceLoc +getBestLocationFromDebugLoc(const llvm::DiagnosticInfoWithDebugLocBase &D, +bool &BadDebugInfo, StringRef &Filename, +unsigned &Line, unsigned &Column) const; + void InlineAsmDiagHandler2(const llvm::SMDiagnostic &, SourceLocation LocCookie); @@ -254,6 +261,8 @@ namespace clang { /// \return True if the diagnostic has been successfully reported, false /// otherwise. bool StackSizeDiagHandler(const llvm::DiagnosticInfoStackSize &D); +/// \brief Specialized handler for unsupported backend feature diagnostic. +void UnsupportedDiagHandler(const llvm::DiagnosticInfoUnsupported &D); /// \brief Specialized handlers for optimization remarks. /// Note that these handlers only accept remarks and they always handle /// them. @@ -439,16 +448,11 @@ BackendConsumer::StackSizeDiagHandler(co return false; } -void BackendConsumer::EmitOptimizationMessage( -const llvm::DiagnosticInfoOptimizationBase &D, unsigned DiagID) { - // We only support warnings and remarks. - assert(D.getSeverity() == llvm::DS_Remark || - D.getSeverity() == llvm::DS_Warning); - +const FullSourceLoc BackendConsumer::getBestLocationFromDebugLoc( +const llvm::DiagnosticInfoWithDebugLocBase &D, bool &BadDebugInfo, StringRef &Filename, +unsigned &Line, unsigned &Column) const { SourceManager &SourceMgr = Context->getSourceManager(); FileManager &FileMgr = SourceMgr.getFileManager(); - StringRef Filename; - unsigned Line, Column; SourceLocation DILoc; if (D.isLocationAvailable()) { @@ -459,6 +463,7 @@ void BackendConsumer::EmitOptimizationMe // source manager, so pass 1 if Column is not set. DILoc = SourceMgr.translateFileLineCol(FE, Line, Column ? Column : 1); } +BadDebugInfo = DILoc.isInvalid(); } // If a location isn't available, try to approximate it using the associated @@ -467,18 +472,63 @@ void BackendConsumer::EmitOptimizationMe FullSourceLoc Loc(DILoc, SourceMgr); if (Loc.isInvalid()) if (const Decl *FD = Gen->GetDeclForMangledName(D.getFunction().getName())) - Loc = FD->getASTContext().getFullLoc(FD->getBodyRBrace()); + Loc = FD->getASTContext().getFul
Re: [PATCH] D16591: Add backend dignostic printer for unsupported features
This revision was automatically updated to reflect the committed changes. Closed by commit rL258950: Add backend dignostic printer for unsupported features (authored by olista01). Changed prior to commit: http://reviews.llvm.org/D16591?vs=46127&id=46144#toc Repository: rL LLVM http://reviews.llvm.org/D16591 Files: cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td cfe/trunk/lib/CodeGen/CodeGenAction.cpp cfe/trunk/test/CodeGen/backend-unsupported-error.ll cfe/trunk/test/Frontend/optimization-remark-analysis.c cfe/trunk/test/Misc/backend-optimization-failure-nodbg.cpp Index: cfe/trunk/lib/CodeGen/CodeGenAction.cpp === --- cfe/trunk/lib/CodeGen/CodeGenAction.cpp +++ cfe/trunk/lib/CodeGen/CodeGenAction.cpp @@ -242,6 +242,13 @@ ((BackendConsumer *)Context)->DiagnosticHandlerImpl(DI); } +/// Get the best possible source location to represent a diagnostic that +/// may have associated debug info. +const FullSourceLoc +getBestLocationFromDebugLoc(const llvm::DiagnosticInfoWithDebugLocBase &D, +bool &BadDebugInfo, StringRef &Filename, +unsigned &Line, unsigned &Column) const; + void InlineAsmDiagHandler2(const llvm::SMDiagnostic &, SourceLocation LocCookie); @@ -254,6 +261,8 @@ /// \return True if the diagnostic has been successfully reported, false /// otherwise. bool StackSizeDiagHandler(const llvm::DiagnosticInfoStackSize &D); +/// \brief Specialized handler for unsupported backend feature diagnostic. +void UnsupportedDiagHandler(const llvm::DiagnosticInfoUnsupported &D); /// \brief Specialized handlers for optimization remarks. /// Note that these handlers only accept remarks and they always handle /// them. @@ -439,16 +448,11 @@ return false; } -void BackendConsumer::EmitOptimizationMessage( -const llvm::DiagnosticInfoOptimizationBase &D, unsigned DiagID) { - // We only support warnings and remarks. - assert(D.getSeverity() == llvm::DS_Remark || - D.getSeverity() == llvm::DS_Warning); - +const FullSourceLoc BackendConsumer::getBestLocationFromDebugLoc( +const llvm::DiagnosticInfoWithDebugLocBase &D, bool &BadDebugInfo, StringRef &Filename, +unsigned &Line, unsigned &Column) const { SourceManager &SourceMgr = Context->getSourceManager(); FileManager &FileMgr = SourceMgr.getFileManager(); - StringRef Filename; - unsigned Line, Column; SourceLocation DILoc; if (D.isLocationAvailable()) { @@ -459,26 +463,72 @@ // source manager, so pass 1 if Column is not set. DILoc = SourceMgr.translateFileLineCol(FE, Line, Column ? Column : 1); } +BadDebugInfo = DILoc.isInvalid(); } // If a location isn't available, try to approximate it using the associated // function definition. We use the definition's right brace to differentiate // from diagnostics that genuinely relate to the function itself. FullSourceLoc Loc(DILoc, SourceMgr); if (Loc.isInvalid()) if (const Decl *FD = Gen->GetDeclForMangledName(D.getFunction().getName())) - Loc = FD->getASTContext().getFullLoc(FD->getBodyRBrace()); + Loc = FD->getASTContext().getFullLoc(FD->getLocation()); + + if (DILoc.isInvalid() && D.isLocationAvailable()) +// If we were not able to translate the file:line:col information +// back to a SourceLocation, at least emit a note stating that +// we could not translate this location. This can happen in the +// case of #line directives. +Diags.Report(Loc, diag::note_fe_backend_invalid_loc) +<< Filename << Line; + + return Loc; +} + +void BackendConsumer::UnsupportedDiagHandler( +const llvm::DiagnosticInfoUnsupported &D) { + // We only support errors. + assert(D.getSeverity() == llvm::DS_Error); + + StringRef Filename; + unsigned Line, Column; + bool BadDebugInfo; + FullSourceLoc Loc = getBestLocationFromDebugLoc(D, BadDebugInfo, Filename, + Line, Column); + + Diags.Report(Loc, diag::err_fe_backend_unsupported) << D.getMessage().str(); + + if (BadDebugInfo) +// If we were not able to translate the file:line:col information +// back to a SourceLocation, at least emit a note stating that +// we could not translate this location. This can happen in the +// case of #line directives. +Diags.Report(Loc, diag::note_fe_backend_invalid_loc) +<< Filename << Line << Column; +} + +void BackendConsumer::EmitOptimizationMessage( +const llvm::DiagnosticInfoOptimizationBase &D, unsigned DiagID) { + // We only support warnings and remarks. + assert(D.getSeverity() == llvm::DS_Remark || + D.getSeverity() == llvm::DS_Warning); + + StringRef Filename; + unsigned Line, Column; + bool BadDebugInfo = false; + FullSourceLoc Loc = getBestLocationFromDebugLoc(D, BadDebugInfo, Filename, + Li
Re: [PATCH] D16632: clang-cl: Take dllexport from original function decl into account
hans added a comment. Hi Stephan, I would rather see that we could get this right in the AST. The problem is that the Befriended::func() definition doesn't have dllexport attached: `-CXXMethodDecl 0x5ba1cf0 parent 0x5b4f288 prev 0x5b4f750 col:18 used func 'void (void)' `-CompoundStmt 0x5ba1de0 If I drop the friend declaration, it looks like this: `-CXXMethodDecl 0x6af2a08 parent 0x6aa0288 prev 0x6aa04c0 col:18 used func 'void (void)' |-CompoundStmt 0x6af2b30 `-DLLExportAttr 0x6af2b20 Inherited It's a tricky situation though. I suspect what's happening is that when we build the AST for the friend decl, we haven't yet propagated the dllexport attribute from the 'Befriended' class to 'func'. If the attribute was put directly on 'func', the friend declaration would pick it up: struct Befriended { static void __declspec(dllexport) func(); struct Befriending { friend void Befriended::func(); }; }; void Befriended::func() {} |-CXXRecordDecl 0x6fe4288 line:1:9 struct Befriended definition | |-CXXRecordDecl 0x6fe43a0 col:9 implicit struct Befriended | |-CXXMethodDecl 0x6fe4480 col:37 func 'void (void)' static | | `-DLLExportAttr 0x6fe4528 | `-CXXRecordDecl 0x6fe4560 line:3:10 struct Befriending definition | |-CXXRecordDecl 0x6fe4670 col:10 implicit struct Befriending | `-FriendDecl 0x6fe4868 col:29 | `-CXXMethodDecl 0x6fe4740 parent 0x6fe4288 prev 0x6fe4480 col:29 func 'void (void)' | `-DLLExportAttr 0x6fe4858 Inherited `-CXXMethodDecl 0x6fe48c8 parent 0x6fe4288 prev 0x6fe4740 col:18 func 'void (void)' |-CompoundStmt 0x7036710 `-DLLExportAttr 0x6fe49e0 Inherited We need to figure out the ordering of applying the dllexport class attribute vs. parsing inner classes. http://reviews.llvm.org/D16632 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16586: Make clang AAPCS compliant w.r.t volatile bitfield accesses
rmaprath added inline comments. Comment at: lib/CodeGen/CGExpr.cpp:1761 @@ +1760,3 @@ +Ptr = Address(AdjustAAPCSBitfieldAccess(Dst, Info, false), + getContext().getTypeAlignInChars(Dst.getType())); + rjmccall wrote: > This alignment computation is wrong; you need to be basing this on the > alignment of the base. It would be easier to do that in the formation of the > LValue in the first place in EmitLValueForField, and then you won't need to > modify as many of these uses. I'm wondering if it would be possible to do the all the adjustments (not just setting the bitfield LValue alignment) within EmitLValueForField(). It would certainly simplify the code a lot (e.g. No need to dissect the load/store GEP as I currently do in AdjustAAPCSBitfieldAccess(), I can intercept the original GEP when it is being constructed). One problem I have is, as part of the adjustments, I have to override the CGBitFieldInfo of the bitfield LValue. But LValue holds a pointer to the CGBitFieldInfo and I'd have to allocate a new CGBitFieldInfo instance and pass it into LValue::MakeBitfield() method. Not sure if that is a good idea (who'd free it?). If you have any suggestions, please let me know. Adjusting alignments in EmitLValueForField() and then again adjusting other bitfield access parameters in the load/store methods would be a bit messy, I imagine. Thanks for all the feedback! http://reviews.llvm.org/D16586 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [clang-tools-extra] r258924 - Add clang-tools-extra documentation to the CMake build.
On Wed, Jan 27, 2016 at 3:37 AM, Alexander Kornienko via cfe-commits wrote: > Author: alexfh > Date: Wed Jan 27 05:37:08 2016 > New Revision: 258924 > > URL: http://llvm.org/viewvc/llvm-project?rev=258924&view=rev > Log: > Add clang-tools-extra documentation to the CMake build. Merged to 3.8 in r258955. ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D16638: [CUDA] Added device-side std::{malloc/free}
tra created this revision. tra added a reviewer: jlebar. tra added a subscriber: cfe-commits. In addition to math functions, we also need to support std::malloc and std::free to match NVCC behavior. http://reviews.llvm.org/D16638 Files: lib/Headers/__clang_cuda_cmath.h lib/Headers/__clang_cuda_runtime_wrapper.h Index: lib/Headers/__clang_cuda_runtime_wrapper.h === --- lib/Headers/__clang_cuda_runtime_wrapper.h +++ lib/Headers/__clang_cuda_runtime_wrapper.h @@ -169,6 +169,10 @@ // CUDA headers. Alas, device_functions.hpp included below needs it. static inline __device__ void __brkpt(int c) { __brkpt(); } +// We also need extern "C" decls for device-side allocator functions. +extern "C" __device__ void free(void *__ptr); +extern "C" __device__ void *malloc(size_t __size); + // Now include *.hpp with definitions of various GPU functions. Alas, // a lot of thins get declared/defined with __host__ attribute which // we don't want and we have to define it out. We also have to include Index: lib/Headers/__clang_cuda_cmath.h === --- lib/Headers/__clang_cuda_cmath.h +++ lib/Headers/__clang_cuda_cmath.h @@ -218,6 +218,9 @@ __DEVICE__ float trunc(float x) { return ::truncf(x); } __DEVICE__ double trunc(double x) { return ::trunc(x); } +__DEVICE__ void free(void *__ptr) { return ::free(__ptr); } +__DEVICE__ void *malloc(size_t __size) { return ::malloc(__size); } + } // namespace std #endif Index: lib/Headers/__clang_cuda_runtime_wrapper.h === --- lib/Headers/__clang_cuda_runtime_wrapper.h +++ lib/Headers/__clang_cuda_runtime_wrapper.h @@ -169,6 +169,10 @@ // CUDA headers. Alas, device_functions.hpp included below needs it. static inline __device__ void __brkpt(int c) { __brkpt(); } +// We also need extern "C" decls for device-side allocator functions. +extern "C" __device__ void free(void *__ptr); +extern "C" __device__ void *malloc(size_t __size); + // Now include *.hpp with definitions of various GPU functions. Alas, // a lot of thins get declared/defined with __host__ attribute which // we don't want and we have to define it out. We also have to include Index: lib/Headers/__clang_cuda_cmath.h === --- lib/Headers/__clang_cuda_cmath.h +++ lib/Headers/__clang_cuda_cmath.h @@ -218,6 +218,9 @@ __DEVICE__ float trunc(float x) { return ::truncf(x); } __DEVICE__ double trunc(double x) { return ::trunc(x); } +__DEVICE__ void free(void *__ptr) { return ::free(__ptr); } +__DEVICE__ void *malloc(size_t __size) { return ::malloc(__size); } + } // namespace std #endif ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [clang-tools-extra] r258926 - [clang-tidy] Fix documentation.
On Wed, Jan 27, 2016 at 3:37 AM, Alexander Kornienko via cfe-commits wrote: > Author: alexfh > Date: Wed Jan 27 05:37:19 2016 > New Revision: 258926 > > URL: http://llvm.org/viewvc/llvm-project?rev=258926&view=rev > Log: > [clang-tidy] Fix documentation. > > Fixed broken links to cppcoreguidelines (anchors specified in the .md file > should be used, not automatic anchors generated by github). > > Fixed formatting, array_view -> span, fixed sphinx errors in > misc-definitions-in-headers.rst. Merged to 3.8 in r258959. ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r258960 - docs/conf.py: update copyright year
Author: hans Date: Wed Jan 27 12:29:16 2016 New Revision: 258960 URL: http://llvm.org/viewvc/llvm-project?rev=258960&view=rev Log: docs/conf.py: update copyright year Modified: clang-tools-extra/trunk/docs/conf.py Modified: clang-tools-extra/trunk/docs/conf.py URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/conf.py?rev=258960&r1=258959&r2=258960&view=diff == --- clang-tools-extra/trunk/docs/conf.py (original) +++ clang-tools-extra/trunk/docs/conf.py Wed Jan 27 12:29:16 2016 @@ -12,6 +12,7 @@ # serve to show the default. import sys, os +from datetime import date # If extensions (or modules to document with autodoc) are in another directory, # add these directories to sys.path here. If the directory is relative to the @@ -41,7 +42,7 @@ master_doc = 'index' # General information about the project. project = u'Extra Clang Tools' -copyright = u'2007-2015, The Clang Team' +copyright = u'2007-%d, The Clang Team' % date.today().year # The version info for the project you're documenting, acts as replacement for # |version| and |release|, also used in various other places throughout the ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r258962 - Emit calls to objc_unsafeClaimAutoreleasedReturnValue when
Author: rjmccall Date: Wed Jan 27 12:32:30 2016 New Revision: 258962 URL: http://llvm.org/viewvc/llvm-project?rev=258962&view=rev Log: Emit calls to objc_unsafeClaimAutoreleasedReturnValue when reclaiming a call result in order to ignore it or assign it to an __unsafe_unretained variable. This avoids adding an unwanted retain/release pair when the return value is not actually returned autoreleased (e.g. when it is returned from a nonatomic getter or a typical collection accessor). This runtime function is only available on the latest Apple OS releases; the backwards-compatibility story is that you don't get the optimization unless your deployment target is recent enough. Sorry. rdar://20530049 Added: cfe/trunk/test/CodeGenObjC/arc-unsafeclaim.m Modified: cfe/trunk/include/clang/Basic/ObjCRuntime.h cfe/trunk/lib/CodeGen/CGDecl.cpp cfe/trunk/lib/CodeGen/CGExprScalar.cpp cfe/trunk/lib/CodeGen/CGObjC.cpp cfe/trunk/lib/CodeGen/CodeGenFunction.h cfe/trunk/lib/CodeGen/CodeGenModule.h Modified: cfe/trunk/include/clang/Basic/ObjCRuntime.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/ObjCRuntime.h?rev=258962&r1=258961&r2=258962&view=diff == --- cfe/trunk/include/clang/Basic/ObjCRuntime.h (original) +++ cfe/trunk/include/clang/Basic/ObjCRuntime.h Wed Jan 27 12:32:30 2016 @@ -308,6 +308,23 @@ public: } } + /// Is objc_unsafeClaimAutoreleasedReturnValue available? + bool hasARCUnsafeClaimAutoreleasedReturnValue() const { +switch (getKind()) { +case MacOSX: + return getVersion() >= VersionTuple(10, 11); +case iOS: + return getVersion() >= VersionTuple(9); +case WatchOS: + return getVersion() >= VersionTuple(2); +case GNUstep: + return false; + +default: + return false; +} + } + /// \brief Try to parse an Objective-C runtime specification from the given /// string. /// Modified: cfe/trunk/lib/CodeGen/CGDecl.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDecl.cpp?rev=258962&r1=258961&r2=258962&view=diff == --- cfe/trunk/lib/CodeGen/CGDecl.cpp (original) +++ cfe/trunk/lib/CodeGen/CGDecl.cpp Wed Jan 27 12:32:30 2016 @@ -715,8 +715,7 @@ void CodeGenFunction::EmitScalarInit(con llvm_unreachable("present but none"); case Qualifiers::OCL_ExplicitNone: -// nothing to do -value = EmitScalarExpr(init); +value = EmitARCUnsafeUnretainedScalarExpr(init); break; case Qualifiers::OCL_Strong: { Modified: cfe/trunk/lib/CodeGen/CGExprScalar.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprScalar.cpp?rev=258962&r1=258961&r2=258962&view=diff == --- cfe/trunk/lib/CodeGen/CGExprScalar.cpp (original) +++ cfe/trunk/lib/CodeGen/CGExprScalar.cpp Wed Jan 27 12:32:30 2016 @@ -1366,8 +1366,9 @@ Value *ScalarExprEmitter::VisitCastExpr( QualType DestTy = CE->getType(); CastKind Kind = CE->getCastKind(); - if (!DestTy->isVoidType()) -TestAndClearIgnoreResultAssign(); + // These cases are generally not written to ignore the result of + // evaluating their sub-expressions, so we clear this now. + bool Ignored = TestAndClearIgnoreResultAssign(); // Since almost all cast kinds apply to scalars, this switch doesn't have // a default case, so the compiler will warn on a missing case. The cases @@ -1494,11 +1495,8 @@ Value *ScalarExprEmitter::VisitCastExpr( return CGF.EmitARCRetainScalarExpr(E); case CK_ARCConsumeObject: return CGF.EmitObjCConsumeObject(E->getType(), Visit(E)); - case CK_ARCReclaimReturnedObject: { -llvm::Value *value = Visit(E); -value = CGF.EmitARCRetainAutoreleasedReturnValue(value); -return CGF.EmitObjCConsumeObject(E->getType(), value); - } + case CK_ARCReclaimReturnedObject: +return CGF.EmitARCReclaimReturnedObject(E, /*allowUnsafe*/ Ignored); case CK_ARCExtendBlockObject: return CGF.EmitARCExtendBlockObject(E); @@ -2993,15 +2991,17 @@ Value *ScalarExprEmitter::VisitBinAssign std::tie(LHS, RHS) = CGF.EmitARCStoreAutoreleasing(E); break; + case Qualifiers::OCL_ExplicitNone: +std::tie(LHS, RHS) = CGF.EmitARCStoreUnsafeUnretained(E, Ignore); +break; + case Qualifiers::OCL_Weak: RHS = Visit(E->getRHS()); LHS = EmitCheckedLValue(E->getLHS(), CodeGenFunction::TCK_Store); RHS = CGF.EmitARCStoreWeak(LHS.getAddress(), RHS, Ignore); break; - // No reason to do any of these differently. case Qualifiers::OCL_None: - case Qualifiers::OCL_ExplicitNone: // __block variables need to have the rhs evaluated first, plus // this should improve codegen just a little. RHS = Visit(E->getRHS()); Modified: cfe/trunk/lib/CodeGen/CGObjC.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/l
[PATCH] D16639: [libcxx] Limit catopen usage to unix-like OSes
bcraig created this revision. bcraig added reviewers: mclow.lists, ed, jfb, EricWF, jroelofs. bcraig added a subscriber: cfe-commits. Herald added subscribers: srhines, danalbert, tberghammer, jfb. Operating systems that are not unix-like are unlikely to have access to catopen. Instead of black-listing each one, we now filter out all non-unix operating systems first. We then exclude the unix-like operating systems that don't have catopen. _WIN32 counts as a unix-like operating system because of cygwin. The catopen existence check was moved form __config to locale because locale is the only user of this information. http://reviews.llvm.org/D16639 Files: include/__config include/locale Index: include/locale === --- include/locale +++ include/locale @@ -199,6 +199,15 @@ // has had a chance to bake for a bit #include #endif + +#if defined(__unix__) || (defined(__APPLE__) && defined(__MACH__)) +// Most unix variants have catopen. These are the specific ones that don't. +#if !defined(_WIN32) && !defined(__ANDROID__) && !defined(_NEWLIB_VERSION) && \ +!defined(__CloudABI__) +#define _LIBCPP_HAS_CATOPEN 1 +#endif +#endif + #ifdef _LIBCPP_HAS_CATOPEN #include #endif Index: include/__config === --- include/__config +++ include/__config @@ -713,11 +713,6 @@ #define _LIBCPP_LOCALE__L_EXTENSIONS 1 #endif -#if !defined(_WIN32) && !defined(__ANDROID__) && !defined(_NEWLIB_VERSION) && \ -!defined(__CloudABI__) -#define _LIBCPP_HAS_CATOPEN 1 -#endif - #ifdef __FreeBSD__ #define _DECLARE_C99_LDBL_MATH 1 #endif Index: include/locale === --- include/locale +++ include/locale @@ -199,6 +199,15 @@ // has had a chance to bake for a bit #include #endif + +#if defined(__unix__) || (defined(__APPLE__) && defined(__MACH__)) +// Most unix variants have catopen. These are the specific ones that don't. +#if !defined(_WIN32) && !defined(__ANDROID__) && !defined(_NEWLIB_VERSION) && \ +!defined(__CloudABI__) +#define _LIBCPP_HAS_CATOPEN 1 +#endif +#endif + #ifdef _LIBCPP_HAS_CATOPEN #include #endif Index: include/__config === --- include/__config +++ include/__config @@ -713,11 +713,6 @@ #define _LIBCPP_LOCALE__L_EXTENSIONS 1 #endif -#if !defined(_WIN32) && !defined(__ANDROID__) && !defined(_NEWLIB_VERSION) && \ -!defined(__CloudABI__) -#define _LIBCPP_HAS_CATOPEN 1 -#endif - #ifdef __FreeBSD__ #define _DECLARE_C99_LDBL_MATH 1 #endif ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16634: [libcxx] Whitelist inclusion of sysctl.h instead of blacklisting
bcraig added a comment. Added cfe-commits to subscriber list. http://reviews.llvm.org/D16634 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
LLVM buildmaster will be restarted tonight
Hello everyone, LLVM buildmaster will be updated and restarted after 6 PM Pacific time today. Thanks Galina ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15120: Add support for __float128 type to be used by targets that support it
hubert.reinterpretcast added a comment. In http://reviews.llvm.org/D15120#337384, @rjmccall wrote: > I think it's not unlikely that float128_t will see future standardization (as > an optional extension, of course), and it would be strange for an operation > between two types to not consistently yield the type of higher rank. It remains that the present standardization effort (as `_Float128`) does not imbue the "interchange" type with inherently higher rank than `long double`. In a parallel to the treatment of extended integer types, the "standard" type has higher rank when the set of values are equivalent between the two. This is consistent with the GCC implementation (online compiler: http://melpon.org/wandbox/permlink/tM3PyGWC5WTWIdKP). > I could see an argument that we should not treat float128_t as a distinct > type from long double on targets where they have the same implementation. > That might avoid the need for special-case manglings. I would prefer this as well. As I have mentioned before, for `__float128` and `-mlong-double-128` on x86-64, GCC ends up with an undesirable situation of treating the types as distinct, but mangling them the same. In the TS, `_Float128 Repository: rL LLVM http://reviews.llvm.org/D15120 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation
Sean: Adding a new CC1 option ProfileClangInstr will make things cleaner. But this won't solve the problem. The root of all the mess is there is no driver level option for IR instrumentation. I need to either "hijack" the -Xclang option, or move the logic to CompilerInvocation.cpp, which both you don't like. The reason is I have to reply on the Driver option -fprofile-instr-generate to have the right link line for the profile library. -fprofile-instr-generate will set the Instrumentation to Clang (regardless use of current cc1 option of -fprofile-instr-generate, or the new proposed -fprofile-clang-instr). For IR instrumentation where the user specifies "-Xclang -fprofile-ir-instr", I need to overwrite the driver level option. To get that, I either parse the -Xclang value (this is "hijack), or I handle it in CC1 (in CompilerInvocation.cpp). I don't see a way to avoid it. Can we use a hidden driver option here for IR instrumentation? On Tue, Jan 26, 2016 at 5:01 PM, Sean Silva wrote: > silvas added inline comments. > > > Comment at: lib/Driver/Tools.cpp:5520 > @@ +5519,3 @@ > +A->claim(); > +if ((StringRef(A->getValue(0)) == "-fprofile-ir-instr") && > +(Args.hasArg(options::OPT_fprofile_instr_generate) || > > This is definitely not the right thing to do. Don't hijack -Xclang (which is > a completely generic thing). > > > Comment at: lib/Frontend/CompilerInvocation.cpp:483 > @@ +482,3 @@ > +Opts.ProfileIRInstr = true; > + else > +// Opts.ClangProfInstrGen will be used for Clang instrumentation only. > > This still isn't factored right. I think at this point the meaning of the > driver-level options is not really useful at CC1 level (too convoluted) for > it to be useful to pass them through. > > The right thing to do here is probably more like: > - refactor so that we have 4 individual CC1 options for InstrProfileOutput, > InstrProfileInput, ProfileIRInstr, ClangProfInstrGen (although probably > rename ClangProfInstrGen and ProfileIRInstr to be consistent with each other, > e.g. "ProfileIRInstr" and "ProfileClangInstr"). > - add logic in Driver to convert from the driver-level options to the CC1 > options. > > > Comment at: test/CodeGen/pgo-instrumentation.c:5 > @@ +4,3 @@ > +// RUN: %clang -O2 -c -Xclang -fprofile-ir-instr -fprofile-instr-generate %s > -mllvm -debug-pass=Structure 2>&1 | FileCheck %s > -check-prefix=CHECK-PGOGENPASS-INVOKED-V1 > +// CHECK-PGOGENPASS-INVOKED-V1: PGOInstrumentationGenPass > +// > > It isn't clear what V1/V2 mean. > > > http://reviews.llvm.org/D15829 > > > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16638: [CUDA] Added device-side std::{malloc/free}
jlebar accepted this revision. This revision is now accepted and ready to land. Comment at: lib/Headers/__clang_cuda_cmath.h:222 @@ +221,3 @@ +__DEVICE__ void free(void *__ptr) { return ::free(__ptr); } +__DEVICE__ void *malloc(size_t __size) { return ::malloc(__size); } + Not really math stuff; maybe they should live in the main header? Comment at: lib/Headers/__clang_cuda_runtime_wrapper.h:172 @@ -171,1 +171,3 @@ +// We also need extern "C" decls for device-side allocator functions. +extern "C" __device__ void free(void *__ptr); Perhaps add a comment explaining where these are implemented? http://reviews.llvm.org/D16638 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation
xur added a comment. Sean: Adding a new CC1 option ProfileClangInstr will make things cleaner. But this won't solve the problem. The root of all the mess is there is no driver level option for IR instrumentation. I need to either "hijack" the -Xclang option, or move the logic to CompilerInvocation.cpp, which both you don't like. The reason is I have to reply on the Driver option -fprofile-instr-generate to have the right link line for the profile library. -fprofile-instr-generate will set the Instrumentation to Clang (regardless use of current cc1 option of -fprofile-instr-generate, or the new proposed -fprofile-clang-instr). For IR instrumentation where the user specifies "-Xclang -fprofile-ir-instr", I need to overwrite the driver level option. To get that, I either parse the -Xclang value (this is "hijack), or I handle it in CC1 (in CompilerInvocation.cpp). I don't see a way to avoid it. Can we use a hidden driver option here for IR instrumentation? http://reviews.llvm.org/D15829 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation
davidxl added inline comments. Comment at: lib/Driver/Tools.cpp:5520 @@ +5519,3 @@ +A->claim(); +if ((StringRef(A->getValue(0)) == "-fprofile-ir-instr") && +(Args.hasArg(options::OPT_fprofile_instr_generate) || silvas wrote: > This is definitely not the right thing to do. Don't hijack -Xclang (which is > a completely generic thing). Why do we need special handling here ? will the existing behavior (simple forwarding) work? Note that intention of the cc1 option is to hide it from the user but make it used by the developers -- so we can make assumption that this option is used only when -fprofile-instr-generate[=...] is specified. You can add diagnostics later during cc1 option processing if it is not the case. Also think about making it easier to flip the default behavior in the future, it might be better to make the cc1 option look like this: -fprofile-instrumentor=[clang|LLVM] if the option is missing, the current default will be 'clang'. In the future just switch it to 'llvm'. Comment at: lib/Frontend/CompilerInvocation.cpp:483 @@ +482,3 @@ +Opts.ProfileIRInstr = true; + else +// Opts.ClangProfInstrGen will be used for Clang instrumentation only. silvas wrote: > This still isn't factored right. I think at this point the meaning of the > driver-level options is not really useful at CC1 level (too convoluted) for > it to be useful to pass them through. > > The right thing to do here is probably more like: > - refactor so that we have 4 individual CC1 options for InstrProfileOutput, > InstrProfileInput, ProfileIRInstr, ClangProfInstrGen (although probably > rename ClangProfInstrGen and ProfileIRInstr to be consistent with each other, > e.g. "ProfileIRInstr" and "ProfileClangInstr"). > - add logic in Driver to convert from the driver-level options to the CC1 > options. It is already pretty close to your proposal -- the only missing thing is cc1 option for ClangProfInstrGen. However given that IR and Clang InstrGen are mutually exclusive, is an additional CC1 option needed? http://reviews.llvm.org/D15829 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r258976 - ARMv7k: select ABI based on v7k Arch rather than watchos OS.
Author: tnorthover Date: Wed Jan 27 13:32:40 2016 New Revision: 258976 URL: http://llvm.org/viewvc/llvm-project?rev=258976&view=rev Log: ARMv7k: select ABI based on v7k Arch rather than watchos OS. Various bits we'd like to use the new ABI actually compile with "-arch armv7k -miphoneos-version-min=9.0". Not ideal, but also not ridiculous given how slices work. Modified: cfe/trunk/lib/Basic/Targets.cpp cfe/trunk/lib/CodeGen/TargetInfo.cpp cfe/trunk/lib/Driver/ToolChains.cpp cfe/trunk/lib/Driver/Tools.cpp cfe/trunk/test/Driver/arch-armv7k.c Modified: cfe/trunk/lib/Basic/Targets.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets.cpp?rev=258976&r1=258975&r2=258976&view=diff == --- cfe/trunk/lib/Basic/Targets.cpp (original) +++ cfe/trunk/lib/Basic/Targets.cpp Wed Jan 27 13:32:40 2016 @@ -4500,7 +4500,7 @@ public: Triple.getOS() == llvm::Triple::UnknownOS || StringRef(CPU).startswith("cortex-m")) { setABI("aapcs"); - } else if (Triple.isWatchOS()) { + } else if (Triple.isWatchABI()) { setABI("aapcs16"); } else { setABI("apcs-gnu"); @@ -4716,7 +4716,7 @@ public: // Unfortunately, __ARM_ARCH_7K__ is now more of an ABI descriptor. The CPU // happens to be Cortex-A7 though, so it should still get __ARM_ARCH_7A__. -if (getTriple().isWatchOS()) +if (getTriple().isWatchABI()) Builder.defineMacro("__ARM_ARCH_7K__", "2"); if (!CPUAttr.empty()) @@ -4905,8 +4905,8 @@ public: BuiltinVaListKind getBuiltinVaListKind() const override { return IsAAPCS ? AAPCSABIBuiltinVaList - : (getTriple().isWatchOS() ? TargetInfo::CharPtrBuiltinVaList - : TargetInfo::VoidPtrBuiltinVaList); + : (getTriple().isWatchABI() ? TargetInfo::CharPtrBuiltinVaList + : TargetInfo::VoidPtrBuiltinVaList); } ArrayRef getGCCRegNames() const override; ArrayRef getGCCRegAliases() const override; @@ -5247,7 +5247,7 @@ public: // ARMleTargetInfo. MaxAtomicInlineWidth = 64; -if (Triple.isWatchOS()) { +if (Triple.isWatchABI()) { // Darwin on iOS uses a variant of the ARM C++ ABI. TheCXXABI.set(TargetCXXABI::WatchOS); Modified: cfe/trunk/lib/CodeGen/TargetInfo.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/TargetInfo.cpp?rev=258976&r1=258975&r2=258976&view=diff == --- cfe/trunk/lib/CodeGen/TargetInfo.cpp (original) +++ cfe/trunk/lib/CodeGen/TargetInfo.cpp Wed Jan 27 13:32:40 2016 @@ -4939,7 +4939,7 @@ void ARMABIInfo::computeInfo(CGFunctionI /// Return the default calling convention that LLVM will use. llvm::CallingConv::ID ARMABIInfo::getLLVMDefaultCC() const { // The default calling convention that LLVM will infer. - if (isEABIHF() || getTarget().getTriple().isWatchOS()) + if (isEABIHF() || getTarget().getTriple().isWatchABI()) return llvm::CallingConv::ARM_AAPCS_VFP; else if (isEABI()) return llvm::CallingConv::ARM_AAPCS; Modified: cfe/trunk/lib/Driver/ToolChains.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains.cpp?rev=258976&r1=258975&r2=258976&view=diff == --- cfe/trunk/lib/Driver/ToolChains.cpp (original) +++ cfe/trunk/lib/Driver/ToolChains.cpp Wed Jan 27 13:32:40 2016 @@ -1075,7 +1075,9 @@ bool Darwin::UseSjLjExceptions(const Arg return false; // Only watchOS uses the new DWARF/Compact unwinding method. - return !isTargetWatchOS(); + llvm::Triple Triple(ComputeLLVMTriple(Args)); + return !(Triple.getArchName() == "armv7k" || + Triple.getArchName() == "thumbv7k") && !isTargetWatchOS(); } bool MachO::isPICDefault() const { return true; } Modified: cfe/trunk/lib/Driver/Tools.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=258976&r1=258975&r2=258976&view=diff == --- cfe/trunk/lib/Driver/Tools.cpp (original) +++ cfe/trunk/lib/Driver/Tools.cpp Wed Jan 27 13:32:40 2016 @@ -698,6 +698,7 @@ arm::FloatABI arm::getARMFloatABI(const case llvm::Triple::TvOS: { // Darwin defaults to "softfp" for v6 and v7. ABI = (SubArch == 6 || SubArch == 7) ? FloatABI::SoftFP : FloatABI::Soft; + ABI = Triple.isWatchABI() ? FloatABI::Hard : ABI; break; } case llvm::Triple::WatchOS: @@ -954,7 +955,7 @@ void Clang::AddARMTargetArgs(const llvm: } else if (Triple.isOSBinFormatMachO()) { if (useAAPCSForMachO(Triple)) { ABIName = "aapcs"; -} else if (Triple.isWatchOS()) { +} else if (Triple.isWatchABI()) { ABIName = "aapcs16"; } else { ABIName = "apcs-gnu"; Mod
Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation
On Wed, Jan 27, 2016 at 11:31 AM, David Li wrote: > davidxl added inline comments. > > > Comment at: lib/Driver/Tools.cpp:5520 > @@ +5519,3 @@ > +A->claim(); > +if ((StringRef(A->getValue(0)) == "-fprofile-ir-instr") && > +(Args.hasArg(options::OPT_fprofile_instr_generate) || > > silvas wrote: >> This is definitely not the right thing to do. Don't hijack -Xclang (which is >> a completely generic thing). > Why do we need special handling here ? will the existing behavior (simple > forwarding) work? The original patch does the simple forwarding. In that case, we handle the cc1 options (choosing b/w IR or clang instrumentation) in CompilerInvocation.cpp. Sean and Chad think this logic should be driver. > > Note that intention of the cc1 option is to hide it from the user but make it > used by the developers -- so we can make assumption that this option is used > only when -fprofile-instr-generate[=...] is specified. You can add > diagnostics later during cc1 option processing if it is not the case. > > Also think about making it easier to flip the default behavior in the future, > it might be better to make the cc1 option look like this: > > -fprofile-instrumentor=[clang|LLVM] > > if the option is missing, the current default will be 'clang'. In the future > just switch it to 'llvm'. > > > Comment at: lib/Frontend/CompilerInvocation.cpp:483 > @@ +482,3 @@ > +Opts.ProfileIRInstr = true; > + else > +// Opts.ClangProfInstrGen will be used for Clang instrumentation only. > > silvas wrote: >> This still isn't factored right. I think at this point the meaning of the >> driver-level options is not really useful at CC1 level (too convoluted) for >> it to be useful to pass them through. >> >> The right thing to do here is probably more like: >> - refactor so that we have 4 individual CC1 options for InstrProfileOutput, >> InstrProfileInput, ProfileIRInstr, ClangProfInstrGen (although probably >> rename ClangProfInstrGen and ProfileIRInstr to be consistent with each >> other, e.g. "ProfileIRInstr" and "ProfileClangInstr"). >> - add logic in Driver to convert from the driver-level options to the CC1 >> options. > It is already pretty close to your proposal -- the only missing thing is cc1 > option for ClangProfInstrGen. However given that IR and Clang InstrGen are > mutually exclusive, is an additional CC1 option needed? > > > http://reviews.llvm.org/D15829 > > > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15120: Add support for __float128 type to be used by targets that support it
hubert.reinterpretcast added a comment. In http://reviews.llvm.org/D15120#337384, @rjmccall wrote: > I think it's not unlikely that float128_t will see future standardization (as > an optional extension, of course), and it would be strange for an operation > between two types to not consistently yield the type of higher rank. It remains that the present standardization effort (as `_Float128`) does not imbue the "interchange" type with inherently higher rank than `long double`. In a parallel to the treatment of extended integer types, the "standard" type has higher rank when the set of values are equivalent between the two. This is consistent with the GCC implementation (online compiler: http://melpon.org/wandbox/permlink/tM3PyGWC5WTWIdKP). > I could see an argument that we should not treat float128_t as a distinct > type from long double on targets where they have the same implementation. > That might avoid the need for special-case manglings. I would prefer this as well (insofar as it saves us from the common type issue). As I have mentioned before, for `__float128` and `-mlong-double-128` on x86-64, GCC ends up with an undesirable situation of treating the types as distinct, but mangling them the same. In the TS, `_Float128` is always distinct from `long double`, which is helpful for portability of `_Generic` expressions with both types. In the end, it seems to come down to what sort of code people will write. If overloads for both `__float128` and `long double` are the norm, then we will need to consider the types distinct. Repository: rL LLVM http://reviews.llvm.org/D15120 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16310: new clang-tidy checker misc-long-cast
LegalizeAdulthood added a comment. In http://reviews.llvm.org/D16310#335844, @danielmarjamaki wrote: > There were some new interesting warnings and imho they were TP. TP? http://reviews.llvm.org/D16310 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16012: Carry raw string literal information through to the AST StringLiteral representation
LegalizeAdulthood added a subscriber: LegalizeAdulthood. LegalizeAdulthood added a comment. In http://reviews.llvm.org/D16012#337208, @rsmith wrote: > What's the benefit of storing this? You can get the same effect by > re-lexing. We don't guarantee that the pretty printed version of the AST > comprises the same sequence of tokens in general. In writing clang-tidy checks, I've had to do re-lexing a number of times and personally I find it to be a complete PITA and very easy to get wrong, plus it results in many review iterations because of all the ways that StringRef enters the picture, whether you're using the plain lexer or the raw lexer, SourceRange or CharSourceRange, yadda yadda yadda. I thought the whole point of the AST was to do this work **once** and store the results of the work for tools. http://reviews.llvm.org/D16012 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r258979 - Class Property: handle class properties.
Author: mren Date: Wed Jan 27 14:00:32 2016 New Revision: 258979 URL: http://llvm.org/viewvc/llvm-project?rev=258979&view=rev Log: Class Property: handle class properties. At places where we handle instance properties, if necessary. rdar://23891898 Modified: cfe/trunk/lib/AST/DeclObjC.cpp cfe/trunk/lib/CodeGen/CGDebugInfo.cpp cfe/trunk/lib/Sema/SemaDeclObjC.cpp cfe/trunk/lib/Sema/SemaObjCProperty.cpp Modified: cfe/trunk/lib/AST/DeclObjC.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclObjC.cpp?rev=258979&r1=258978&r2=258979&view=diff == --- cfe/trunk/lib/AST/DeclObjC.cpp (original) +++ cfe/trunk/lib/AST/DeclObjC.cpp Wed Jan 27 14:00:32 2016 @@ -122,7 +122,7 @@ bool ObjCContainerDecl::HasUserDeclaredS // declaration of this property. If one found, presumably a setter will // be provided (properties declared in categories will not get // auto-synthesized). - for (const auto *P : Cat->instance_properties()) + for (const auto *P : Cat->properties()) if (P->getIdentifier() == Property->getIdentifier()) { if (P->getPropertyAttributes() & ObjCPropertyDecl::OBJC_PR_readwrite) return true; @@ -1837,7 +1837,7 @@ void ObjCProtocolDecl::collectInheritedP ProtocolPropertyMap &PM) const { if (const ObjCProtocolDecl *PDecl = getDefinition()) { bool MatchFound = false; -for (auto *Prop : PDecl->instance_properties()) { +for (auto *Prop : PDecl->properties()) { if (Prop == Property) continue; if (Prop->getIdentifier() == Property->getIdentifier()) { Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=258979&r1=258978&r2=258979&view=diff == --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original) +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Wed Jan 27 14:00:32 2016 @@ -1822,11 +1822,11 @@ llvm::DIType *CGDebugInfo::CreateTypeDef { llvm::SmallPtrSet PropertySet; for (const ObjCCategoryDecl *ClassExt : ID->known_extensions()) - for (auto *PD : ClassExt->instance_properties()) { + for (auto *PD : ClassExt->properties()) { PropertySet.insert(PD->getIdentifier()); AddProperty(PD); } -for (const auto *PD : ID->instance_properties()) { +for (const auto *PD : ID->properties()) { // Don't emit duplicate metadata for properties that were already in a // class extension. if (!PropertySet.insert(PD->getIdentifier()).second) Modified: cfe/trunk/lib/Sema/SemaDeclObjC.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclObjC.cpp?rev=258979&r1=258978&r2=258979&view=diff == --- cfe/trunk/lib/Sema/SemaDeclObjC.cpp (original) +++ cfe/trunk/lib/Sema/SemaDeclObjC.cpp Wed Jan 27 14:00:32 2016 @@ -3637,7 +3637,7 @@ Decl *Sema::ActOnAtEnd(Scope *S, SourceR // ProcessPropertyDecl is responsible for diagnosing conflicts with any // user-defined setter/getter. It also synthesizes setter/getter methods // and adds them to the DeclContext and global method pools. - for (auto *I : CDecl->instance_properties()) + for (auto *I : CDecl->properties()) ProcessPropertyDecl(I); CDecl->setAtEndRange(AtEnd); } Modified: cfe/trunk/lib/Sema/SemaObjCProperty.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaObjCProperty.cpp?rev=258979&r1=258978&r2=258979&view=diff == --- cfe/trunk/lib/Sema/SemaObjCProperty.cpp (original) +++ cfe/trunk/lib/Sema/SemaObjCProperty.cpp Wed Jan 27 14:00:32 2016 @@ -1904,8 +1904,8 @@ Sema::AtomicPropertySetterGetterRules (O for (auto *Prop : Ext->instance_properties()) PM[Prop->getIdentifier()] = Prop; -for (ObjCContainerDecl::PropertyMap::iterator I = PM.begin(), E = PM.end(); - I != E; ++I) { + for (ObjCContainerDecl::PropertyMap::iterator I = PM.begin(), E = PM.end(); + I != E; ++I) { const ObjCPropertyDecl *Property = I->second; ObjCMethodDecl *GetterMethod = nullptr; ObjCMethodDecl *SetterMethod = nullptr; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16529: [clang-tidy] Add modernize-raw-string-literal check
LegalizeAdulthood added inline comments. Comment at: clang-tidy/modernize/RawStringLiteralCheck.cpp:27 @@ +26,3 @@ + // we only transform ASCII string literals + if (!Literal->isAscii()) +return false; alexfh wrote: > Why can't the check convert non-ascii string literals to corresponding raw > string literals? Just doing one thing at a time and not trying to create a "kitchen sink" check on the first pass. Comment at: clang-tidy/modernize/RawStringLiteralCheck.cpp:57 @@ +56,3 @@ + + // already a raw string literal if R comes before " + if (Text.find_first_of("R") < Text.find_first_of(R"(")")) alexfh wrote: > nit: Capitalization, punctuation. Same for other comments in the file. I don't understand what exactly you are asking me to change. There is something wrong with the comment? Comment at: clang-tidy/modernize/RawStringLiteralCheck.cpp:77 @@ +76,3 @@ + std::string Delimiter; + for (int Counter = 0; containsDelimiter(Bytes, Delimiter); ++Counter) { +if (Counter == 0) { alexfh wrote: > Why don't you try an empty delimiter? It will cover a majority of cases. > > Also, even though the function is used only when generating fix-it hints, it > still should be more effective than it is now. Most of the heap allocations > and copies caused by the usage of std::ostringstream, std::string and > std::string::operator+ can be avoided, e.g. like this: > > static const StringRef Delimiters[2][] = > {{"", "R\"(", ")\""}, {"lit", "R\"lit(", ")lit\""}, {"str", "R\"str(", > ")str\""}, > /* add more different ones to make it unlikely to meet all of these in a > single literal in the wild */}; > for (const auto &Delim : Delimiters) { > if (Bytes.find(Delim[2]) != StringRef::npos) > return (Delim[1] + Bytes + Delim[2]).str(); > } > // Give up gracefully on literals having all our delimiters. The very first thing tried is an empty delimiter. I coded a solution to the problem that always works. I find it less understandable to try a bunch of random delimiters and then fail if they are all present in the string. Then the whole algorithm becomes more complicated because even though I **could** construct a fixit, I'm failing stupidly because all my "favorite" delimiters were used in your string. Sometimes I feel these reviews over-obsess with allocations and efficiency instead of implementing a simple general algorithm that works reliably. We don't even have any measurements to show that this performance consideration is dominant or even noticeable, but I'm being asked to take a perfectly correct algorithm and cram it into a StringRef corset. Comment at: clang-tidy/modernize/RawStringLiteralCheck.cpp:103 @@ +102,3 @@ + // raw string literals require C++11 or later + if (!Options.CPlusPlus11 && !Options.CPlusPlus14 && !Options.CPlusPlus1z) +return; aaron.ballman wrote: > Can just use !Options.CPlusPlus11 -- 11 is implied in 14 and 1z. Then the documentation needs to be updated. (Actually I couldn't find any documentation on these flags and how they relate to each other because they use preprocessor hell to generate bitfields in a structure.) Comment at: clang-tidy/modernize/RawStringLiteralCheck.cpp:117 @@ +116,3 @@ +Result.Context->getLangOpts()); +StringRef Replacement = asRawStringLiteral(Literal); +diag(Literal->getLocStart(), alexfh wrote: > `Replacement` will point to deleted memory right after this line. For the record: I **hate** StringRef. I spend about 80% of my time on clang-tidy submissions dicking around with StringRef based on all the review comments. It is a very hard class to use properly. If a usage like this results in referencing deleted memory, it shouldn't even compile. http://reviews.llvm.org/D16529 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16529: [clang-tidy] Add modernize-raw-string-literal check
aaron.ballman added inline comments. Comment at: clang-tidy/modernize/RawStringLiteralCheck.cpp:57 @@ +56,3 @@ + + // already a raw string literal if R comes before " + if (Text.find_first_of("R") < Text.find_first_of(R"(")")) LegalizeAdulthood wrote: > alexfh wrote: > > nit: Capitalization, punctuation. Same for other comments in the file. > I don't understand what exactly you are asking me to change. There is > something wrong with the comment? ` // already a raw string literal if R comes before "` Should be: ` // Already a raw string literal if R comes before ".` http://reviews.llvm.org/D16529 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16529: [clang-tidy] Add modernize-raw-string-literal check
LegalizeAdulthood added inline comments. Comment at: clang-tidy/modernize/RawStringLiteralCheck.cpp:107 @@ +106,3 @@ + if (const auto *Literal = Result.Nodes.getNodeAs("lit")) +preferRawStringLiterals(Result, Literal); +} alexfh wrote: > I don't see a reason to have a separate function, I'd just inline it here. There is no reason to inline it. http://reviews.llvm.org/D16529 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r258980 - Class Property: create accessors (class methods) for class property.
Author: mren Date: Wed Jan 27 14:10:32 2016 New Revision: 258980 URL: http://llvm.org/viewvc/llvm-project?rev=258980&view=rev Log: Class Property: create accessors (class methods) for class property. Change a few places where we assume property accessors can only be instance methods. rdar://23891898 Added: cfe/trunk/test/SemaObjC/objc-class-property.m Modified: cfe/trunk/include/clang/AST/DeclObjC.h cfe/trunk/lib/Sema/SemaDeclObjC.cpp cfe/trunk/lib/Sema/SemaObjCProperty.cpp Modified: cfe/trunk/include/clang/AST/DeclObjC.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclObjC.h?rev=258980&r1=258979&r2=258980&view=diff == --- cfe/trunk/include/clang/AST/DeclObjC.h (original) +++ cfe/trunk/include/clang/AST/DeclObjC.h Wed Jan 27 14:10:32 2016 @@ -1754,8 +1754,9 @@ public: /// including in all categories except for category passed /// as argument. ObjCMethodDecl *lookupPropertyAccessor(const Selector Sel, - const ObjCCategoryDecl *Cat) const { -return lookupMethod(Sel, true/*isInstance*/, + const ObjCCategoryDecl *Cat, + bool IsClassProperty) const { +return lookupMethod(Sel, !IsClassProperty/*isInstance*/, false/*shallowCategoryLookup*/, true /* followsSuper */, Cat); Modified: cfe/trunk/lib/Sema/SemaDeclObjC.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclObjC.cpp?rev=258980&r1=258979&r2=258980&view=diff == --- cfe/trunk/lib/Sema/SemaDeclObjC.cpp (original) +++ cfe/trunk/lib/Sema/SemaDeclObjC.cpp Wed Jan 27 14:10:32 2016 @@ -2734,7 +2734,8 @@ void Sema::MatchAllMethodDeclarations(co for (auto *I : CDecl->class_methods()) { if (!ClsMapSeen.insert(I->getSelector()).second) continue; -if (!ClsMap.count(I->getSelector())) { +if (!I->isPropertyAccessor() && +!ClsMap.count(I->getSelector())) { if (ImmediateClass) WarnUndefinedMethod(*this, IMPDecl->getLocation(), I, IncompleteImpl, diag::warn_undef_method_impl); @@ -2743,12 +2744,14 @@ void Sema::MatchAllMethodDeclarations(co IMPDecl->getClassMethod(I->getSelector()); assert(CDecl->getClassMethod(I->getSelector()) && "Expected to find the method through lookup as well"); - if (!WarnCategoryMethodImpl) -WarnConflictingTypedMethods(ImpMethodDecl, I, -isa(CDecl)); - else -WarnExactTypedMethods(ImpMethodDecl, I, - isa(CDecl)); + // ImpMethodDecl may be null as in a @dynamic property. + if (ImpMethodDecl) { +if (!WarnCategoryMethodImpl) + WarnConflictingTypedMethods(ImpMethodDecl, I, + isa(CDecl)); +else if (!I->isPropertyAccessor()) + WarnExactTypedMethods(ImpMethodDecl, I, isa(CDecl)); + } } } Modified: cfe/trunk/lib/Sema/SemaObjCProperty.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaObjCProperty.cpp?rev=258980&r1=258979&r2=258980&view=diff == --- cfe/trunk/lib/Sema/SemaObjCProperty.cpp (original) +++ cfe/trunk/lib/Sema/SemaObjCProperty.cpp Wed Jan 27 14:10:32 2016 @@ -1745,7 +1745,8 @@ static void DiagnoseUnimplementedAccesso // the class is going to implement them. if (!SMap.count(Method) && (PrimaryClass == nullptr || - !PrimaryClass->lookupPropertyAccessor(Method, C))) { + !PrimaryClass->lookupPropertyAccessor(Method, C, + Prop->isClassProperty( { S.Diag(IMPDecl->getLocation(), isa(CDecl) ? diag::warn_setter_getter_impl_required_in_category : @@ -1916,8 +1917,12 @@ Sema::AtomicPropertySetterGetterRules (O if (!(AttributesAsWritten & ObjCPropertyDecl::OBJC_PR_atomic) && !(AttributesAsWritten & ObjCPropertyDecl::OBJC_PR_nonatomic)) { - GetterMethod = IMPDecl->getInstanceMethod(Property->getGetterName()); - SetterMethod = IMPDecl->getInstanceMethod(Property->getSetterName()); + GetterMethod = Property->isClassProperty() ? + IMPDecl->getClassMethod(Property->getGetterName()) : + IMPDecl->getInstanceMethod(Property->getGetterName()); + SetterMethod = Property->isClassProperty() ? + IMPDecl->getClassMethod(Property->getSetterName()) : + IMPDecl->getInstanceMethod(Property->getSetterName()); LookedUpGetterSetter = true; if (GetterMethod) { Diag(GetterMethod->getLocation(), @@ -1942,8 +1947,12 @@ Sema::Ato
r258981 - clang-format: [Java] Remove unnecessary line break after complex annotations
Author: djasper Date: Wed Jan 27 14:14:23 2016 New Revision: 258981 URL: http://llvm.org/viewvc/llvm-project?rev=258981&view=rev Log: clang-format: [Java] Remove unnecessary line break after complex annotations Before: @Annotation("Some" + " text") List list; After: @Annotation("Some" + " text") List list; Modified: cfe/trunk/lib/Format/ContinuationIndenter.cpp cfe/trunk/unittests/Format/FormatTestJava.cpp Modified: cfe/trunk/lib/Format/ContinuationIndenter.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/ContinuationIndenter.cpp?rev=258981&r1=258980&r2=258981&view=diff == --- cfe/trunk/lib/Format/ContinuationIndenter.cpp (original) +++ cfe/trunk/lib/Format/ContinuationIndenter.cpp Wed Jan 27 14:14:23 2016 @@ -151,6 +151,7 @@ bool ContinuationIndenter::mustBreak(con return true; if ((startsNextParameter(Current, Style) || Previous.is(tok::semi) || (Previous.is(TT_TemplateCloser) && Current.is(TT_StartOfName) && +Style.Language == FormatStyle::LK_Cpp && // FIXME: This is a temporary workaround for the case where clang-format // sets BreakBeforeParameter to avoid bin packing and this creates a // completely unnecessary line break after a template type that isn't Modified: cfe/trunk/unittests/Format/FormatTestJava.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestJava.cpp?rev=258981&r1=258980&r2=258981&view=diff == --- cfe/trunk/unittests/Format/FormatTestJava.cpp (original) +++ cfe/trunk/unittests/Format/FormatTestJava.cpp Wed Jan 27 14:14:23 2016 @@ -312,6 +312,9 @@ TEST_F(FormatTestJava, Annotations) { " String bbb) {}\n" "}", getStyleWithColumns(60)); + verifyFormat("@Annotation(\"Some\"\n" + "+ \" text\")\n" + "List list;"); } TEST_F(FormatTestJava, Generics) { ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation
We first need to nail the use model of the option, and then talk about implementation. To clarify, I think the following should be the way: (assuming the current clang instrumetnation is the default): 1) To turn on clang based instrumentation: -fprofile-instr-generate[=] 2) To turn on clang based instrumentation explicitly: -fprofile-instr-generate[=] -Xclang -fprofile-instrumentor=clang 3) To turn on IR based instrumentation: -fprofile-instr-generate[=] -Xclang -fprofile-instrumentor=llvm There is no need to change driver handling -- just let it forward. In CompilerInvocation.cpp, diagnostics can be emitted if user does -Xclang -fprofile-instrumentor=xxx without specfiying -fprofile-instr-generate Handling of -fprofile-instr-use=<> is different which does not depend on -fprofile-instrumentor, so it should be done in a different patch. David On Wed, Jan 27, 2016 at 11:38 AM, Rong Xu wrote: > On Wed, Jan 27, 2016 at 11:31 AM, David Li wrote: >> davidxl added inline comments. >> >> >> Comment at: lib/Driver/Tools.cpp:5520 >> @@ +5519,3 @@ >> +A->claim(); >> +if ((StringRef(A->getValue(0)) == "-fprofile-ir-instr") && >> +(Args.hasArg(options::OPT_fprofile_instr_generate) || >> >> silvas wrote: >>> This is definitely not the right thing to do. Don't hijack -Xclang (which >>> is a completely generic thing). >> Why do we need special handling here ? will the existing behavior (simple >> forwarding) work? > > The original patch does the simple forwarding. In that case, we handle > the cc1 options (choosing b/w IR or clang instrumentation) in > CompilerInvocation.cpp. Sean and Chad think this logic should be > driver. > >> >> Note that intention of the cc1 option is to hide it from the user but make >> it used by the developers -- so we can make assumption that this option is >> used only when -fprofile-instr-generate[=...] is specified. You can add >> diagnostics later during cc1 option processing if it is not the case. >> >> Also think about making it easier to flip the default behavior in the >> future, it might be better to make the cc1 option look like this: >> >> -fprofile-instrumentor=[clang|LLVM] >> >> if the option is missing, the current default will be 'clang'. In the future >> just switch it to 'llvm'. >> >> >> Comment at: lib/Frontend/CompilerInvocation.cpp:483 >> @@ +482,3 @@ >> +Opts.ProfileIRInstr = true; >> + else >> +// Opts.ClangProfInstrGen will be used for Clang instrumentation only. >> >> silvas wrote: >>> This still isn't factored right. I think at this point the meaning of the >>> driver-level options is not really useful at CC1 level (too convoluted) for >>> it to be useful to pass them through. >>> >>> The right thing to do here is probably more like: >>> - refactor so that we have 4 individual CC1 options for InstrProfileOutput, >>> InstrProfileInput, ProfileIRInstr, ClangProfInstrGen (although probably >>> rename ClangProfInstrGen and ProfileIRInstr to be consistent with each >>> other, e.g. "ProfileIRInstr" and "ProfileClangInstr"). >>> - add logic in Driver to convert from the driver-level options to the CC1 >>> options. >> It is already pretty close to your proposal -- the only missing thing is cc1 >> option for ClangProfInstrGen. However given that IR and Clang InstrGen are >> mutually exclusive, is an additional CC1 option needed? >> >> >> http://reviews.llvm.org/D15829 >> >> >> ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16639: [libcxx] Limit catopen usage to unix-like OSes
ed added a comment. I'm usually not a big fan of using definitions such as `__unix__`, because it is pretty vague and defined inconsistently. That said, if you're going to use this definition, then there is some good news: CloudABI doesn't define it, meaning that you could just remove it from this list. http://reviews.llvm.org/D16639 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15120: Add support for __float128 type to be used by targets that support it
rjmccall added a comment. In http://reviews.llvm.org/D15120#337552, @hubert.reinterpretcast wrote: > In http://reviews.llvm.org/D15120#337384, @rjmccall wrote: > > > I think it's not unlikely that float128_t will see future standardization > > (as an optional extension, of course), and it would be strange for an > > operation between two types to not consistently yield the type of higher > > rank. > > > It remains that the present standardization effort (as `_Float128`) does not > imbue the "interchange" type with inherently higher rank than `long double`. > In a parallel to the treatment of extended integer types, the "standard" type > has higher rank when the set of values are equivalent between the two. This > is consistent with the GCC implementation (online compiler: > http://melpon.org/wandbox/permlink/tM3PyGWC5WTWIdKP). Do we have anyone involved in this standardization effort? It seems like a really poor idea to having type ranking be target-dependent. > > I could see an argument that we should not treat float128_t as a distinct > > type from long double on targets where they have the same implementation. > > That might avoid the need for special-case manglings. > > > I would prefer this as well (insofar as it saves us from the common type > issue). As I have mentioned before, for `__float128` and `-mlong-double-128` > on x86-64, GCC ends up with an undesirable situation of treating the types as > distinct, but mangling them the same. Does Clang currently support that option? > In the TS, `_Float128` is always distinct from `long double`, which is > helpful for portability of `_Generic` expressions with both types. In the > end, it seems to come down to what sort of code people will write. If > overloads for both `__float128` and `long double` are the norm, then we will > need to consider the types distinct. Makes sense. Repository: rL LLVM http://reviews.llvm.org/D15120 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16634: [libcxx] Whitelist inclusion of sysctl.h instead of blacklisting
bcraig marked 2 inline comments as done. bcraig added a comment. http://reviews.llvm.org/D16634 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r258994 - ARMv7k: simplify logic for deciding sjlj-exceptions.
Author: tnorthover Date: Wed Jan 27 16:14:02 2016 New Revision: 258994 URL: http://llvm.org/viewvc/llvm-project?rev=258994&view=rev Log: ARMv7k: simplify logic for deciding sjlj-exceptions. Slight change of behaviour in the odd armv7+watchos case, which should match the other runtime components. Modified: cfe/trunk/lib/Driver/ToolChains.cpp cfe/trunk/test/Driver/arch-armv7k.c Modified: cfe/trunk/lib/Driver/ToolChains.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains.cpp?rev=258994&r1=258993&r2=258994&view=diff == --- cfe/trunk/lib/Driver/ToolChains.cpp (original) +++ cfe/trunk/lib/Driver/ToolChains.cpp Wed Jan 27 16:14:02 2016 @@ -1076,8 +1076,7 @@ bool Darwin::UseSjLjExceptions(const Arg // Only watchOS uses the new DWARF/Compact unwinding method. llvm::Triple Triple(ComputeLLVMTriple(Args)); - return !(Triple.getArchName() == "armv7k" || - Triple.getArchName() == "thumbv7k") && !isTargetWatchOS(); + return !Triple.isWatchABI(); } bool MachO::isPICDefault() const { return true; } Modified: cfe/trunk/test/Driver/arch-armv7k.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/arch-armv7k.c?rev=258994&r1=258993&r2=258994&view=diff == --- cfe/trunk/test/Driver/arch-armv7k.c (original) +++ cfe/trunk/test/Driver/arch-armv7k.c Wed Jan 27 16:14:02 2016 @@ -9,3 +9,6 @@ // match. // RUN: %clang -target x86_64-apple-macosx10.9 -arch armv7k -miphoneos-version-min=9.0 -c %s -### 2>&1 | FileCheck %s + +// RUN: %clang -target x86_64-apple-macosx10.9 -arch armv7 -mwatchos-version-min=9.0 -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SJLJ +// CHECK-SJLJ: "-fsjlj-exceptions" ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16538: [cc1as] Add MCTargetOptions argument to createAsmBackend
echristo added a comment. Hi Joel, Since you're likely going to need to start filling in the Options there anyhow, can you lift the one from the TODO and start filling it in? -eric http://reviews.llvm.org/D16538 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r258997 - Strengthen cfi-check-fail test.
Author: eugenis Date: Wed Jan 27 16:28:56 2016 New Revision: 258997 URL: http://llvm.org/viewvc/llvm-project?rev=258997&view=rev Log: Strengthen cfi-check-fail test. r258993 allows stricter testing for basic block labels by making sure that they are always followed by ":". Use this to improve the test. Modified: cfe/trunk/test/CodeGen/cfi-check-fail.c Modified: cfe/trunk/test/CodeGen/cfi-check-fail.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/cfi-check-fail.c?rev=258997&r1=258996&r2=258997&view=diff == --- cfe/trunk/test/CodeGen/cfi-check-fail.c (original) +++ cfe/trunk/test/CodeGen/cfi-check-fail.c Wed Jan 27 16:28:56 2016 @@ -14,58 +14,58 @@ void caller(void (*f)()) { // CHECK: %[[ICMP_NOT_NULL:.*]] = icmp ne i8* %[[DATA]], null // CHECK: br i1 %[[ICMP_NOT_NULL]], label %[[CONT0:.*]], label %[[TRAP:.*]], -// CHECK: [[TRAP]] +// CHECK: [[TRAP]]: // CHECK-NEXT: call void @llvm.trap() // CHECK-NEXT: unreachable -// CHECK: [[CONT0]] +// CHECK: [[CONT0]]: // CHECK: %[[A:.*]] = bitcast i8* %[[DATA]] to { i8, { i8*, i32, i32 }, i8* }* // CHECK: %[[KINDPTR:.*]] = getelementptr {{.*}} %[[A]], i32 0, i32 0 // CHECK: %[[KIND:.*]] = load i8, i8* %[[KINDPTR]], align 4 // CHECK: %[[NOT_0:.*]] = icmp ne i8 %[[KIND]], 0 // CHECK: br i1 %[[NOT_0]], label %[[CONT1:.*]], label %[[HANDLE0:.*]], !prof -// CHECK: [[HANDLE0]] +// CHECK: [[HANDLE0]]: // CHECK: %[[DATA0:.*]] = ptrtoint i8* %[[DATA]] to i64, // CHECK: %[[ADDR0:.*]] = ptrtoint i8* %[[ADDR]] to i64, // CHECK: call void @__ubsan_handle_cfi_check_fail(i64 %[[DATA0]], i64 %[[ADDR0]]) // CHECK: br label %[[CONT1]] -// CHECK: [[CONT1]] +// CHECK: [[CONT1]]: // CHECK: %[[NOT_1:.*]] = icmp ne i8 %[[KIND]], 1 // CHECK: br i1 %[[NOT_1]], label %[[CONT2:.*]], label %[[HANDLE1:.*]], !nosanitize -// CHECK: [[HANDLE1]] +// CHECK: [[HANDLE1]]: // CHECK-NEXT: call void @llvm.trap() // CHECK-NEXT: unreachable -// CHECK: [[CONT2]] +// CHECK: [[CONT2]]: // CHECK: %[[NOT_2:.*]] = icmp ne i8 %[[KIND]], 2 // CHECK: br i1 %[[NOT_2]], label %[[CONT3:.*]], label %[[HANDLE2:.*]], !prof -// CHECK: [[HANDLE2]] +// CHECK: [[HANDLE2]]: // CHECK: %[[DATA2:.*]] = ptrtoint i8* %[[DATA]] to i64, // CHECK: %[[ADDR2:.*]] = ptrtoint i8* %[[ADDR]] to i64, // CHECK: call void @__ubsan_handle_cfi_check_fail_abort(i64 %[[DATA2]], i64 %[[ADDR2]]) // CHECK: unreachable -// CHECK: [[CONT3]] +// CHECK: [[CONT3]]: // CHECK: %[[NOT_3:.*]] = icmp ne i8 %[[KIND]], 3 // CHECK: br i1 %[[NOT_3]], label %[[CONT4:.*]], label %[[HANDLE3:.*]], !prof -// CHECK: [[HANDLE3]] +// CHECK: [[HANDLE3]]: // CHECK: %[[DATA3:.*]] = ptrtoint i8* %[[DATA]] to i64, // CHECK: %[[ADDR3:.*]] = ptrtoint i8* %[[ADDR]] to i64, // CHECK: call void @__ubsan_handle_cfi_check_fail(i64 %[[DATA3]], i64 %[[ADDR3]]) // CHECK: br label %[[CONT4]] -// CHECK: [[CONT4]] +// CHECK: [[CONT4]]: // CHECK: %[[NOT_4:.*]] = icmp ne i8 %[[KIND]], 4 // CHECK: br i1 %[[NOT_4]], label %[[CONT5:.*]], label %[[HANDLE4:.*]], !nosanitize -// CHECK: [[HANDLE4]] +// CHECK: [[HANDLE4]]: // CHECK-NEXT: call void @llvm.trap() // CHECK-NEXT: unreachable -// CHECK: [[CONT5]] +// CHECK: [[CONT5]]: // CHECK: ret void ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16584: [libcxx] Work around for clang calling GAS after having already failed.
EricWF added a comment. I'm OK signing off on this myself. http://reviews.llvm.org/D16584 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D8822: Proposed fix for PR23076 (conditional branch debug line info)
dblaikie added a comment. Fair question on overloaded operators... Taking the original code: 1: int main() { 2: if ( 3: tr() 4: && 5: tr() 6: ) 7: func(); 8: if ( 9: fa() 10: && 11: tr() 12: ) 13: func(); 14: } & making tr/fa return a user defined type with a boolean member and provide op&& and op|| overloads that test the bool member in the obvious way. Clang (with patch) 3, 5, 4, 5, 7 9, 11, 10, 11, 14 So that's somewhat problematic - we shouldn't visit 11 at all. (but we are today, as is GCC... so maybe NBD?) GCC 4.8: 4, 2, 7 10, 8, 14 Clang ToT: 3, 5, 4, 3, 7 9, 11, 10, 9, 14 I think using the end of the condition is problematic/confusing. I'm not sure why this doesn't show up in the primitive value version, but it seems like it should (& we should end up stepping to the end of the condition (which would be the close paren of the function call, not the close paren of the 'if ()')) Perhaps we should use the close paren of the 'if ()' but tehre's no source location for that readily available - I guess the way to get there is to navigate to the next token from the end of the condition expression... ? http://reviews.llvm.org/D8822 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15999: Adding doxygen comments to the LLVM intrinsics (part 2, _wmmintrin_pclmul.h)
kromanova added a comment. In http://reviews.llvm.org/D15999#335653, @echristo wrote: > Honestly if they've been reviewed like that internally I'm ok with you just > committing them - especially if they look like this. > > The only concerns I'd have are in the case of "This intrinsic corresponds to > the instruction" (side note, use the "the"? I commented on a case > inline). This isn't always the case with all of our intrinsics when the > compiler lowers them to a shuffle intrinsic or some such, or it's optimized, > etc. Personally I'd leave that line out, though I understand it exists in a > lot of similar documentation. Hi Eric, I agree. Sometimes the instruction that corresponds to a specific intrinsic is optimized out, sometimes it will get lowered to something else, etc. However, I think keeping the instruction name in the documentation is extremely useful. In general, intrinsic documentation (especially in the form of comments) is not very complete. When I need to know what a specific intrinsic is doing (and I very often have to look up intrinsics!), I find the corresponding instruction name and go dig in AMD's or Intel's Architecture Programmer's manuals, where I could find all the details I need. Programmer's manuals instruction descriptions are much more detailed and complete. However, it's too much information to add to the comments. :) As you know, Intel's and MS's intrinsics guides are also specifying corresponding instruction names for the intrinsics. I suspect they had the same idea that I just described. I briefly chatted with Paul Robinson and he suggested to say "This intrinsic is equivalent to the instruction" instead, because this sentence doesn't give a false impression that one will definitely see this particular instruction in the generated code. Intel's intrinsics documentation says something like that, e.g: "The corresponding Intel® AVX instruction is VBLENDPD" What do you think/prefer? And, yes, I will add "the" before "corresponds to". Thanks! Easy enough with the generator. :) http://reviews.llvm.org/D15999 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15999: Adding doxygen comments to the LLVM intrinsics (part 2, _wmmintrin_pclmul.h)
kromanova added a comment. In http://reviews.llvm.org/D15999#335653, @echristo wrote: > Honestly if they've been reviewed like that internally I'm ok with you just > committing them - especially if they look like this. > > The only concerns I'd have are in the case of "This intrinsic corresponds to > the instruction" (side note, use the "the"? I commented on a case > inline). This isn't always the > case with all of our intrinsics when the compiler lowers them to a shuffle > intrinsic or some such, or it's optimized, etc. Personally I'd leave that > line out, though I understand it exists > in a lot of similar documentation. BTW, in some cases, our documentation won't be as specific and will say "This intrinsic (e.g. _mm_store_ps1 ) corresponds to the Shuffling + MOVSS instruction" or "No AVX instruction corresponds to this intrinsic (e.g. _mm256_set_pd)" or "Composite SSE2 instruction corresponds to this intrinsic (e.g. _mm_set_sd). Microsoft and Intel's documentation are very similar with this respect. See the description of _mm_set_sd intrinsic that I just mentioned. https://software.intel.com/en-us/node/524261 https://msdn.microsoft.com/en-us/library/dksztbt9%28v=vs.90%29.aspx http://reviews.llvm.org/D15999 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16605: Implement `std::experimental::ostream_joiner`
EricWF added a comment. - Please add the header to `test/libcxx/double_include.sh.cpp` - Please add a `_LIBCPP_VERSION` test in `test/libcxx/experimental/iterator` - The `ostream_joiner` type and it's members should be given visibility attributes. Comment at: include/experimental/iterator:62 @@ +61,3 @@ + +_LIBCPP_BEGIN_NAMESPACE_LFTS + Wrong namespace. This gives `std::experimental::fundamentals_v2`. This brings up the bigger question of how we want to manage two TS's. Comment at: include/experimental/iterator:64 @@ +63,3 @@ + +template > +class ostream_joiner { Is `_Delim` allowed to be a reference? We might get better diagnostics if we static assert some requirements on `_Delim`. Comment at: include/experimental/iterator:85 @@ +84,3 @@ +template +ostream_joiner& operator=(const _Tp& __v) +{ People are going to try and use this as an assignment operator. We either need to static assert in the body, or disable the operator entirely. I'm not sure which is better. It depends on if we want a generated operator=. Comment at: include/experimental/iterator:94 @@ +93,3 @@ + +ostream_joiner& operator*() _NOEXCEPT { return *this; } +ostream_joiner& operator++()_NOEXCEPT { return *this; } Unrelated note: Are we ever going to stop guarding `noexcept` behind a macro? Comment at: test/std/experimental/iterator/nothing_to_do.pass.cpp:1 @@ +1,2 @@ +//===--===// +// LIT will only emit a warning if all sub directories are empty as well. Do we need this file for `testit`^ Comment at: test/std/experimental/iterator/ostream.joiner/ostream.joiner.creation/make_ostream_joiner.pass.cpp:35 @@ +34,3 @@ +std::basic_stringstream sstream; +auto joiner = exp::make_ostream_joiner(sstream, d); +while (first != last) Can you test that `joiner` has the expected type? Comment at: test/std/experimental/iterator/ostream.joiner/ostream.joiner.ops/ostream_joiner.op.assign.pass.cpp:37 @@ +36,3 @@ +std::basic_ostream<_CharT, _Traits>& +operator<<(std::basic_ostream<_CharT, _Traits>& os, const mutating_delimiter &d) +{ return os << d.get(); } Nice test. Is there a reason to pass the delimiter as `const` here? It seems equally valid that the mutating delimiter has a non-const `operator<<`. http://reviews.llvm.org/D16605 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16605: Implement `std::experimental::ostream_joiner`
EricWF added inline comments. Comment at: include/experimental/iterator:85 @@ +84,3 @@ +template +ostream_joiner& operator=(const _Tp& __v) +{ EricWF wrote: > People are going to try and use this as an assignment operator. We either > need to static assert in the body, or disable the operator entirely. > I'm not sure which is better. It depends on if we want a generated operator=. Disregard the comment above. It's not possible to call this with an instance of `ostream_joiner`. I thought it might be possible like it is with constructors. http://reviews.llvm.org/D16605 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r259000 - [sancov] sancov tool documentation
Author: aizatsky Date: Wed Jan 27 17:56:12 2016 New Revision: 259000 URL: http://llvm.org/viewvc/llvm-project?rev=259000&view=rev Log: [sancov] sancov tool documentation Differential Revision: http://reviews.llvm.org/D16432 Modified: cfe/trunk/docs/SanitizerCoverage.rst Modified: cfe/trunk/docs/SanitizerCoverage.rst URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/SanitizerCoverage.rst?rev=259000&r1=258999&r2=259000&view=diff == --- cfe/trunk/docs/SanitizerCoverage.rst (original) +++ cfe/trunk/docs/SanitizerCoverage.rst Wed Jan 27 17:56:12 2016 @@ -94,6 +94,40 @@ numbers: cov.cc:3 cov.cc:5 +Sancov Tool +=== + +A new experimental ``sancov`` tool is developed to process coverage files. +The tool is part of LLVM project and is currently supported only on Linux. +It can handle symbolization tasks autonomously without needed any extra +support from environment. + +.. code-block:: console + +USAGE: sancov [options] + +Action (required) + -print- Print coverage addresses + -covered-functions- Print all covered funcions. + -not-covered-functions- Print all not covered funcions. + -html-report - Print HTML coverage report. + +Options + -blacklist= - Blacklist file (sanitizer blacklist format). + -demangle - Print demangled function name. + -obj= - Path to object file to be symbolized + -strip_path_prefix= - Strip this prefix from file paths in reports + + +Automatic HTML Report Generation + + +If ``*SAN_OPTIONS`` contains ``html_cov_report=1`` option set, then html +coverage report would be automatically generated alongside the coverage files. +The ``sancov`` binary should be present in ``PATH`` or +``sancov_path=http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13420: Fix deduction of __atomic_load's parameter types.
EricWF added a comment. ping. http://reviews.llvm.org/D13420 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation
xur updated this revision to Diff 46199. xur added a comment. This new version implemented the usage model David proposed. We now have a cc1 option -fprofile-instrumentor={llvm | clang} to choose which instrumentation to use. The slight difference from David's proposal is that we can use this option with -fprofile-instr-use. Thanks, -Rong http://reviews.llvm.org/D15829 Files: include/clang/Basic/DiagnosticDriverKinds.td include/clang/Driver/CC1Options.td include/clang/Frontend/CodeGenOptions.def lib/CodeGen/BackendUtil.cpp lib/CodeGen/CGStmt.cpp lib/CodeGen/CodeGenFunction.cpp lib/CodeGen/CodeGenFunction.h lib/CodeGen/CodeGenModule.cpp lib/CodeGen/CodeGenPGO.cpp lib/Frontend/CompilerInvocation.cpp test/CodeGen/Inputs/pgotest.profraw test/CodeGen/pgo-instrumentation.c Index: test/CodeGen/pgo-instrumentation.c === --- /dev/null +++ test/CodeGen/pgo-instrumentation.c @@ -0,0 +1,28 @@ +// Test if PGO instrumentation and use pass are invoked. +// +// Ensure Pass PGOInstrumentationGenPass is invoked. +// RUN: %clang -O2 -c -Xclang -fprofile-instrumentor=llvm -fprofile-instr-generate %s -mllvm -debug-pass=Structure 2>&1 | FileCheck %s -check-prefix=CHECK-PGOGENPASS-INVOKED-INSTR-GEN +// CHECK-PGOGENPASS-INVOKED-INSTR-GEN: PGOInstrumentationGenPass +// +// Ensure Pass PGOInstrumentationGenPass is invoked. +// RUN: %clang -O2 -c -Xclang -fprofile-instrumentor=llvm -fprofile-generate %s -mllvm -debug-pass=Structure 2>&1 | FileCheck %s -check-prefix=CHECK-PGOGENPASS-INVOKED-GEN +// CHECK-PGOGENPASS-INVOKED-GEN: PGOInstrumentationGenPass +// +// Ensure Pass PGOInstrumentationGenPass is not invoked. +// RUN: %clang -O2 -c -Xclang -fprofile-instrumentor=clang -fprofile-instr-generate %s -mllvm -debug-pass=Structure 2>&1 | FileCheck %s -check-prefix=CHECK-PGOGENPASS-INVOKED-INSTR-GEN-CLANG +// CHECK-PGOGENPASS-INVOKED-INSTR-GEN-CLANG-NOT: PGOInstrumentationGenPass +// +// Ensure Pass PGOInstrumentationUsePass is invoked. +// RUN: llvm-profdata merge -o %t.profdata %S/Inputs/pgotest.profraw +// RUN: %clang -O2 -c -Xclang -fprofile-instrumentor=llvm -fprofile-instr-use=%t.profdata %s -mllvm -debug-pass=Structure 2>&1 | FileCheck %s -check-prefix=CHECK-PGOUSEPASS-INVOKED-INSTR-USE +// CHECK-PGOUSEPASS-INVOKED-INSTR-USE: PGOInstrumentationUsePass +// +// Ensure Pass PGOInstrumentationUsePass is invoked. +// RUN: llvm-profdata merge -o %t.profdata %S/Inputs/pgotest.profraw +// RUN: %clang -O2 -c -Xclang -fprofile-instrumentor=llvm -fprofile-use=%t.profdata %s -mllvm -debug-pass=Structure 2>&1 | FileCheck %s -check-prefix=CHECK-PGOUSEPASS-INVOKED-USE +// CHECK-PGOUSEPASS-INVOKED-USE: PGOInstrumentationUsePass +// +// Ensure Pass PGOInstrumentationUsePass is not invoked. +// RUN: llvm-profdata merge -o %t.profdata %S/Inputs/pgotest.profraw +// RUN: %clang -O2 -c -Xclang -fprofile-instrumentor=clang -fprofile-use=%t.profdata %s -mllvm -debug-pass=Structure 2>&1 | FileCheck %s -check-prefix=CHECK-PGOUSEPASS-INVOKED-USE-CLANG +// CHECK-PGOUSEPASS-INVOKED-USE-CLANG-NOT: PGOInstrumentationUsePass Index: lib/Frontend/CompilerInvocation.cpp === --- lib/Frontend/CompilerInvocation.cpp +++ lib/Frontend/CompilerInvocation.cpp @@ -477,8 +477,28 @@ Opts.DisableIntegratedAS = Args.hasArg(OPT_fno_integrated_as); Opts.Autolink = !Args.hasArg(OPT_fno_autolink); Opts.SampleProfileFile = Args.getLastArgValue(OPT_fprofile_sample_use_EQ); - Opts.ProfileInstrGenerate = Args.hasArg(OPT_fprofile_instr_generate) || - Args.hasArg(OPT_fprofile_instr_generate_EQ); + + enum PGOInstrumentor { Unknown, Clang, LLVM }; + if (Arg *A = Args.getLastArg(OPT_fprofile_instrumentor_EQ)) { +StringRef Value = A->getValue(); +PGOInstrumentor Method = llvm::StringSwitch(Value) + .Case("clang", Clang) + .Case("llvm", LLVM) + .Default(Unknown); +if (Method == Unknown) + Diags.Report(diag::err_drv_invalid_pgo_instrumentor) + << A->getAsString(Args) << Value; +else { + Opts.ProfileIRInstr = (Method == LLVM); + Opts.ProfileClangInstr = (Method == Clang); +} + } else { +// Default PGO instrumentor is Clang instrumentation. +Opts.ProfileClangInstr = Args.hasArg(OPT_fprofile_instr_generate) || + Args.hasArg(OPT_fprofile_instr_generate_EQ); +Opts.ProfileIRInstr = false; + } + Opts.InstrProfileOutput = Args.getLastArgValue(OPT_fprofile_instr_generate_EQ); Opts.InstrProfileInput = Args.getLastArgValue(OPT_fprofile_instr_use_EQ); Opts.CoverageMapping = Index: lib/CodeGen/CodeGenPGO.cpp === --- lib/CodeGen/CodeGenPGO.cpp +++ lib/CodeGen/CodeGenPGO.cpp @@ -37,7 +37,7 @@ PGOReader ? PGOReader->getVersion() : llvm::IndexedInstrProf::Version); // If we're generating a profile, create a variable for the name. - if (C
Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation
davidxl added inline comments. Comment at: include/clang/Frontend/CodeGenOptions.def:106 @@ -105,3 +105,3 @@ -CODEGENOPT(ProfileInstrGenerate , 1, 0) ///< Instrument code to generate -///< execution counts to use with PGO. +CODEGENOPT(ProfileClangInstr, 1, 0) ///< Clang Instrumentation to generate +///< execution counts to use with PGO. For the sake of being consistent with the new cc1 option, making this an enum option is better: ENUM_CODEGENOPT(ProfileInstr,... ) which takes two legal values ProfInstrClang, ProfInstrLLVM I'd like to hear Sean's opinion on this. http://reviews.llvm.org/D15829 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation
silvas added inline comments. Comment at: lib/Frontend/CompilerInvocation.cpp:483 @@ +482,3 @@ +Opts.ProfileIRInstr = true; + else +// Opts.ClangProfInstrGen will be used for Clang instrumentation only. davidxl wrote: > silvas wrote: > > This still isn't factored right. I think at this point the meaning of the > > driver-level options is not really useful at CC1 level (too convoluted) for > > it to be useful to pass them through. > > > > The right thing to do here is probably more like: > > - refactor so that we have 4 individual CC1 options for InstrProfileOutput, > > InstrProfileInput, ProfileIRInstr, ClangProfInstrGen (although probably > > rename ClangProfInstrGen and ProfileIRInstr to be consistent with each > > other, e.g. "ProfileIRInstr" and "ProfileClangInstr"). > > - add logic in Driver to convert from the driver-level options to the CC1 > > options. > It is already pretty close to your proposal -- the only missing thing is cc1 > option for ClangProfInstrGen. However given that IR and Clang InstrGen are > mutually exclusive, is an additional CC1 option needed? There are 3 possibilities: - clang instr - ir instr - no instr A simple binary flag can only encode 2. So some sort of extra information needs to be passed. Off the top of my head, I thought of just adding an extra flag, but I like your suggestion `-fprofile-instrumentor=[clang|LLVM]`. That is clean and avoids having to deal with an error for the "clang instr + ir instr" case. http://reviews.llvm.org/D15829 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation
silvas added inline comments. Comment at: include/clang/Frontend/CodeGenOptions.def:106 @@ -105,3 +105,3 @@ -CODEGENOPT(ProfileInstrGenerate , 1, 0) ///< Instrument code to generate -///< execution counts to use with PGO. +CODEGENOPT(ProfileClangInstr, 1, 0) ///< Clang Instrumentation to generate +///< execution counts to use with PGO. davidxl wrote: > For the sake of being consistent with the new cc1 option, making this an enum > option is better: > > ENUM_CODEGENOPT(ProfileInstr,... ) which takes two legal values > ProfInstrClang, ProfInstrLLVM > > I'd like to hear Sean's opinion on this. SGTM. I was going to suggest something similar. This is more consistent with e.g. OPT_debug_info_kind_EQ and the other multi-value options. Comment at: lib/CodeGen/CodeGenModule.cpp:150 @@ -149,3 +149,3 @@ - if (!CodeGenOpts.InstrProfileInput.empty()) { + if (!CodeGenOpts.ProfileIRInstr && !CodeGenOpts.InstrProfileInput.empty()) { auto ReaderOrErr = This seems like a latent bug: `!CodeGenOpts.ProfileIRInstr` is true when we are doing no instrumentation. Using an ENUM_CODEGENOPT will fix this and related issues. Comment at: lib/Frontend/CompilerInvocation.cpp:495 @@ +494,3 @@ +} + } else { +// Default PGO instrumentor is Clang instrumentation. The work being done in this `else` is redundant. `-fprofile-instrumentor={clang,llvm,none}` (with "none" being the default if the argument isn't passed) covers this case as `-fprofile-instrumentor=clang`. The driver can be changed to translate `-fprofile-instr-generate` into `-fprofile-instrumentor=clang`. (and `-fprofile-instrument=` might be a better name) http://reviews.llvm.org/D15829 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16605: Implement `std::experimental::ostream_joiner`
mclow.lists added inline comments. Comment at: include/experimental/iterator:62 @@ +61,3 @@ + +_LIBCPP_BEGIN_NAMESPACE_LFTS + EricWF wrote: > Wrong namespace. This gives `std::experimental::fundamentals_v2`. This brings > up the bigger question of how we want to manage two TS's. I think that the right thing is to flip all the LFTS over to v2. Comment at: include/experimental/iterator:94 @@ +93,3 @@ + +ostream_joiner& operator*() _NOEXCEPT { return *this; } +ostream_joiner& operator++()_NOEXCEPT { return *this; } EricWF wrote: > Unrelated note: Are we ever going to stop guarding `noexcept` behind a macro? Maybe. Depends on how much work it turns out to be. I don't want to make that decision here, though. Comment at: test/std/experimental/iterator/nothing_to_do.pass.cpp:1 @@ +1,2 @@ +//===--===// +// EricWF wrote: > LIT will only emit a warning if all sub directories are empty as well. Do we > need this file for `testit`^ Well, if anyone was still running `testit`, yes. But it's a nice simple test. Does including the header file actually work? See `test/std/experimental/optional` or `string_view` for similar tests. Comment at: test/std/experimental/iterator/ostream.joiner/ostream.joiner.creation/make_ostream_joiner.pass.cpp:35 @@ +34,3 @@ +std::basic_stringstream sstream; +auto joiner = exp::make_ostream_joiner(sstream, d); +while (first != last) EricWF wrote: > Can you test that `joiner` has the expected type? Sure. Comment at: test/std/experimental/iterator/ostream.joiner/ostream.joiner.ops/ostream_joiner.op.assign.pass.cpp:37 @@ +36,3 @@ +std::basic_ostream<_CharT, _Traits>& +operator<<(std::basic_ostream<_CharT, _Traits>& os, const mutating_delimiter &d) +{ return os << d.get(); } EricWF wrote: > Nice test. Is there a reason to pass the delimiter as `const` here? It seems > equally valid that the mutating delimiter has a non-const `operator<<`. I'll add that as well. http://reviews.llvm.org/D16605 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16605: Implement `std::experimental::ostream_joiner`
mclow.lists updated this revision to Diff 46203. mclow.lists added a comment. Addressed Eric's comments. (at least some of them) http://reviews.llvm.org/D16605 Files: test/std/experimental/iterator/nothing_to_do.pass.cpp test/std/experimental/iterator/ostream.joiner/ostream.joiner.cons/ostream_joiner.cons.pass.cpp test/std/experimental/iterator/ostream.joiner/ostream.joiner.creation/make_ostream_joiner.pass.cpp test/std/experimental/iterator/ostream.joiner/ostream.joiner.ops/ostream_joiner.op.assign.pass.cpp test/std/experimental/iterator/ostream.joiner/ostream.joiner.ops/ostream_joiner.op.postincrement.pass.cpp test/std/experimental/iterator/ostream.joiner/ostream.joiner.ops/ostream_joiner.op.pretincrement.pass.cpp test/std/experimental/iterator/ostream.joiner/ostream.joiner.ops/ostream_joiner.op.star.pass.cpp Index: test/std/experimental/iterator/ostream.joiner/ostream.joiner.ops/ostream_joiner.op.star.pass.cpp === --- test/std/experimental/iterator/ostream.joiner/ostream.joiner.ops/ostream_joiner.op.star.pass.cpp +++ test/std/experimental/iterator/ostream.joiner/ostream.joiner.ops/ostream_joiner.op.star.pass.cpp @@ -0,0 +1,46 @@ +//===--===// +// +// The LLVM Compiler Infrastructure +// +// This file is dual licensed under the MIT and the University of Illinois Open +// Source Licenses. See LICENSE.TXT for details. +// +//===--===// + +// UNSUPPORTED: c++98, c++03, c++11 + +// +// +// template > +// class ostream_joiner; +// +// ostream_joiner & operator*() noexcept +// returns *this; + +#include +#include +#include + +#include "test_macros.h" + +namespace exp = std::experimental; + +template +void test ( exp::ostream_joiner &oj ) { +static_assert((noexcept(*oj)), "" ); +exp::ostream_joiner &ret = *oj; +assert( &ret == &oj ); +} + +int main () { + +{ exp::ostream_joiner oj(std::cout, '8'); test(oj); } +{ exp::ostream_joiner oj(std::cout, std::string("9"));test(oj); } +{ exp::ostream_joiner oj(std::cout, std::wstring(L"10")); test(oj); } +{ exp::ostream_joiner oj(std::cout, 11); test(oj); } + +{ exp::ostream_joiner oj(std::wcout, '8'); test(oj); } +{ exp::ostream_joiner oj(std::wcout, std::string("9"));test(oj); } +{ exp::ostream_joiner oj(std::wcout, std::wstring(L"10")); test(oj); } +{ exp::ostream_joiner oj(std::wcout, 11); test(oj); } +} Index: test/std/experimental/iterator/ostream.joiner/ostream.joiner.ops/ostream_joiner.op.pretincrement.pass.cpp === --- test/std/experimental/iterator/ostream.joiner/ostream.joiner.ops/ostream_joiner.op.pretincrement.pass.cpp +++ test/std/experimental/iterator/ostream.joiner/ostream.joiner.ops/ostream_joiner.op.pretincrement.pass.cpp @@ -0,0 +1,46 @@ +//===--===// +// +// The LLVM Compiler Infrastructure +// +// This file is dual licensed under the MIT and the University of Illinois Open +// Source Licenses. See LICENSE.TXT for details. +// +//===--===// + +// UNSUPPORTED: c++98, c++03, c++11 + +// +// +// template > +// class ostream_joiner; +// +// ostream_joiner & operator++() noexcept +// returns *this; + +#include +#include +#include + +#include "test_macros.h" + +namespace exp = std::experimental; + +template +void test ( exp::ostream_joiner &oj ) { +static_assert((noexcept(++oj)), "" ); +exp::ostream_joiner &ret = ++oj; +assert( &ret == &oj ); +} + +int main () { + +{ exp::ostream_joiner oj(std::cout, '8'); test(oj); } +{ exp::ostream_joiner oj(std::cout, std::string("9"));test(oj); } +{ exp::ostream_joiner oj(std::cout, std::wstring(L"10")); test(oj); } +{ exp::ostream_joiner oj(std::cout, 11); test(oj); } + +{ exp::ostream_joiner oj(std::wcout, '8'); test(oj); } +{ exp::ostream_joiner oj(std::wcout, std::string("9"));test(oj); } +{ exp::ostream_joiner oj(std::wcout, std::wstring(L"10")); test(oj); } +{ exp::ostream_joiner oj(std::wcout, 11); test(oj); } +} Index: test/std/experimental/iterator/ostream.joiner/ostream.joiner.ops/ostream_joiner.op.postincrement.pass.cpp === --- test/std/experimental/iterator/ostream.joiner/ostream.joiner.ops/ostream_joiner.op.postincrement.pass.cpp +++ test/std/experimental/iterator/ostream.joiner/ostream.joiner.ops/ostream_joiner.op.postincrement.pass.cpp @@ -0,0
Re: [PATCH] D16605: Implement `std::experimental::ostream_joiner`
mclow.lists marked 4 inline comments as done. mclow.lists added a comment. http://reviews.llvm.org/D16605 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16605: Implement `std::experimental::ostream_joiner`
EricWF added a comment. One last thing. The spec implicitly defines the copy/move/assignment operators for "ostream_joiner" by making "_Delim" an exposition only member. Essentially "ostream_joiner" should only copy/moveable if "_Delim" is. I think we should add tests that ensure we actually follow this behavior. Comment at: test/std/experimental/iterator/nothing_to_do.pass.cpp:2 @@ +1,3 @@ +//===--===// +// +// The LLVM Compiler Infrastructure Maybe call it "include_header.pass.cpp" then. http://reviews.llvm.org/D16605 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r259011 - [Sema] Make extended vectors of `bool` an error.
Author: gbiv Date: Wed Jan 27 19:38:18 2016 New Revision: 259011 URL: http://llvm.org/viewvc/llvm-project?rev=259011&view=rev Log: [Sema] Make extended vectors of `bool` an error. In OpenCL, `bool` vectors are a reserved type, and are therefore illegal. Outside of OpenCL, if we try to make an extended vector of N `bool`s, Clang will lower it to an `[N x i1]`. LLVM has no ABI for bitvectors, so lots of operations on such vectors are thoroughly broken. As a result, this patch makes them illegal in everything else, as well. :) Differential Revision: http://reviews.llvm.org/D15721 Added: cfe/trunk/test/SemaOpenCL/bool-vectors.cl Modified: cfe/trunk/lib/Sema/SemaType.cpp cfe/trunk/test/CodeGen/convertvector.c cfe/trunk/test/Sema/ext_vector_casts.c Modified: cfe/trunk/lib/Sema/SemaType.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=259011&r1=259010&r2=259011&view=diff == --- cfe/trunk/lib/Sema/SemaType.cpp (original) +++ cfe/trunk/lib/Sema/SemaType.cpp Wed Jan 27 19:38:18 2016 @@ -2183,10 +2183,16 @@ QualType Sema::BuildArrayType(QualType T /// Run the required checks for the extended vector type. QualType Sema::BuildExtVectorType(QualType T, Expr *ArraySize, SourceLocation AttrLoc) { - // unlike gcc's vector_size attribute, we do not allow vectors to be defined + // Unlike gcc's vector_size attribute, we do not allow vectors to be defined // in conjunction with complex types (pointers, arrays, functions, etc.). - if (!T->isDependentType() && - !T->isIntegerType() && !T->isRealFloatingType()) { + // + // Additionally, OpenCL prohibits vectors of booleans (they're considered a + // reserved data type under OpenCL v2.0 s6.1.4), we don't support selects + // on bitvectors, and we have no well-defined ABI for bitvectors, so vectors + // of bool aren't allowed. + if ((!T->isDependentType() && !T->isIntegerType() && + !T->isRealFloatingType()) || + T->isBooleanType()) { Diag(AttrLoc, diag::err_attribute_invalid_vector_type) << T; return QualType(); } @@ -2200,7 +2206,7 @@ QualType Sema::BuildExtVectorType(QualTy return QualType(); } -// unlike gcc's vector_size attribute, the size is specified as the +// Unlike gcc's vector_size attribute, the size is specified as the // number of elements, not the number of bytes. unsigned vectorSize = static_cast(vecSize.getZExtValue()); Modified: cfe/trunk/test/CodeGen/convertvector.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/convertvector.c?rev=259011&r1=259010&r2=259011&view=diff == --- cfe/trunk/test/CodeGen/convertvector.c (original) +++ cfe/trunk/test/CodeGen/convertvector.c Wed Jan 27 19:38:18 2016 @@ -9,14 +9,6 @@ typedef unsigned long vector8ulong _ typedef unsigned short vector8ushort __attribute__((__vector_size__(16))); #ifdef __cplusplus -#define BOOL bool -#else -#define BOOL _Bool -#endif - -typedef BOOL vector8bool __attribute__((__ext_vector_type__(8))); - -#ifdef __cplusplus extern "C" { #endif @@ -32,13 +24,6 @@ vector8double flt_ext(vector8float x) { // CHECK: fpext <8 x float> %{{[^ ]}} to <8 x double> } -vector8bool flt_tobool(vector8float x) { - return __builtin_convertvector(x, vector8bool); - // CHECK-LABEL: @flt_tobool - // CHECK-NOT: fptoui <8 x float> %{{[^ ]}} to <8 x i1> - // CHECK: fcmp une <8 x float> %{{[^ ]}}, zeroinitializer -} - vector8long flt_tosi(vector8float x) { return __builtin_convertvector(x, vector8long); // CHECK-LABEL: @flt_tosi @@ -69,13 +54,6 @@ vector8long int_sext(vector8short x) { // CHECK: sext <8 x i16> %{{[^ ]}} to <8 x i64> } -vector8bool int_tobool(vector8short x) { - return __builtin_convertvector(x, vector8bool); - // CHECK-LABEL: @int_tobool - // CHECK-NOT: trunc <8 x i16> %{{[^ ]}} to <8 x i1> - // CHECK: icmp ne <8 x i16> %{{[^ ]}}, zeroinitializer -} - vector8float int_tofp(vector8short x) { return __builtin_convertvector(x, vector8float); // CHECK-LABEL: @int_tofp Modified: cfe/trunk/test/Sema/ext_vector_casts.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/ext_vector_casts.c?rev=259011&r1=259010&r2=259011&view=diff == --- cfe/trunk/test/Sema/ext_vector_casts.c (original) +++ cfe/trunk/test/Sema/ext_vector_casts.c Wed Jan 27 19:38:18 2016 @@ -1,5 +1,7 @@ // RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 -fsyntax-only -verify -fno-lax-vector-conversions -Wconversion %s +typedef __attribute__((ext_vector_type(8))) _Bool BoolVector; // expected-error {{invalid vector element type '_Bool'}} + typedef __attribute__(( ext_vector_type(2) )) float float2; typedef __attribute__(( ext_vector_type(3) )) float float3; typedef __attribute__(( ext
Re: [PATCH] D15721: [Sema] Fix ICE on casting a vector of bools to a vector of T
This revision was automatically updated to reflect the committed changes. Closed by commit rL259011: [Sema] Make extended vectors of `bool` an error. (authored by gbiv). Changed prior to commit: http://reviews.llvm.org/D15721?vs=46088&id=46205#toc Repository: rL LLVM http://reviews.llvm.org/D15721 Files: cfe/trunk/lib/Sema/SemaType.cpp cfe/trunk/test/CodeGen/convertvector.c cfe/trunk/test/Sema/ext_vector_casts.c cfe/trunk/test/SemaOpenCL/bool-vectors.cl Index: cfe/trunk/test/CodeGen/convertvector.c === --- cfe/trunk/test/CodeGen/convertvector.c +++ cfe/trunk/test/CodeGen/convertvector.c @@ -9,14 +9,6 @@ typedef unsigned short vector8ushort __attribute__((__vector_size__(16))); #ifdef __cplusplus -#define BOOL bool -#else -#define BOOL _Bool -#endif - -typedef BOOL vector8bool __attribute__((__ext_vector_type__(8))); - -#ifdef __cplusplus extern "C" { #endif @@ -32,13 +24,6 @@ // CHECK: fpext <8 x float> %{{[^ ]}} to <8 x double> } -vector8bool flt_tobool(vector8float x) { - return __builtin_convertvector(x, vector8bool); - // CHECK-LABEL: @flt_tobool - // CHECK-NOT: fptoui <8 x float> %{{[^ ]}} to <8 x i1> - // CHECK: fcmp une <8 x float> %{{[^ ]}}, zeroinitializer -} - vector8long flt_tosi(vector8float x) { return __builtin_convertvector(x, vector8long); // CHECK-LABEL: @flt_tosi @@ -69,13 +54,6 @@ // CHECK: sext <8 x i16> %{{[^ ]}} to <8 x i64> } -vector8bool int_tobool(vector8short x) { - return __builtin_convertvector(x, vector8bool); - // CHECK-LABEL: @int_tobool - // CHECK-NOT: trunc <8 x i16> %{{[^ ]}} to <8 x i1> - // CHECK: icmp ne <8 x i16> %{{[^ ]}}, zeroinitializer -} - vector8float int_tofp(vector8short x) { return __builtin_convertvector(x, vector8float); // CHECK-LABEL: @int_tofp Index: cfe/trunk/test/SemaOpenCL/bool-vectors.cl === --- cfe/trunk/test/SemaOpenCL/bool-vectors.cl +++ cfe/trunk/test/SemaOpenCL/bool-vectors.cl @@ -0,0 +1,3 @@ +// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only + +typedef __attribute__((ext_vector_type(16))) _Bool bool8; // expected-error{{invalid vector element type 'bool'}} Index: cfe/trunk/test/Sema/ext_vector_casts.c === --- cfe/trunk/test/Sema/ext_vector_casts.c +++ cfe/trunk/test/Sema/ext_vector_casts.c @@ -1,5 +1,7 @@ // RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 -fsyntax-only -verify -fno-lax-vector-conversions -Wconversion %s +typedef __attribute__((ext_vector_type(8))) _Bool BoolVector; // expected-error {{invalid vector element type '_Bool'}} + typedef __attribute__(( ext_vector_type(2) )) float float2; typedef __attribute__(( ext_vector_type(3) )) float float3; typedef __attribute__(( ext_vector_type(4) )) int int4; Index: cfe/trunk/lib/Sema/SemaType.cpp === --- cfe/trunk/lib/Sema/SemaType.cpp +++ cfe/trunk/lib/Sema/SemaType.cpp @@ -2183,10 +2183,16 @@ /// Run the required checks for the extended vector type. QualType Sema::BuildExtVectorType(QualType T, Expr *ArraySize, SourceLocation AttrLoc) { - // unlike gcc's vector_size attribute, we do not allow vectors to be defined + // Unlike gcc's vector_size attribute, we do not allow vectors to be defined // in conjunction with complex types (pointers, arrays, functions, etc.). - if (!T->isDependentType() && - !T->isIntegerType() && !T->isRealFloatingType()) { + // + // Additionally, OpenCL prohibits vectors of booleans (they're considered a + // reserved data type under OpenCL v2.0 s6.1.4), we don't support selects + // on bitvectors, and we have no well-defined ABI for bitvectors, so vectors + // of bool aren't allowed. + if ((!T->isDependentType() && !T->isIntegerType() && + !T->isRealFloatingType()) || + T->isBooleanType()) { Diag(AttrLoc, diag::err_attribute_invalid_vector_type) << T; return QualType(); } @@ -2200,7 +2206,7 @@ return QualType(); } -// unlike gcc's vector_size attribute, the size is specified as the +// Unlike gcc's vector_size attribute, the size is specified as the // number of elements, not the number of bytes. unsigned vectorSize = static_cast(vecSize.getZExtValue()); Index: cfe/trunk/test/CodeGen/convertvector.c === --- cfe/trunk/test/CodeGen/convertvector.c +++ cfe/trunk/test/CodeGen/convertvector.c @@ -9,14 +9,6 @@ typedef unsigned short vector8ushort __attribute__((__vector_size__(16))); #ifdef __cplusplus -#define BOOL bool -#else -#define BOOL _Bool -#endif - -typedef BOOL vector8bool __attribute__((__ext_vector_type__(8))); - -#ifdef __cplusplus extern "C" { #endif @@ -32,13 +24,6 @@ // CHECK: fpext <8 x float> %{{[^ ]}} to <8 x double> } -vector8bool flt
Re: [PATCH] D16529: [clang-tidy] Add modernize-raw-string-literal check
alexfh added inline comments. Comment at: clang-tidy/modernize/RawStringLiteralCheck.cpp:27 @@ +26,3 @@ + // we only transform ASCII string literals + if (!Literal->isAscii()) +return false; LegalizeAdulthood wrote: > alexfh wrote: > > Why can't the check convert non-ascii string literals to corresponding raw > > string literals? > Just doing one thing at a time and not trying to create a "kitchen sink" > check on the first pass. It should be rather straightforward to implement handling of the other types of string literals, but if you still want to postpone this, then a TODO would be more useful than the comment that just states that "we only transform ASCII string literals" (without explaining why). Comment at: clang-tidy/modernize/RawStringLiteralCheck.cpp:57 @@ +56,3 @@ + + // already a raw string literal if R comes before " + if (Text.find_first_of("R") < Text.find_first_of(R"(")")) aaron.ballman wrote: > LegalizeAdulthood wrote: > > alexfh wrote: > > > nit: Capitalization, punctuation. Same for other comments in the file. > > I don't understand what exactly you are asking me to change. There is > > something wrong with the comment? > ` // already a raw string literal if R comes before "` > > Should be: > > ` // Already a raw string literal if R comes before ".` Sorry for the brevity. Aaron explained what I meant and here's the motivation (http://llvm.org/docs/CodingStandards.html#commenting): > When writing comments, write them as English prose, which means they should > use proper capitalization, punctuation, etc. Comment at: clang-tidy/modernize/RawStringLiteralCheck.cpp:77 @@ +76,3 @@ + std::string Delimiter; + for (int Counter = 0; containsDelimiter(Bytes, Delimiter); ++Counter) { +if (Counter == 0) { LegalizeAdulthood wrote: > alexfh wrote: > > Why don't you try an empty delimiter? It will cover a majority of cases. > > > > Also, even though the function is used only when generating fix-it hints, > > it still should be more effective than it is now. Most of the heap > > allocations and copies caused by the usage of std::ostringstream, > > std::string and std::string::operator+ can be avoided, e.g. like this: > > > > static const StringRef Delimiters[2][] = > > {{"", "R\"(", ")\""}, {"lit", "R\"lit(", ")lit\""}, {"str", "R\"str(", > > ")str\""}, > > /* add more different ones to make it unlikely to meet all of these in > > a single literal in the wild */}; > > for (const auto &Delim : Delimiters) { > > if (Bytes.find(Delim[2]) != StringRef::npos) > > return (Delim[1] + Bytes + Delim[2]).str(); > > } > > // Give up gracefully on literals having all our delimiters. > The very first thing tried is an empty delimiter. > > I coded a solution to the problem that always works. > > I find it less understandable to try a bunch of random delimiters and then > fail if they are all present in the string. Then the whole algorithm becomes > more complicated because even though I **could** construct a fixit, I'm > failing stupidly because all my "favorite" delimiters were used in your > string. > > Sometimes I feel these reviews over-obsess with allocations and efficiency > instead of implementing a simple general algorithm that works reliably. > > We don't even have any measurements to show that this performance > consideration is dominant or even noticeable, but I'm being asked to take a > perfectly correct algorithm and cram it into a StringRef corset. > The very first thing tried is an empty delimiter. Ah, sorry, this wasn't clear to me at first. > I coded a solution to the problem that always works. We first need to agree on whether we're trying to create a solution that works for hypothetical worst-case that has no chances to appear outside of the tests for your check, or we're trying to target real code. The solution I suggest is more than enough for real code. For example, I've just searched for the `\)[a-z]\\\"` regex (a sequence that would prevent a single-letter delimiter) in a huge codebase and there were only ~20 hits (roughly half of them in tests for handling raw string literals in clang-format and other C++ tools). Needless to say that `)lit\"` wasn't found, nor `)str\"`. > I find it less understandable to try a bunch of random delimiters and then > fail if they are all present in the string. Then the whole algorithm becomes > more complicated because even though I could construct a fixit, I'm failing > stupidly because all my "favorite" delimiters were used in your string. Good news is that this code will "fail" (and it should fail gracefully, e.g. just skip the fixit) extremely infrequently (see above). And by having a list of "favorite" delimiters we can make the resulting code look better in the (extremely rare) case when the version you suggest would have to use `)lit0"`, for example. > Sometim
Re: [clang-tools-extra] r258926 - [clang-tidy] Fix documentation.
Thanks! On Wed, Jan 27, 2016 at 7:30 PM, Hans Wennborg wrote: > On Wed, Jan 27, 2016 at 3:37 AM, Alexander Kornienko via cfe-commits > wrote: > > Author: alexfh > > Date: Wed Jan 27 05:37:19 2016 > > New Revision: 258926 > > > > URL: http://llvm.org/viewvc/llvm-project?rev=258926&view=rev > > Log: > > [clang-tidy] Fix documentation. > > > > Fixed broken links to cppcoreguidelines (anchors specified in the .md > file > > should be used, not automatic anchors generated by github). > > > > Fixed formatting, array_view -> span, fixed sphinx errors in > > misc-definitions-in-headers.rst. > > Merged to 3.8 in r258959. > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits