[PATCH] D83648: [Driver] Fix integrated_as definition by setting it as a DriverOption

MaskRay added a comment. Fixed by 6a75496836ea14bcfd2f4b59d35a1cad4ac58cee :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83648/new/ https://reviews.llvm.org/D83648

[PATCH] D83648: [Driver] Fix integrated_as definition by setting it as a DriverOption

pzheng updated this revision to Diff 278361. pzheng added a comment. Add target triple used in test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83648/new/ https://reviews.llvm.org/D83648 Files: clang/include/clang/Driver/Options.td

[PATCH] D83648: [Driver] Fix integrated_as definition by setting it as a DriverOption

pzheng marked 2 inline comments as done. pzheng added a comment. In D83648#2146606 , @MaskRay wrote: > Created http://lists.llvm.org/pipermail/cfe-dev/2020-July/066245.html > [cfe-dev] Usage of clang::driver::options::DriverOption (-Xarch_ & gcc >

[PATCH] D83648: [Driver] Fix integrated_as definition by setting it as a DriverOption

MaskRay added a comment. Created http://lists.llvm.org/pipermail/cfe-dev/2020-July/066245.html [cfe-dev] Usage of clang::driver::options::DriverOption (-Xarch_ & gcc toolchain) to ask about the use case. When I get time, I'll work on a patch fixing the whole class of options instead of just

[PATCH] D83648: [Driver] Fix integrated_as definition by setting it as a DriverOption

MaskRay added a comment. Actually, the way we tag options recognized by GCC is wrong. We should use a more specific group instead of `DriverOption`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83648/new/ https://reviews.llvm.org/D83648

[PATCH] D83648: [Driver] Fix integrated_as definition by setting it as a DriverOption

MaskRay added inline comments. Comment at: clang/test/Driver/integrated-as.c:10 +// RUN: %clang -### -fintegrated-as %s 2>&1 | FileCheck %s -check-prefix FIAS-LINK + MaskRay wrote: > This test is incorrect. You need a specific target triple to select >

[PATCH] D83648: [Driver] Fix integrated_as definition by setting it as a DriverOption

MaskRay added a comment. In D83648#2146472 , @pzheng wrote: > Actually, this patch won't change --help because it just reduced some > duplication by extracting the common part (" the integrated assembler") of > the help message into the "help" of

[PATCH] D83648: [Driver] Fix integrated_as definition by setting it as a DriverOption

pzheng updated this revision to Diff 277316. pzheng added a comment. Add a test case. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83648/new/ https://reviews.llvm.org/D83648 Files: clang/include/clang/Driver/Options.td

[PATCH] D83648: [Driver] Fix integrated_as definition by setting it as a DriverOption

dblaikie added a comment. In D83648#2146472 , @pzheng wrote: > Actually, this patch won't change --help because it just reduced some > duplication by extracting the common part (" the integrated assembler") of > the help message into the "help" of

[PATCH] D83648: [Driver] Fix integrated_as definition by setting it as a DriverOption

pzheng added a comment. Actually, this patch won't change --help because it just reduced some duplication by extracting the common part (" the integrated assembler") of the help message into the "help" of "OptOutFFlag". Sorry for the confusion. The real fix here is to restore "integrated_as"

[PATCH] D83648: [Driver] Fix integrated_as definition by setting it as a DriverOption

dblaikie added a comment. Is there missing test coverage for this in some way? (how does this patch change the observable behavior of clang? I guess --help would change?) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83648/new/

[PATCH] D83648: [Driver] Fix integrated_as definition by setting it as a DriverOption

pzheng created this revision. pzheng added reviewers: MaskRay, dblaikie, echristo. Herald added subscribers: cfe-commits, dang. Herald added a project: clang. DriverOption seems to be accidentally removed from integrated_as definition in commit e5158b5