This is an archive of the discontinued LLVM Phabricator instance.

[flang][fir] Update flang test tool support classes.
ClosedPublic

Authored by schweitz on Feb 19 2021, 11:42 AM.

Details

Summary

This updates the various classes that support the compliation of Fortran. These classes are shared by the test tools.

Diff Detail

Event Timeline

schweitz created this revision.Feb 19 2021, 11:42 AM
schweitz requested review of this revision.Feb 19 2021, 11:42 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 19 2021, 11:42 AM
clementval accepted this revision.Feb 19 2021, 12:18 PM

LGTM. There are couple of clang-tidy warnings you might wants to go over before pushing.

flang/include/flang/Optimizer/Support/FIRContext.h
18

I thing we get this warning quite often. Is there a difference between header guard in Flang and the one clang-tidy expect?

26

Do you want to add // namespace mlir?

This revision is now accepted and ready to land.Feb 19 2021, 12:18 PM
schweitz added inline comments.Feb 19 2021, 12:25 PM
flang/include/flang/Optimizer/Support/FIRContext.h
18

Yes, it seems to complain about every header file in flang/include/.

It looks like clang-tidy should be fine with this identifier spelling.

Tracking it here: https://github.com/flang-compiler/f18-llvm-project/issues/631

26

Looks like a sharp edge between clang-format and clang-tidy. clang-format doesn't put it in if there is only 1 declaration. clang-tidy whines about it.

clementval added inline comments.Feb 19 2021, 12:49 PM
flang/include/flang/Optimizer/Support/FIRContext.h
18

Ok

26

Yeah I guess it makes sense since it is so close it's still readable without adding noise.

This revision was landed with ongoing or failed builds.Feb 19 2021, 4:03 PM
This revision was automatically updated to reflect the committed changes.