Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[AST] explicit PrintOptions default constructor #18142

Merged
merged 1 commit into from
Aug 23, 2018

Conversation

omochi
Copy link
Collaborator

@omochi omochi commented Jul 23, 2018

This PR add default constructor of PrintOptions explicitly.
This is already implemented by c++ compiler implicitly.
But, it can not be called from lldb session.

(lldb) p f->getLoweredFunctionType()->getString(PrintOptions())
error: call to implicitly-deleted default constructor of 'swift::PrintOptions'
default constructor of 'PrintOptions' is implicitly deleted because field 'BracketOptions' has no default constructor

If I add this explicitly, it become available.

(lldb) p f->getLoweredFunctionType()->getString(PrintOptions())
(std::__1::string) $1 = "@convention(c) (Int32, UnsafeMutablePointer<Optional<UnsafeMutablePointer<Int8>>>) -> Int32"

This is very useful for reading and studying swift compiler source code with debugger.

supplement

If I add default constructor of BracketOptions to follow error message,
It still can not be called because of another error.

(lldb) p PrintOptions()
error: Couldn't lookup symbols:
  __ZNSt3__18functionIFNS_12basic_stringIcNS_11char_traitsIcEENS_9allocatorIcEEEEPKN5swift9ValueDeclEEEC1Ev
  __ZNSt3__18functionIFbPKN5swift13ExtensionDeclEEEC1Ev
  __ZNSt3__16vectorIN5swift11AnyAttrKindENS_9allocatorIS2_EEEC1Ev
  __ZNSt3__110shared_ptrIN5swift18ShouldPrintCheckerEEC1Ev

lldb seems can not call templated symbol which has not been instantiated.

@jrose-apple
Copy link
Contributor

jrose-apple commented Jul 23, 2018

This looks like an LLDB bug. There should be no circumstances where Clang can synthesize a default constructor but LLDB can't (other than the template instantiation thing you mentioned).

We generally use the dump() methods for this purpose, and make sure one of those can always be called from the debugger. Is that not sufficient for what you're doing?

@dcci
Copy link
Member

dcci commented Jul 23, 2018

cc @fredriss / @shafik

@fredriss
Copy link
Contributor

It's a know issue with current debuggers (not only LLDB). Templates are not described in the debug information, only template instantiations, thus the debugger doesn't know about anything that hasn't been instantiated. Even if it's been instantiated but all the instances have been inlined, the debugger won't be able to access it.

@jrose-apple
Copy link
Contributor

Sorry, we were asking about the default constructor part, not the template part.

@fredriss
Copy link
Contributor

Oh, I see. Sounds more like a Clang bug than and LLDB one. I suppose what happens is that Clang doesn't emit the debug info describing implicitly generated constructors?

@jrose-apple
Copy link
Contributor

If I'm understanding correctly, this is a case where the constructor wasn't generated, but it should be possible for LLDB to generate it.

@fredriss
Copy link
Contributor

Currently LLDB never generates methods/*tors, it just calls them and they have to be present in the generated image. We are playing with the idea of generating what's not there, but it is quite tricky.

If the error message about implicitly deleting the constructor is correct, lldb would not have been able to generate it anyway. If the error message is incorrect, then it's worth an LLDB bug report.

@omochi
Copy link
Collaborator Author

omochi commented Jul 24, 2018

Thanks to response.

I know about dump().
But they emit outputs to stdout of process.
I often debug swiftc by xcode with Wait for executable to be launched.
(Because terminal is easier than xcode's scheme dialog)
So there are 2 window which are terminal with $ swiftc and xcode.
So I use (lldb) expr type->dump() in xcode, then I need to switch terminal to read result.
Its inconvenience.
If I get result by value in lldb session, its useful.

Anyway I understand about lldb behavior and thanks to tell details.

I will investigate about using dump(raw_osteam) overload to solve my case.

@omochi
Copy link
Collaborator Author

omochi commented Aug 10, 2018

@jrose-apple Hello.
I am working about this issue now.
https://bugs.swift.org/browse/SR-8499

Type::getString(PrintOptions) is important about this.

If Type::dump() prints escaping,
Type::getString() does not print @escaping sometime.

Former see resolved type information.
Latter see ParameterTypeFlags in AnyFunctionType::Param.
They are different sometime.

I am diving into ConstraintSystem, so its very complex.
I want to see temporary status about Type::getString.

It is a problem that default constructor value of PrintOptions can not be created from lldb.
I can use other value such like PrintOptions::printVerbose(),
But It may cause another behavior.

@jrose-apple
Copy link
Contributor

All right. Please add a comment that says "This is explicit to guarantee that it can be called from LLDB", or something similar.

@omochi
Copy link
Collaborator Author

omochi commented Aug 10, 2018

Thanks! I will do later.

@omochi
Copy link
Collaborator Author

omochi commented Aug 23, 2018

@jrose-apple Please review it.

Copy link
Member

@dcci dcci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume you tried using PrintOptions() = default and it doesn't work? Otherwise, lgtm.

@dcci
Copy link
Member

dcci commented Aug 23, 2018

@swift-ci please test

@swift-ci
Copy link
Collaborator

Build failed
Swift Test OS X Platform
Git Sha - 42ed1fdb9cded4882732d87f5e16d6ed7924925f

@swift-ci
Copy link
Collaborator

Build failed
Swift Test Linux Platform
Git Sha - 42ed1fdb9cded4882732d87f5e16d6ed7924925f

@jrose-apple jrose-apple merged commit c0114ea into apple:master Aug 23, 2018
@omochi omochi deleted the print-options-default-ctor branch February 16, 2020 11:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants