This is an archive of the discontinued LLVM Phabricator instance.

[flang][driver] Make --version and -version consistent with clang
ClosedPublic

Authored by ekieri on Mar 27 2022, 12:47 AM.

Details

Summary

This patch makes -version valid, and --version invalid, for
flang-new -fc1. The invocation

flang-new --version

remains valid. This behaviour is consistent with clang
(and with clang -cc1 and clang -cc1as).

Previously, flang-new -fc1 accepted --version (as per Options.td), but
the frontend driver acutally checks for -version. As a result,

flang-new -fc1 --version

triggered no action, emitted no message, and stalled waiting for
standard input.

Fixes #51438

Diff Detail

Event Timeline

ekieri created this revision.Mar 27 2022, 12:47 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald Transcript
ekieri requested review of this revision.Mar 27 2022, 12:47 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 27 2022, 12:47 AM
PeteSteinfeld accepted this revision.Mar 27 2022, 11:08 PM

Looks good.

Thanks for doing this, Emil!

This revision is now accepted and ready to land.Mar 27 2022, 11:08 PM
awarzynski accepted this revision.Mar 28 2022, 1:06 AM

Makes sense, many thanks for doing this!

clang/include/clang/Driver/Options.td
5758

[nit] IMHO this file lacks comments, hence the suggestion :)

ekieri updated this revision to Diff 418643.Mar 28 2022, 10:47 AM

Address review comment.
Fix typo in summary.

ekieri edited the summary of this revision. (Show Details)Mar 28 2022, 10:48 AM
ekieri marked an inline comment as done.Mar 28 2022, 11:27 AM
ekieri added inline comments.
clang/include/clang/Driver/Options.td
5758

Sure!

On the topic of this file's structure, I found the nested "let Flags" quite confusing -- Setting "Flags" inside "def version" does not work, as it would be overwritten by the outer "let Flags" on line 4820. But that is a different patch.

awarzynski added inline comments.Mar 28 2022, 11:48 AM
clang/include/clang/Driver/Options.td
5758

On the topic of this file's structure, I found the nested "let Flags" quite confusing -- Setting "Flags" inside "def version" does not work, as it would be overwritten by the outer "let Flags" on line 4820. But that is a different patch.

Fixing that is high on my TODO list, just never high enough :( Happy to review if you get a chance to prepare something! ;-)

This revision was landed with ongoing or failed builds.Mar 28 2022, 2:01 PM
This revision was automatically updated to reflect the committed changes.
ekieri marked an inline comment as done.