This is an archive of the discontinued LLVM Phabricator instance.

Fix an assertion failure in FormatASTNodeDiagnosticArgument.
ClosedPublic

Authored by alexfh on May 15 2017, 11:28 AM.

Details

Summary

The test being added in this patch used to cause an assertion failure:

/build/./bin/clang -cc1 -internal-isystem /build/lib/clang/5.0.0/include -nostdsysteminc -verify -fsyntax-only -std=c++11 -Wshadow-all /src/tools/clang/test/SemaCXX/warn-shadow.cpp

Exit Code: 134

Command Output (stderr):

clang: /src/tools/clang/lib/AST/ASTDiagnostic.cpp:424: void clang::FormatASTNodeDiagnosticArgument(DiagnosticsEngine::ArgumentKind, intptr_t, llvm::StringRef, llvm::StringRef, ArrayRef<DiagnosticsEngine::ArgumentValue>, SmallVectorImpl<char> &, void *, ArrayRef<intptr_t>): Assertion `isa<NamedDecl>(DC) && "Expected a NamedDecl"' failed.
#0 0x0000000001c7a1b4 PrintStackTraceSignalHandler(void*) (/build/./bin/clang+0x1c7a1b4)
#1 0x0000000001c7a4e6 SignalHandler(int) (/build/./bin/clang+0x1c7a4e6)
#2 0x00007f30880078d0 __restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0xf8d0)
#3 0x00007f3087054067 gsignal (/lib/x86_64-linux-gnu/libc.so.6+0x35067)
#4 0x00007f3087055448 abort (/lib/x86_64-linux-gnu/libc.so.6+0x36448)
#5 0x00007f308704d266 (/lib/x86_64-linux-gnu/libc.so.6+0x2e266)
#6 0x00007f308704d312 (/lib/x86_64-linux-gnu/libc.so.6+0x2e312)
#7 0x00000000035b7f22 clang::FormatASTNodeDiagnosticArgument(clang::DiagnosticsEngine::ArgumentKind, long, llvm::StringRef, llvm::StringRef, llvm::ArrayRef<std::pair<clang::DiagnosticsEngine::ArgumentKind, long> >, llvm::SmallVectorImpl<char>&, void*, llvm::ArrayRef<long>) (/build/
./bin/clang+0x35b7f22)
#8 0x0000000001ddbae4 clang::Diagnostic::FormatDiagnostic(char const*, char const*, llvm::SmallVectorImpl<char>&) const (/build/./bin/clang+0x1ddbae4)
#9 0x0000000001ddb323 clang::Diagnostic::FormatDiagnostic(char const*, char const*, llvm::SmallVectorImpl<char>&) const (/build/./bin/clang+0x1ddb323)
#10 0x00000000022878a4 clang::TextDiagnosticBuffer::HandleDiagnostic(clang::DiagnosticsEngine::Level, clang::Diagnostic const&) (/build/./bin/clang+0x22878a4)
#11 0x0000000001ddf387 clang::DiagnosticIDs::ProcessDiag(clang::DiagnosticsEngine&) const (/build/./bin/clang+0x1ddf387)
#12 0x0000000001dd9dea clang::DiagnosticsEngine::EmitCurrentDiagnostic(bool) (/build/./bin/clang+0x1dd9dea)
#13 0x0000000002cad00c clang::Sema::EmitCurrentDiagnostic(unsigned int) (/build/./bin/clang+0x2cad00c)
#14 0x0000000002d91cd2 clang::Sema::CheckShadow(clang::NamedDecl*, clang::NamedDecl*, clang::LookupResult const&) (/build/./bin/clang+0x2d91cd2)

Stack dump:
0. Program arguments: /build/./bin/clang -cc1 -internal-isystem /build/lib/clang/5.0.0/include -nostdsysteminc -verify -fsyntax-only -std=c++11 -Wshadow-all /src/tools/clang/test/SemaCXX/warn-shadow.cpp

  1. /src/tools/clang/test/SemaCXX/warn-shadow.cpp:214:23: current parser token ';'
  2. /src/tools/clang/test/SemaCXX/warn-shadow.cpp:213:26: parsing function body 'handleLinkageSpec'
  3. /src/tools/clang/test/SemaCXX/warn-shadow.cpp:213:26: in compound statement ('{}')

/build/tools/clang/test/SemaCXX/Output/warn-shadow.cpp.script: line 1: 15595 Aborted (core dumped) /build/./bin/clang -cc1 -internal-isystem /build/lib/clang/5.0.0/include -nostdsysteminc -verify -fsyntax-only -std=c++11 -Wshadow-all /src/tools/clang/test/SemaCXX/warn-shadow.cpp

Diff Detail

Repository
rL LLVM

Event Timeline

alexfh created this revision.May 15 2017, 11:28 AM

The patch is trivial, I'm mostly wondering about the wording and whether we want to expand the "linkage specification" to specify whether it is extern "C" or extern "C++", for example.

alexfh edited the summary of this revision. (Show Details)May 15 2017, 11:33 AM
rsmith added inline comments.May 15 2017, 1:02 PM
test/SemaCXX/warn-shadow.cpp
214 ↗(On Diff #99034)

We should be producing a diagnostic talking about the enclosing namespace / TU scope, not the linkage specification. Whoever is emitting the diagnostic seems to be missing a call to getRedeclContext() -- although perhaps we could do that in the diagnostic code instead.

alexfh updated this revision to Diff 99112.May 16 2017, 1:08 AM

Instead of handling LinkageSpecDecl, use getRedeclContext() when issuing -Wshadow diagnostic.

alexfh added inline comments.May 16 2017, 1:15 AM
test/SemaCXX/warn-shadow.cpp
214 ↗(On Diff #99034)

I tried skipping transparent declaration contexts in FormatASTNodeDiagnosticArgument, but this causes three tests fail due to skipped enumeration names:

error: 'error' diagnostics expected but not seen: 
  File /src/tools/clang/test/Modules/Inputs/odr/b.h Line 5 (directive at /src/tools/clang/test/Modules/odr.cpp:21): 'E::e2' from module 'b' is not present in definition of 'E' in module 'a'
error: 'error' diagnostics seen but not expected: 
  File /src/tools/clang/test/Modules/Inputs/odr/b.h Line 5: 'E::e2' from module 'b' is not present in definition of the global namespace in module 'a'

error: 'error' diagnostics expected but not seen: 
  File /src/tools/clang/test/SemaCXX/nested-name-spec.cpp Line 434: no member named 'X2' in 'PR16951::enumerator_2'
error: 'error' diagnostics seen but not expected: 
  File /src/tools/clang/test/SemaCXX/nested-name-spec.cpp Line 434: no member named 'X2' in namespace 'PR16951'

error: 'error' diagnostics expected but not seen: 
  File /src/tools/clang/test/SemaTemplate/instantiate-non-dependent-types.cpp Line 26 (directive at /src/tools/clang/test/SemaTemplate/instantiate-non-dependent-types.cpp:25): no member named '~Colors' in 'Colors'
error: 'error' diagnostics seen but not expected: 
  File /src/tools/clang/test/SemaTemplate/instantiate-non-dependent-types.cpp Line 26: no member named '~Colors' in the global namespace

This doesn't seem right, so I changed the code emitting -Wshadow to use getRedeclContext() instead. PTAL

alexfh marked an inline comment as done.May 16 2017, 1:16 AM
rsmith accepted this revision.May 17 2017, 5:44 PM

LGTM, thanks!

This revision is now accepted and ready to land.May 17 2017, 5:44 PM
This revision was automatically updated to reflect the committed changes.