Page MenuHomePhabricator

[Symbol] Change ClangASTContext::GetCXXClassName return type
ClosedPublic

Authored by xiaobai on Wed, Oct 30, 1:57 PM.

Details

Summary

Instead of filling out a std::string and returning a bool to indicate
success, returning a std::string directly and testing to see if it's
empty seems like a cleaner solution overall.

Diff Detail

Event Timeline

xiaobai created this revision.Wed, Oct 30, 1:57 PM
Herald added a project: Restricted Project. · View Herald TranscriptWed, Oct 30, 1:57 PM

Could this return an Optional<std::string> or Expected<std::string>? It's not clear from a std::string return value that the method can fail and it will return an empty string on error.

Could this return an Optional<std::string> or Expected<std::string>? It's not clear from a std::string return value that the method can fail and it will return an empty string on error.

That sounds like a better idea to me. Will do that instead.

xiaobai updated this revision to Diff 227188.Wed, Oct 30, 3:21 PM

Use llvm::Optional

teemperor accepted this revision.Wed, Oct 30, 3:42 PM

LGTM, thanks!

This revision is now accepted and ready to land.Wed, Oct 30, 3:42 PM
JDevlieghere accepted this revision.Wed, Oct 30, 4:05 PM
labath accepted this revision.Thu, Oct 31, 1:28 AM

awesome

This revision was automatically updated to reflect the committed changes.