[PATCH] D90194: [Driver] split LangOptions::SSPOff into SSPOFF and SSPUnspecified

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

In D90194#2377337 <https://reviews.llvm.org/D90194#2377337>, @rnk wrote:

> Given that only three tests fail when the `nossp` attribute gets added from `-cc1` with no stack protector, I think it's reasonable to add it and skip adding the extra enum.

Sorry, when I said:

In D90194#2354908 <https://reviews.llvm.org/D90194#2354908>, @nickdesaulniers wrote:

>> An alternative approach to consider would be not adding a new enum, and making -stack-protector ALWAYS be specified when invoking cc1. I did not quantify the amount of changes to existing tests, but am happy to do so if reviewers would like.
>
> Making `-stack-protector` always be passed, and default to `0` rather than being unspecified causes the following tests to fail:
>
>   Failed Tests (3):
>     Clang :: Driver/cl-options.c
>     Clang :: Driver/fsanitize.c
>     Clang :: Driver/stack-protector.c
>
> which is much less than I was expecting.  It's trivial to not create a new enum value and always pass `-stack-protector <N>` along.  On top of this patch:

I followed up that it wasn't as simple as 3 tests failing.  I followed up with:

In D90194#2360392 <https://reviews.llvm.org/D90194#2360392>, @nickdesaulniers wrote:

>> It's trivial to not create a new enum value and always pass -stack-protector <N> along
>
> That was incorrect.  Rebasing this on top of D90271 <https://reviews.llvm.org/D90271> causes 49 test failures, because the diff I posted above didn't actually remove the new enum or changes to clang/lib/Frontend/CompilerInvocation.cpp.  So I've answered my own question; we should add this enum value.  Let me rebase this differently then.

I'm happy to rewrite this patch to not introduce the new enum, but it's going to be much larger than this patch due to churn in the tests.  Lets me take a crack at it and I'll post it as a separate review.  Then I think it would help illuminate the differences in approach and we can take our pick.  Thanks all for the feedback.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90194/new/

https://reviews.llvm.org/D90194