Page MenuHomePhabricator

Support attribute used in member funcs of class templates

Authored by rafauler on Jan 18 2019, 12:23 PM.

Diff Detail

Event Timeline

rafauler created this revision.Jan 18 2019, 12:23 PM
rafauler updated this revision to Diff 182581.Jan 18 2019, 12:24 PM

Removing facebook tags

Thanks for looking into my almost 6 year old bug!


Could you mention PR17480 in this test file as well?


Any reason to use the preprocessor here instead of __attribute__ directly?


CHECK: instead of CHECK-DAG should be sufficient.

rafauler updated this revision to Diff 183657.Jan 25 2019, 4:34 PM

No problem, thanks for your suggestions!

aaron.ballman added a subscriber: aaron.ballman.
aaron.ballman added inline comments.

const auto *?


const MemberSpecializationInfo *?


const auto *?


You can elide the last argument.


Also, can you bump this comment down below the RUN lines?

rafauler updated this revision to Diff 184029.Jan 29 2019, 12:39 AM

Rebase + Aaron's suggestions. Thanks for the suggestions!

aaron.ballman accepted this revision.Jan 30 2019, 5:17 AM

LGTM aside from some formatting nits.


Elide braces


Elide braces


I'd go ahead and leave this on the same line as the previous RUN line. We're often less picky about 80-col limits for RUN lines unless they're obnoxiously long. You can drop the -O0 from the RUN line as well (I believe).


You should run the patch through clang-format -- the indentation looks incorrect here.

This revision is now accepted and ready to land.Jan 30 2019, 5:17 AM
rafauler updated this revision to Diff 184467.Jan 31 2019, 12:54 AM

Thanks for reviewing!

This revision was automatically updated to reflect the committed changes.

This change broke building Swift on my Linux box. The compiler confusingly complains about the *same* template being both explicitly specialized and implicitly instantiated:

/home/dave/s/u/swift/stdlib/public/runtime/ProtocolConformance.cpp:34:38: error: explicit specialization of 'dump' after instantiation
template <> void ProtocolDescriptor::dump() const {
/home/dave/s/u/swift/stdlib/public/runtime/ProtocolConformance.cpp:34:18: note: implicit instantiation first required here
template <> void ProtocolDescriptor::dump() const {

Is this expected?

Hi davezarzycki, thanks for reporting this! This is not expected, I'll revert this diff for now. Sorry for this.

phosek added a subscriber: phosek.Mar 8 2019, 11:47 AM

This broke the 2-stage LTO build of LLVM, the error is:

ld.lld: /usr/local/google/home/phosek/clang-llvm/llvm-project/llvm/lib/IR/Constants.cpp:995: static llvm::Constant *llvm::ConstantArray::getImpl(llvm::ArrayType *, ArrayRef<llvm::Constant *>): Assertion `V[i]->getType() == Ty->getElementType() && "Wrong type in array element initializer"' failed.

The invocation this came from is:

/usr/local/google/home/phosek/clang-llvm/llvm-project/llvm-build/fuchsia-xyz/./bin/clang++  -fPIC -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -std=c++11 -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wstring-conversion -fdiagnostics-color -ffunction-sections -fdata-sections -flto -O3 -gline-tables-only  -ldl -lpthread -fuse-ld=lld -Wl,--color-diagnostics -Wl,-allow-shlib-undefined    -Wl,-O3 -Wl,--gc-sections tools/llvm-cov/CMakeFiles/llvm-cov.dir/llvm-cov.cpp.o tools/llvm-cov/CMakeFiles/llvm-cov.dir/gcov.cpp.o tools/llvm-cov/CMakeFiles/llvm-cov.dir/CodeCoverage.cpp.o tools/llvm-cov/CMakeFiles/llvm-cov.dir/CoverageExporterJson.cpp.o tools/llvm-cov/CMakeFiles/llvm-cov.dir/CoverageExporterLcov.cpp.o tools/llvm-cov/CMakeFiles/llvm-cov.dir/CoverageFilters.cpp.o tools/llvm-cov/CMakeFiles/llvm-cov.dir/CoverageReport.cpp.o tools/llvm-cov/CMakeFiles/llvm-cov.dir/CoverageSummaryInfo.cpp.o tools/llvm-cov/CMakeFiles/llvm-cov.dir/SourceCoverageView.cpp.o tools/llvm-cov/CMakeFiles/llvm-cov.dir/SourceCoverageViewHTML.cpp.o tools/llvm-cov/CMakeFiles/llvm-cov.dir/SourceCoverageViewText.cpp.o tools/llvm-cov/CMakeFiles/llvm-cov.dir/TestingSupport.cpp.o  -o bin/llvm-cov  -Wl,-rpath,"\$ORIGIN/../lib" lib/libLLVMCore.a lib/libLLVMSupport.a lib/libLLVMObject.a lib/libLLVMCoverage.a lib/libLLVMProfileData.a lib/libLLVMObject.a lib/libLLVMBitReader.a lib/libLLVMMCParser.a lib/libLLVMMC.a lib/libLLVMDebugInfoCodeView.a lib/libLLVMDebugInfoMSF.a lib/libLLVMCore.a lib/libLLVMBinaryFormat.a lib/libLLVMSupport.a -lz -lrt -ldl -lm lib/libLLVMDemangle.a
Herald added a project: Restricted Project. · View Herald TranscriptMar 8 2019, 11:47 AM