Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NFC] Check for defined(__GNUC__) before use #116076

Merged
merged 2 commits into from
Nov 14, 2024

Conversation

DavidTruby
Copy link
Member

@DavidTruby DavidTruby commented Nov 13, 2024

This silences some spurious warnings on Windows builds with clang-cl that __GNUC__ is
not defined if -Wundef is passed, which is the default in MLIR.
These warnings make Windows builds of LLVM very noisy when MLIR is included.

This silences some spurious warnings on Windows builds that __GNUC__ is
not defined. These warnings make Windows builds very noisy.
@llvmbot
Copy link

llvmbot commented Nov 13, 2024

@llvm/pr-subscribers-llvm-adt

Author: David Truby (DavidTruby)

Changes

This silences some spurious warnings on Windows builds that GNUC is
not defined. These warnings make Windows builds very noisy.


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

1 Files Affected:

  • (modified) llvm/include/llvm/ADT/iterator_range.h (+2-1)
diff --git a/llvm/include/llvm/ADT/iterator_range.h b/llvm/include/llvm/ADT/iterator_range.h
index 6c66def0fcd77b..8e9b22f2d4dfa7 100644
--- a/llvm/include/llvm/ADT/iterator_range.h
+++ b/llvm/include/llvm/ADT/iterator_range.h
@@ -43,7 +43,8 @@ class iterator_range {
   IteratorT begin_iterator, end_iterator;
 
 public:
-#if __GNUC__ == 7 || (__GNUC__ == 8 && __GNUC_MINOR__ < 4)
+#if defined(__GNUC__) &&                                                       \
+    (__GNUC__ == 7 || (__GNUC__ == 8 && __GNUC_MINOR__ < 4))
   // Be careful no to break gcc-7 and gcc-8 < 8.4 on the mlir target.
   // See https://github.com/llvm/llvm-project/issues/63843
   template <typename Container>

Copy link
Member

@zero9178 zero9178 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM thank you! Ran into this recently as well and can confirm this fixes it

Might be worth being more precise in the commit message that this only happens when using clang-cl and -Wundef, which MLIR does eg. So this is even an in-tree issue

Copy link
Contributor

@RoboTux RoboTux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM as well, thanks for the fix.

@DavidTruby DavidTruby merged commit 8781a43 into llvm:main Nov 14, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants