Deprecate -fheinous-gnu-extensions; introduce a new warning flag (llvm#105821)

AaronBallman · web-flow · commit c505ce9df700 · 2024-08-23T15:38:21.000-04:00
The new warning flag is `-Winvalid-gnu-asm-cast`, which is enabled by
default and is a downgradable diagnostic which defaults to an error.

This language dialect flag only controls whether a single diagnostic is
emitted as a warning or as an error, and has never been expanded to
include other behaviors. Given the rather perjorative name, it's better
for us to just expose a diagnostic flag for the one warning in question
and let the user elect to do `-Wno-error=` if they need to.

There's not a lot of use of the language dialect flag in the wild, but
there is some use of it. For the time being, this aliases the -f flag to
`-Wno-error=invalid-gnu-asm-cast`, but the -f flag can eventually be
removed.
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
@@ -174,6 +174,10 @@ New Compiler Flags
 Deprecated Compiler Flags
 -------------------------
 
+- ``-fheinous-gnu-extensions`` is deprecated; it is now equivalent to
+  specifying ``-Wno-error=invalid-gnu-asm-cast`` and may be removed in the
+  future.
+
 Modified Compiler Flags
 -----------------------
 
@@ -238,6 +242,11 @@ Improvements to Clang's diagnostics
 
 - Improved diagnostic when trying to befriend a concept. (#GH45182).
 
+- Added the ``-Winvalid-gnu-asm-cast`` diagnostic group to control warnings
+  about use of "noop" casts for lvalues (a GNU extension). This diagnostic is
+  a warning which defaults to being an error, is enabled by default, and is
+  also controlled by the now-deprecated ``-fheinous-gnu-extensions`` flag.
+
 Improvements to Clang's time-trace
 ----------------------------------
 
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9365,9 +9365,6 @@ let CategoryName = "Inline Assembly Issue" in {
     "invalid input size for constraint '%0'">;
   def err_asm_invalid_output_size : Error<
     "invalid output size for constraint '%0'">;
-  def err_invalid_asm_cast_lvalue : Error<
-    "invalid use of a cast in a inline asm context requiring an lvalue: "
-    "remove the cast or build with -fheinous-gnu-extensions">;
   def err_invalid_asm_value_for_constraint
       : Error <"value '%0' out of range for constraint '%1'">;
   def err_asm_non_addr_value_in_memory_constraint : Error <
@@ -9381,9 +9378,8 @@ let CategoryName = "Inline Assembly Issue" in {
   def warn_asm_label_on_auto_decl : Warning<
     "ignored asm label '%0' on automatic variable">;
   def warn_invalid_asm_cast_lvalue : Warning<
-    "invalid use of a cast in an inline asm context requiring an lvalue: "
-    "accepted due to -fheinous-gnu-extensions, but clang may remove support "
-    "for this in the future">;
+    "invalid use of a cast in an inline asm context requiring an lvalue">,
+    InGroup<DiagGroup<"invalid-gnu-asm-cast">>, DefaultError;
   def warn_asm_mismatched_size_modifier : Warning<
     "value size does not match register size specified by the constraint "
     "and modifier">,
diff --git a/clang/include/clang/Basic/LangOptions.def b/clang/include/clang/Basic/LangOptions.def
@@ -190,7 +190,6 @@ LANGOPT(POSIXThreads      , 1, 0, "POSIX thread support")
 LANGOPT(Blocks            , 1, 0, "blocks extension to C")
 BENIGN_LANGOPT(EmitAllDecls      , 1, 0, "emitting all declarations")
 LANGOPT(MathErrno         , 1, 1, "errno in math functions")
-BENIGN_LANGOPT(HeinousExtensions , 1, 0, "extensions that we really don't like and may be ripped out at any time")
 LANGOPT(Modules           , 1, 0, "modules semantics")
 COMPATIBLE_LANGOPT(CPlusPlusModules, 1, 0, "C++ modules syntax")
 LANGOPT(SkipODRCheckInGMF, 1, 0, "Skip ODR checks for decls in the global module fragment")
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
@@ -1011,6 +1011,8 @@ def Wwrite_strings : Flag<["-"], "Wwrite-strings">, Group<W_Group>,
   Flags<[HelpHidden]>, Visibility<[ClangOption, CC1Option]>;
 def Wno_write_strings : Flag<["-"], "Wno-write-strings">, Group<W_Group>,
   Flags<[HelpHidden]>, Visibility<[ClangOption, CC1Option]>;
+def Winvalid_gnu_asm_cast : Flag<["-"], "Winvalid-gnu-asm-cast">, Group<W_Group>,
+  Flags<[HelpHidden]>, Visibility<[ClangOption, CC1Option]>;
 def W_Joined : Joined<["-"], "W">, Group<W_Group>,
   Visibility<[ClangOption, CC1Option, CLOption, DXCOption, FC1Option, FlangOption]>,
   MetaVarName<"<warning>">, HelpText<"Enable the specified warning">;
@@ -2761,9 +2763,13 @@ defm gnu89_inline : BoolFOption<"gnu89-inline",
   NegFlag<SetFalse>>, ShouldParseIf<!strconcat("!", cplusplus.KeyPath)>;
 def fgnu_runtime : Flag<["-"], "fgnu-runtime">, Group<f_Group>,
   HelpText<"Generate output compatible with the standard GNU Objective-C runtime">;
+// This used to be a standalone flag but is now mapped to
+// -Wno-error=invalid-gnu-asm-cast, which is the only thing the flag used to
+// control.
 def fheinous_gnu_extensions : Flag<["-"], "fheinous-gnu-extensions">,
-  Visibility<[ClangOption, CC1Option]>,
-  MarshallingInfoFlag<LangOpts<"HeinousExtensions">>;
+  Alias<W_Joined>, AliasArgs<["no-error=invalid-gnu-asm-cast"]>,
+  HelpText<"(Deprecated) Controls whether '-Winvalid-gnu-asm-cast' defaults to "
+           "an error or a warning">;
 def filelist : Separate<["-"], "filelist">, Flags<[LinkerInput]>,
                Group<Link_Group>;
 def : Flag<["-"], "findirect-virtual-calls">, Alias<fapple_kext>;
diff --git a/clang/lib/Sema/SemaStmtAsm.cpp b/clang/lib/Sema/SemaStmtAsm.cpp
@@ -71,13 +71,8 @@ static void removeLValueToRValueCast(Expr *E) {
 /// and fix the argument with removing LValueToRValue cast from the expression.
 static void emitAndFixInvalidAsmCastLValue(const Expr *LVal, Expr *BadArgument,
                                            Sema &S) {
-  if (!S.getLangOpts().HeinousExtensions) {
-    S.Diag(LVal->getBeginLoc(), diag::err_invalid_asm_cast_lvalue)
-        << BadArgument->getSourceRange();
-  } else {
-    S.Diag(LVal->getBeginLoc(), diag::warn_invalid_asm_cast_lvalue)
-        << BadArgument->getSourceRange();
-  }
+  S.Diag(LVal->getBeginLoc(), diag::warn_invalid_asm_cast_lvalue)
+      << BadArgument->getSourceRange();
   removeLValueToRValueCast(BadArgument);
 }
 
diff --git a/clang/test/Analysis/asm.cpp b/clang/test/Analysis/asm.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_analyze_cc1 -triple=x86_64-unknown-unknown \
-// RUN:      -analyzer-checker debug.ExprInspection,core -fheinous-gnu-extensions -w %s -verify
+// RUN:      -analyzer-checker debug.ExprInspection,core -Wno-error=invalid-gnu-asm-cast -w %s -verify
 
 int clang_analyzer_eval(int);
 
diff --git a/clang/test/Analysis/cfg.c b/clang/test/Analysis/cfg.c
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple x86_64-apple-darwin12 -fheinous-gnu-extensions %s > %t 2>&1
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple x86_64-apple-darwin12 -Wno-error=invalid-gnu-asm-cast %s > %t 2>&1
 // RUN: FileCheck --input-file=%t --check-prefix=CHECK %s
 
 // This file is the C version of cfg.cpp.
diff --git a/clang/test/Analysis/cfg.cpp b/clang/test/Analysis/cfg.cpp
@@ -1,6 +1,6 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple x86_64-apple-darwin12 -fheinous-gnu-extensions -std=c++11 -analyzer-config cfg-rich-constructors=false %s > %t 2>&1
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple x86_64-apple-darwin12 -Wno-error=invalid-gnu-asm-cast -std=c++11 -analyzer-config cfg-rich-constructors=false %s > %t 2>&1
 // RUN: FileCheck --input-file=%t -check-prefixes=CHECK,WARNINGS %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple x86_64-apple-darwin12 -fheinous-gnu-extensions -std=c++11 -analyzer-config cfg-rich-constructors=true %s > %t 2>&1
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple x86_64-apple-darwin12 -Wno-error=invalid-gnu-asm-cast -std=c++11 -analyzer-config cfg-rich-constructors=true %s > %t 2>&1
 // RUN: FileCheck --input-file=%t -check-prefixes=CHECK,ANALYZER %s
 
 // This file tests how we construct two different flavors of the Clang CFG -
diff --git a/clang/test/Driver/heinous-gnu-extensions.c b/clang/test/Driver/heinous-gnu-extensions.c
@@ -0,0 +1,5 @@
+// RUN: %clang -### -fsyntax-only -fheinous-gnu-extensions %s 2>&1 | FileCheck %s
+
+// CHECK: -Wno-error=invalid-gnu-asm-cast
+
+int main(void) {}
diff --git a/clang/test/Misc/warning-flags.c b/clang/test/Misc/warning-flags.c
@@ -18,7 +18,7 @@ This test serves two purposes:
 
 The list of warnings below should NEVER grow.  It should gradually shrink to 0.
 
-CHECK: Warnings without flags (64):
+CHECK: Warnings without flags (63):
 
 CHECK-NEXT:   ext_expected_semi_decl_list
 CHECK-NEXT:   ext_missing_whitespace_after_macro_name
@@ -55,7 +55,6 @@ CHECK-NEXT:   warn_fe_macro_contains_embedded_newline
 CHECK-NEXT:   warn_ignoring_ftabstop_value
 CHECK-NEXT:   warn_implements_nscopying
 CHECK-NEXT:   warn_incompatible_qualified_id
-CHECK-NEXT:   warn_invalid_asm_cast_lvalue
 CHECK-NEXT:   warn_invalid_cpu_supports
 CHECK-NEXT:   warn_maynot_respond
 CHECK-NEXT:   warn_method_param_redefinition
diff --git a/clang/test/Sema/heinous-extensions-off.c b/clang/test/Sema/heinous-extensions-off.c
@@ -1,10 +1,9 @@
 // RUN: %clang_cc1 %s -verify
 
-int foo(void) {
-        int a;
-        // PR3788
-        asm("nop" : : "m"((int)(a))); // expected-error {{cast in a inline asm context requiring an lvalue}}
-        // PR3794
-        asm("nop" : "=r"((unsigned)a)); // expected-error {{cast in a inline asm context requiring an lvalue}}
+void foo(void) {
+  int a;
+  // PR3788
+  asm("nop" : : "m"((int)(a))); // expected-error {{invalid use of a cast in an inline asm context requiring an lvalue}}
+  // PR3794
+  asm("nop" : "=r"((unsigned)a)); // expected-error {{invalid use of a cast in an inline asm context requiring an lvalue}}
 }
-
diff --git a/clang/test/Sema/heinous-extensions-on.c b/clang/test/Sema/heinous-extensions-on.c
@@ -1,9 +1,9 @@
-// RUN: %clang_cc1 %s -verify -fheinous-gnu-extensions
+// RUN: %clang_cc1 %s -verify -Wno-error=invalid-gnu-asm-cast
 
 void foo(void) {
   int a;
   // PR3788
-  asm("nop" : : "m"((int)(a))); // expected-warning {{cast in an inline asm context requiring an lvalue}}
+  asm("nop" : : "m"((int)(a))); // expected-warning {{invalid use of a cast in an inline asm context requiring an lvalue}}
   // PR3794
-  asm("nop" : "=r"((unsigned)a)); // expected-warning {{cast in an inline asm context requiring an lvalue}}
+  asm("nop" : "=r"((unsigned)a)); // expected-warning {{invalid use of a cast in an inline asm context requiring an lvalue}}
 }

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple x86_64-apple-darwin12 -fheinous-gnu-extensions %s > %t 2>&1`
	`1`	`+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple x86_64-apple-darwin12 -Wno-error=invalid-gnu-asm-cast %s > %t 2>&1`
`2`	`2`	`// RUN: FileCheck --input-file=%t --check-prefix=CHECK %s`
`3`	`3`
`4`	`4`	`// This file is the C version of cfg.cpp.`