Skip to content

[clang] Remove FEM_Indeterminable - #137247

Merged
ojhunt merged 2 commits into
llvm:mainfrom
swiftlang:users/ojhunt/remove-fem-indeterminable
Apr 28, 2025
Merged

[clang] Remove FEM_Indeterminable#137247
ojhunt merged 2 commits into
llvm:mainfrom
swiftlang:users/ojhunt/remove-fem-indeterminable

Conversation

@ojhunt

@ojhunt ojhunt commented Apr 24, 2025

Copy link
Copy Markdown
Contributor

Remove FEM_Indeterminable as it is unused and cannot be stored safely in an unsigned bitfield

Remove FEM_Indeterminable as it is unused and cannot be stored safely in an unsigned bitfield
@ojhunt
ojhunt requested review from mstorsjo and zahiraam April 24, 2025 20:32
@ojhunt ojhunt self-assigned this Apr 24, 2025
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Apr 24, 2025
@llvmbot

llvmbot commented Apr 24, 2025

Copy link
Copy Markdown
Member

@llvm/pr-subscribers-clang

Author: Oliver Hunt (ojhunt)

Changes

Remove FEM_Indeterminable as it is unused and cannot be stored safely in an unsigned bitfield


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

1 Files Affected:

  • (modified) clang/include/clang/Basic/LangOptions.h (+1-4)
diff --git a/clang/include/clang/Basic/LangOptions.h b/clang/include/clang/Basic/LangOptions.h
index bbebf7af9ede3..1bfc0d8e88556 100644
--- a/clang/include/clang/Basic/LangOptions.h
+++ b/clang/include/clang/Basic/LangOptions.h
@@ -304,10 +304,7 @@ class LangOptionsBase {
   };
 
   /// Possible float expression evaluation method choices.
-  enum FPEvalMethodKind {
-    /// The evaluation method cannot be determined or is inconsistent for this
-    /// target.
-    FEM_Indeterminable = -1,
+  enum FPEvalMethodKind : unsigned {
     /// Use the declared type for fp arithmetic.
     FEM_Source = 0,
     /// Use the type double for fp arithmetic.

@ojhunt

ojhunt commented Apr 24, 2025

Copy link
Copy Markdown
Contributor Author

@zahiraam you added these originally, I can't find any use of FEM_Indeterminable, and no obvious place where any one is doing <0 or <FEM_Source so removing it seems reasonable?

Because language options, etc are stored in unsigned bitfields a negative enum will always come out wrong (the warning being emitted now that we do actually check against a stated preferred_type is correct here). I considered just making it 4 but given it's not used I'm not sure there's a reason to keep it around at all?

@ojhunt
ojhunt merged commit 6b973f2 into llvm:main Apr 28, 2025
@ojhunt
ojhunt deleted the users/ojhunt/remove-fem-indeterminable branch April 28, 2025 15:17
@AaronBallman

Copy link
Copy Markdown
Contributor

Why did this get merged before there were any reviews or responses?

@ojhunt

ojhunt commented Apr 28, 2025

Copy link
Copy Markdown
Contributor Author

Hmmm, I thought it had been?

ojhunt added a commit that referenced this pull request Apr 28, 2025
ojhunt added a commit that referenced this pull request Apr 28, 2025
@ojhunt

ojhunt commented Apr 28, 2025

Copy link
Copy Markdown
Contributor Author

I've reverted, not sure why I thought it had been reviewed?

@AaronBallman

Copy link
Copy Markdown
Contributor

I've reverted, not sure why I thought it had been reviewed?

Thanks! And I'm happy enough blaming GitHub's poor Ux for all confusion. :-)

@zahiraam -- any chance you can look into this?

@zahiraam

zahiraam commented Apr 28, 2025

Copy link
Copy Markdown
Contributor

I've reverted, not sure why I thought it had been reviewed?

Thanks! And I'm happy enough blaming GitHub's poor Ux for all confusion. :-)

@zahiraam -- any chance you can look into this?

Yes I can, in a couple of days. @ojhunt I hope this is OK?

@ojhunt

ojhunt commented Apr 28, 2025

Copy link
Copy Markdown
Contributor Author

@zahiraam it's currently causing a warning (@Endilll asking for an update is why I ended up looking at this on my phone)

@zahiraam

Copy link
Copy Markdown
Contributor

@zahiraam it's currently causing a warning (@Endilll asking for an update is why I ended up looking at this on my phone)

What is the warning?

@nico

nico commented Apr 30, 2025

Copy link
Copy Markdown
Contributor

What is the warning?

=> #137247

IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
Remove FEM_Indeterminable as it is unused and cannot be stored safely in
an unsigned bitfield
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
llvm-sync Bot pushed a commit to arm/arm-toolchain that referenced this pull request May 6, 2025
GeorgeARM pushed a commit to GeorgeARM/llvm-project that referenced this pull request May 7, 2025
Remove FEM_Indeterminable as it is unused and cannot be stored safely in
an unsigned bitfield
GeorgeARM pushed a commit to GeorgeARM/llvm-project that referenced this pull request May 7, 2025
markrvmurray pushed a commit to markrvmurray/llvm-mc6809 that referenced this pull request Jun 14, 2026
OpenHarmonySCM-noreply pushed a commit to openharmony/third_party_llvm-project that referenced this pull request Jul 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants