[clang] Remove HelpHidden from the -resource-dir option

[carlosgalvezp]

Currently the "getter" option -print-resource-dir is visible when typing --help, but the corresponding "setter" option -resource-dir is not.

This option is useful when one is using clang on a non-standard location, or when one is building a libtooling-based tool (e.g. IWYU) based on a local clang build. In that case, we need to specify the correct path to the resource directory for things to work.

Existing documentation already makes use of this option, for example here:

https://clang.llvm.org/docs/StandardCPlusPlusModules.html#possible-issues-failed-to-find-system-headers

There is thus no reason to keep this option hidden from the help and documentation.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Carlos Galvez (carlosgalvezp)

Changes

Currently the "getter" flag '-print-resource-dir' is public, but the corresponding "setter" flag '-resource-dir' is not.

This flag is useful when one is using clang on a non-standard location, or when one is building a libtooling-based tool (e.g. IWYU) based on a local clang build. In that case, we need to specify the correct path to the resource directory for things to work.

Existing documentation already makes use of this flag, for example here:

https://clang.llvm.org/docs/StandardCPlusPlusModules.html#possible-issues-failed-to-find-system-headers

There is thus no reason to keep this flag hidden.

---

Full diff: https://github.com/llvm/llvm-project/pull/163131.diff

1 Files Affected:

• (modified) clang/include/clang/Driver/Options.td (+7-5)

diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index a55a5236b2da1..f4c9011266aa3 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -6121,11 +6121,13 @@ def rewrite_legacy_objc : Flag<["-"], "rewrite-legacy-objc">,
   HelpText<"Rewrite Legacy Objective-C source to C++">;
 def rdynamic : Flag<["-"], "rdynamic">, Group<Link_Group>,
   Visibility<[ClangOption, FlangOption]>;
-def resource_dir : Separate<["-"], "resource-dir">,
-  Flags<[NoXarchOption, HelpHidden]>,
-  Visibility<[ClangOption, CC1Option, CLOption, DXCOption, FlangOption, FC1Option]>,
-  HelpText<"The directory which holds the compiler resource files">,
-  MarshallingInfoString<HeaderSearchOpts<"ResourceDir">>;
+def resource_dir
+    : Separate<["-"], "resource-dir">,
+      Flags<[NoXarchOption]>,
+      Visibility<[ClangOption, CC1Option, CLOption, DXCOption, FlangOption,
+                  FC1Option]>,
+      HelpText<"The directory which holds the compiler resource files">,
+      MarshallingInfoString<HeaderSearchOpts<"ResourceDir">>;
 def resource_dir_EQ : Joined<["-"], "resource-dir=">, Flags<[NoXarchOption]>,
   Visibility<[ClangOption, CLOption, DXCOption, FlangOption]>,
   Alias<resource_dir>;

[AaronBallman]

I think these changes should come with tests in clang/test/Driver and should also come with a release note in clang/docs/ReleaseNotes.rst. I've added the options maintainers to see if they agree with making the option public; I think it's reasonable, but I could be wrong for reasons I don't know.

[jansvoboda11]

Looks good, but I agree this needs a test and I'd prefer the formatting change to be reverted, so it clearer what actually changed in the TableGen file.

[carlosgalvezp]

I agree this needs a test

Actually I would argue such a test already exists 😅

https://github.com/llvm/llvm-project/blob/main/clang/test/Driver/print-resource-dir.c

Or do you have some ideas for how it could be tested in a different way?

[jansvoboda11]

Ah, with the formatting change reverted, I see your patch only makes the flag appear in the help text. It's already accepted by the driver, as the test demonstrates. In that case no extra test is necessary, and the release note probably needs to be updated to match the actual change.

[carlosgalvezp]

Will the flag automatically show up in the HTML documentation? I didn't find any .rst file to add this to, so I'm guessing it's automatically generated.

[MaskRay]

I was confused by the title (unclear what "public" means). Removing HelpHidden from -resource-dir would be clearer. Note: don't omit the leading - or -- when referring to options. While flags is used to refer options in some other command line parsing utilities, we only use this term for options without a value.

[jansvoboda11]

LGTM, thanks!

[carlosgalvezp]

CI appears broken for Windows. Is it OK to merge anyway?

[DavidSpickett]

Failure is Clang-Unit.Basic/_/BasicTests_exe/SuppressionMappingTest/LongestMatchWins which is unrelated and I know I've seen fail on the bots. I think it has been reverted or fixed, so if you rebase CI should pass.

If not then go ahead and merge.