Page MenuHomePhabricator

[API] Remove use of ClangASTContext from SBTarget
ClosedPublic

Authored by xiaobai on Jul 18 2019, 5:11 PM.

Details

Summary

The methods to find types in a Target aren't clang specific and are
pretty generalizable to type systems. Additionally, to support some of
the use cases in SBTarget, I've added a "GetScratchTypeSystems" method
to Target to support getting all type systems for a target we are
debugging.

Diff Detail

Repository
rL LLVM

Event Timeline

xiaobai created this revision.Jul 18 2019, 5:11 PM
jingham accepted this revision.Jul 18 2019, 5:26 PM

This looks fine to me. Makes it really clear that we need SBTarget::FindFirstTypeForLanguage, etc. But FindFirstType was always a crapshoot anyway...

This revision is now accepted and ready to land.Jul 18 2019, 5:26 PM
JDevlieghere requested changes to this revision.Jul 18 2019, 10:04 PM

All uses of this new function drop the error on the ground. Does that mean it doesn't matter? If it does, should we return an expected instead? Should we stop on the first error, or is it fine to overwrite when iterating over languages_for_expressions? It seems like the error handling needs some more work here.

This revision now requires changes to proceed.Jul 18 2019, 10:04 PM

All uses of this new function drop the error on the ground. Does that mean it doesn't matter? If it does, should we return an expected instead? Should we stop on the first error, or is it fine to overwrite when iterating over languages_for_expressions? It seems like the error handling needs some more work here.

If you at the actual function GetScratchTypeSystemForLanguage, the error is completely unused. I think dropping the Status returning an llvm::Expected is probably the right thing to do. In that case, I don't think we should stop if GetScratchTypeSystemForLanguage returns a None value. I'll update this tomorrow.

xiaobai updated this revision to Diff 212482.Tue, Jul 30, 5:49 PM

Update GetScatchTypeSystems to account for changes to TypeSystem usage

xiaobai updated this revision to Diff 212483.Tue, Jul 30, 5:51 PM

Removed argument to GetScratchTypeSystems from SBTarget

This revision is now accepted and ready to land.Tue, Jul 30, 5:57 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptWed, Jul 31, 1:47 PM