[PATCH] D88566: be more specific when testing for no fuse-ld warnings

[Ties Stuij via Phabricator via cfe-commits]

stuij added a comment.

> If this cannot be reproduced with the OSS LLVM, I am not sure you should 
> adjust such a test.

Ok, fair enough. Thanks for the comment.




Comment at: clang/test/Driver/fuse-ld.c:15
 // RUN:   FileCheck %s --check-prefix=CHECK-NO-WARN
-// CHECK-NO-WARN-NOT: warning:
+// CHECK-NO-WARN-NOT: warning: 'fuse-ld'
 

MaskRay wrote:
> stuij wrote:
> > MaskRay wrote:
> > > How does this line trigger unrelated warnings? Can you dump it?
> > see my top-level comment
> The impoerant bit is that the original message disallows any warning. The new 
> message with an incorrect 'fuse-ld' (instead of '-fuse-ld') seems really 
> questionable.
Ai, a typo :(


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88566

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88566: be more specific when testing for no fuse-ld warnings

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D88566#2317248 , @stuij wrote:

> Hi @MaskRay. Yes, so we're seeing a warning specific to our Armcompiler 
> toolchain, so I'm guessing that isn't relevant to OSS LLVM:
> `armclang: warning: '--target=x86_64-unknown-linux' is not supported.`

The test uses '-###' and nothing related to X86 backend 
(-DLLVM_TARGETS_TO_BUILD does not include X86), so the intention may be 
quetionable.

> As David Green pointed out, we have a perfectly fine workaround. But I 
> figured that a similar situation might crop up in OSS LLVM, and this way the 
> test is a bit more future proof, and we might spare some future 
> head-scratching.

If this cannot be reproduced with the OSS LLVM, I am not sure you should adjust 
such a test.




Comment at: clang/test/Driver/fuse-ld.c:15
 // RUN:   FileCheck %s --check-prefix=CHECK-NO-WARN
-// CHECK-NO-WARN-NOT: warning:
+// CHECK-NO-WARN-NOT: warning: 'fuse-ld'
 

stuij wrote:
> MaskRay wrote:
> > How does this line trigger unrelated warnings? Can you dump it?
> see my top-level comment
The impoerant bit is that the original message disallows any warning. The new 
message with an incorrect 'fuse-ld' (instead of '-fuse-ld') seems really 
questionable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88566

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88566: be more specific when testing for no fuse-ld warnings

[Ties Stuij via Phabricator via cfe-commits]

stuij abandoned this revision.
stuij added a comment.

Abandoned because lack of reaction for such an unimportant issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88566

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88566: be more specific when testing for no fuse-ld warnings

[Ties Stuij via Phabricator via cfe-commits]

stuij added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88566

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88566: be more specific when testing for no fuse-ld warnings

[Ties Stuij via Phabricator via cfe-commits]

stuij added inline comments.



Comment at: clang/test/Driver/fuse-ld.c:15
 // RUN:   FileCheck %s --check-prefix=CHECK-NO-WARN
-// CHECK-NO-WARN-NOT: warning:
+// CHECK-NO-WARN-NOT: warning: 'fuse-ld'
 

MaskRay wrote:
> How does this line trigger unrelated warnings? Can you dump it?
see my top-level comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88566

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88566: be more specific when testing for no fuse-ld warnings

[Ties Stuij via Phabricator via cfe-commits]

stuij added a comment.

Hi @MaskRay. Yes, so we're seeing a warning specific to our Armcompiler 
toolchain, so I'm guessing that isn't relevant to OSS LLVM:
`armclang: warning: '--target=x86_64-unknown-linux' is not supported.`

As David Green pointed out, we have a perfectly fine workaround. But I figured 
that a similar situation might crop up in OSS LLVM, and this way the test is a 
bit more future proof, and we might spare some future head-scratching.

However I feel bad already for wasting our time with such a minor change. Feel 
free to reject it :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88566

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88566: be more specific when testing for no fuse-ld warnings

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/test/Driver/fuse-ld.c:15
 // RUN:   FileCheck %s --check-prefix=CHECK-NO-WARN
-// CHECK-NO-WARN-NOT: warning:
+// CHECK-NO-WARN-NOT: warning: 'fuse-ld'
 

How does this line trigger unrelated warnings? Can you dump it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88566

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88566: be more specific when testing for no fuse-ld warnings

[Dave Green via Phabricator via cfe-commits]

dmgreen added a comment.

(Not that I'm against this, either way sounds fine to me)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88566

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88566: be more specific when testing for no fuse-ld warnings

[Dave Green via Phabricator via cfe-commits]

dmgreen added a comment.

In the past we have usually disabled the downstream warning for similar 
catch-all warning lines.




Comment at: clang/test/Driver/fuse-ld.c:5
 // RUN:   FileCheck %s --check-prefix=CHECK-ABSOLUTE-LD
 // CHECK-ABSOLUTE-LD: warning: '-fuse-ld=' taking a path is deprecated. Use 
'--ld-path=' instead
 // CHECK-ABSOLUTE-LD: /usr/local/bin/or1k-linux-ld

This line has a = on the end. Would the other warning below also have that? 
(Negative tests are often tricky).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88566

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88566: be more specific when testing for no fuse-ld warnings

[Ties Stuij via Phabricator via cfe-commits]

stuij updated this revision to Diff 295262.
stuij added a comment.

slight change in commit message


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88566

Files:
  clang/test/Driver/fuse-ld.c


Index: clang/test/Driver/fuse-ld.c
===
--- clang/test/Driver/fuse-ld.c
+++ clang/test/Driver/fuse-ld.c
@@ -12,7 +12,7 @@
 // RUN: %clang %s -### -target x86_64-unknown-linux \
 // RUN:   -fuse-ld=/usr/local/bin/or1k-linux-ld 2>&1 | \
 // RUN:   FileCheck %s --check-prefix=CHECK-NO-WARN
-// CHECK-NO-WARN-NOT: warning:
+// CHECK-NO-WARN-NOT: warning: 'fuse-ld'
 
 // RUN: %clang %s -### \
 // RUN: -target x86_64-unknown-freebsd 2>&1 \


Index: clang/test/Driver/fuse-ld.c
===
--- clang/test/Driver/fuse-ld.c
+++ clang/test/Driver/fuse-ld.c
@@ -12,7 +12,7 @@
 // RUN: %clang %s -### -target x86_64-unknown-linux \
 // RUN:   -fuse-ld=/usr/local/bin/or1k-linux-ld 2>&1 | \
 // RUN:   FileCheck %s --check-prefix=CHECK-NO-WARN
-// CHECK-NO-WARN-NOT: warning:
+// CHECK-NO-WARN-NOT: warning: 'fuse-ld'
 
 // RUN: %clang %s -### \
 // RUN: -target x86_64-unknown-freebsd 2>&1 \
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88566: be more specific when testing for no fuse-ld warnings

[Ties Stuij via Phabricator via cfe-commits]

stuij created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
stuij requested review of this revision.

This test broke for our toolchain as this test triggered unrelated
warnings. Being more specific about not expecting fuse-ld warnings won't
invalidate the test, while playing a bit nicer with possible unrelated features.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88566

Files:
  clang/test/Driver/fuse-ld.c


Index: clang/test/Driver/fuse-ld.c
===
--- clang/test/Driver/fuse-ld.c
+++ clang/test/Driver/fuse-ld.c
@@ -12,7 +12,7 @@
 // RUN: %clang %s -### -target x86_64-unknown-linux \
 // RUN:   -fuse-ld=/usr/local/bin/or1k-linux-ld 2>&1 | \
 // RUN:   FileCheck %s --check-prefix=CHECK-NO-WARN
-// CHECK-NO-WARN-NOT: warning:
+// CHECK-NO-WARN-NOT: warning: 'fuse-ld'
 
 // RUN: %clang %s -### \
 // RUN: -target x86_64-unknown-freebsd 2>&1 \


Index: clang/test/Driver/fuse-ld.c
===
--- clang/test/Driver/fuse-ld.c
+++ clang/test/Driver/fuse-ld.c
@@ -12,7 +12,7 @@
 // RUN: %clang %s -### -target x86_64-unknown-linux \
 // RUN:   -fuse-ld=/usr/local/bin/or1k-linux-ld 2>&1 | \
 // RUN:   FileCheck %s --check-prefix=CHECK-NO-WARN
-// CHECK-NO-WARN-NOT: warning:
+// CHECK-NO-WARN-NOT: warning: 'fuse-ld'
 
 // RUN: %clang %s -### \
 // RUN: -target x86_64-unknown-freebsd 2>&1 \
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits