This is an archive of the discontinued LLVM Phabricator instance.

[ASTImporter] Separate unittest files

Authored by martong on May 10 2019, 8:02 AM.

Diff Detail

rC Clang

Event Timeline

martong created this revision.May 10 2019, 8:02 AM
Herald added a reviewer: shafik. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript
a_sidorin accepted this revision.May 11 2019, 10:43 AM

Hi Gabor,
I like this change! LGTM, just a few nits.

20 ↗(On Diff #199020)

Is this FIXME obsolete now?

37 ↗(On Diff #199020)

Can we use StringRef (or, at least const auto *)?

49 ↗(On Diff #199020)

Double spaces in the end of sentences look a bit strange.

139 ↗(On Diff #199020)

Optional: F0/F1 (where F stands for "function", I guess) can be replaced with D0/D1 (for decl) since the code is generic.

157 ↗(On Diff #199020)

This should fit a single line.

190 ↗(On Diff #199020)


This revision is now accepted and ready to land.May 11 2019, 10:43 AM
martong marked 8 inline comments as done.May 13 2019, 2:46 AM

Thanks for the review Alexei!

20 ↗(On Diff #199020)

Yes, that's correct, thank you for finding it!

49 ↗(On Diff #199020)

Ok I removed them. (Note, this is the result of some autoformatting option in vim, could not discover which specific setting causes the insertion of double spaces.)

martong updated this revision to Diff 199225.May 13 2019, 2:47 AM
martong marked 2 inline comments as done.
  • Address Alexei's comments
This revision was automatically updated to reflect the committed changes.