Page MenuHomePhabricator

Support attribute used in member funcs of class templates
ClosedPublic

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!

test/CodeGenCXX/attr-used-member-function-implicit-instantiation.cpp
2

Could you mention PR17480 in this test file as well?

9

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

17

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.
lib/Sema/SemaTemplateInstantiateDecl.cpp
2182

const auto *?

2184

const MemberSpecializationInfo *?

2186

const auto *?

2191

You can elide the last argument.

test/CodeGenCXX/attr-used-member-function-implicit-instantiation.cpp
2

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.

lib/Sema/SemaTemplateInstantiateDecl.cpp
2185

Elide braces

2187

Elide braces

test/CodeGenCXX/attr-used-member-function-implicit-instantiation.cpp
3

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).

7

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