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

[lld] Add explicit conversion for enum to Twine. #100627

Merged
merged 1 commit into from
Jul 25, 2024

Conversation

ScottTodd
Copy link
Member

This fixes error: ambiguous conversion for functional-style cast from 'lld::macho::InputSection::Kind' to 'llvm::Twine', observed when building with clang-9 and reported here: #96268 (comment).

@ellishg
Copy link
Contributor

ellishg commented Jul 25, 2024

Thanks for fixing! If you publish the draft I will accept.

@ScottTodd ScottTodd marked this pull request as ready for review July 25, 2024 18:30
@ScottTodd ScottTodd requested a review from ellishg July 25, 2024 18:30
@llvmbot
Copy link
Member

llvmbot commented Jul 25, 2024

@llvm/pr-subscribers-lld

Author: Scott Todd (ScottTodd)

Changes

This fixes error: ambiguous conversion for functional-style cast from 'lld::macho::InputSection::Kind' to 'llvm::Twine', observed when building with clang-9 and reported here: #96268 (comment).


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

1 Files Affected:

  • (modified) lld/MachO/BPSectionOrderer.cpp (+2-2)
diff --git a/lld/MachO/BPSectionOrderer.cpp b/lld/MachO/BPSectionOrderer.cpp
index 26d4e0cb3987d..f6a974370836b 100644
--- a/lld/MachO/BPSectionOrderer.cpp
+++ b/lld/MachO/BPSectionOrderer.cpp
@@ -48,9 +48,9 @@ getRelocHash(const Reloc &reloc,
     sectionIdx = sectionIdxIt->getSecond();
   std::string kind;
   if (isec)
-    kind = ("Section " + Twine(isec->kind())).str();
+    kind = ("Section " + Twine((uint8_t)isec->kind())).str();
   if (auto *sym = reloc.referent.dyn_cast<Symbol *>()) {
-    kind += (" Symbol " + Twine(sym->kind())).str();
+    kind += (" Symbol " + Twine((uint8_t)sym->kind())).str();
     if (auto *d = dyn_cast<Defined>(sym)) {
       if (isa_and_nonnull<CStringInputSection>(isec))
         return getRelocHash(kind, 0, isec->getOffset(d->value), reloc.addend);

@llvmbot
Copy link
Member

llvmbot commented Jul 25, 2024

@llvm/pr-subscribers-lld-macho

Author: Scott Todd (ScottTodd)

Changes

This fixes error: ambiguous conversion for functional-style cast from 'lld::macho::InputSection::Kind' to 'llvm::Twine', observed when building with clang-9 and reported here: #96268 (comment).


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

1 Files Affected:

  • (modified) lld/MachO/BPSectionOrderer.cpp (+2-2)
diff --git a/lld/MachO/BPSectionOrderer.cpp b/lld/MachO/BPSectionOrderer.cpp
index 26d4e0cb3987d..f6a974370836b 100644
--- a/lld/MachO/BPSectionOrderer.cpp
+++ b/lld/MachO/BPSectionOrderer.cpp
@@ -48,9 +48,9 @@ getRelocHash(const Reloc &reloc,
     sectionIdx = sectionIdxIt->getSecond();
   std::string kind;
   if (isec)
-    kind = ("Section " + Twine(isec->kind())).str();
+    kind = ("Section " + Twine((uint8_t)isec->kind())).str();
   if (auto *sym = reloc.referent.dyn_cast<Symbol *>()) {
-    kind += (" Symbol " + Twine(sym->kind())).str();
+    kind += (" Symbol " + Twine((uint8_t)sym->kind())).str();
     if (auto *d = dyn_cast<Defined>(sym)) {
       if (isa_and_nonnull<CStringInputSection>(isec))
         return getRelocHash(kind, 0, isec->getOffset(d->value), reloc.addend);

@ScottTodd
Copy link
Member Author

Thanks for the quick review! Confirmed that our downstream build (using clang-9) was successful with this patch: https://github.com/iree-org/iree/actions/runs/10099225942/job/27927980200?pr=18004

@ScottTodd ScottTodd merged commit 4f80508 into llvm:main Jul 25, 2024
10 of 11 checks passed
@ScottTodd ScottTodd deleted the fix-twine-conversion branch July 25, 2024 18:37
@llvm-ci
Copy link
Collaborator

llvm-ci commented Jul 27, 2024

LLVM Buildbot has detected a new failure on builder llvm-x86_64-debian-dylib running on gribozavr4 while building lld at step 9 "test-build-unified-tree-check-clang-extra".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/60/builds/3498

Here is the relevant piece of the build log for the reference:

Step 9 (test-build-unified-tree-check-clang-extra) failure: test (failure)
******************** TEST 'Clang Tools :: clang-doc/basic-project.test' FAILED ********************
Exit Code: 1

Command Output (stdout):
--
Emiting docs in html format.
Mapping decls...
Collecting infos...
Reducing 5 infos...
Generating docs...
Generating assets for docs...

--
Command Output (stderr):
--
RUN: at line 1: rm -rf /b/1/llvm-x86_64-debian-dylib/build/tools/clang/tools/extra/test/clang-doc/Output/basic-project.test.tmp && mkdir -p /b/1/llvm-x86_64-debian-dylib/build/tools/clang/tools/extra/test/clang-doc/Output/basic-project.test.tmp/docs /b/1/llvm-x86_64-debian-dylib/build/tools/clang/tools/extra/test/clang-doc/Output/basic-project.test.tmp/build
+ rm -rf /b/1/llvm-x86_64-debian-dylib/build/tools/clang/tools/extra/test/clang-doc/Output/basic-project.test.tmp
+ mkdir -p /b/1/llvm-x86_64-debian-dylib/build/tools/clang/tools/extra/test/clang-doc/Output/basic-project.test.tmp/docs /b/1/llvm-x86_64-debian-dylib/build/tools/clang/tools/extra/test/clang-doc/Output/basic-project.test.tmp/build
RUN: at line 2: sed 's|$test_dir|/b/1/llvm-x86_64-debian-dylib/llvm-project/clang-tools-extra/test/clang-doc|g' /b/1/llvm-x86_64-debian-dylib/llvm-project/clang-tools-extra/test/clang-doc/Inputs/basic-project/database_template.json > /b/1/llvm-x86_64-debian-dylib/build/tools/clang/tools/extra/test/clang-doc/Output/basic-project.test.tmp/build/compile_commands.json
+ sed 's|$test_dir|/b/1/llvm-x86_64-debian-dylib/llvm-project/clang-tools-extra/test/clang-doc|g' /b/1/llvm-x86_64-debian-dylib/llvm-project/clang-tools-extra/test/clang-doc/Inputs/basic-project/database_template.json
RUN: at line 3: clang-doc --format=html --output=/b/1/llvm-x86_64-debian-dylib/build/tools/clang/tools/extra/test/clang-doc/Output/basic-project.test.tmp/docs --executor=all-TUs /b/1/llvm-x86_64-debian-dylib/build/tools/clang/tools/extra/test/clang-doc/Output/basic-project.test.tmp/build/compile_commands.json
+ clang-doc --format=html --output=/b/1/llvm-x86_64-debian-dylib/build/tools/clang/tools/extra/test/clang-doc/Output/basic-project.test.tmp/docs --executor=all-TUs /b/1/llvm-x86_64-debian-dylib/build/tools/clang/tools/extra/test/clang-doc/Output/basic-project.test.tmp/build/compile_commands.json
[1/3] Processing file /b/1/llvm-x86_64-debian-dylib/llvm-project/clang-tools-extra/test/clang-doc/Inputs/basic-project/src/Rectangle.cpp
[2/3] Processing file /b/1/llvm-x86_64-debian-dylib/llvm-project/clang-tools-extra/test/clang-doc/Inputs/basic-project/src/Circle.cpp
[3/3] Processing file /b/1/llvm-x86_64-debian-dylib/llvm-project/clang-tools-extra/test/clang-doc/Inputs/basic-project/src/Calculator.cpp
RUN: at line 4: FileCheck /b/1/llvm-x86_64-debian-dylib/llvm-project/clang-tools-extra/test/clang-doc/basic-project.test -input-file=/b/1/llvm-x86_64-debian-dylib/build/tools/clang/tools/extra/test/clang-doc/Output/basic-project.test.tmp/docs/index_json.js -check-prefix=JSON-INDEX
+ FileCheck /b/1/llvm-x86_64-debian-dylib/llvm-project/clang-tools-extra/test/clang-doc/basic-project.test -input-file=/b/1/llvm-x86_64-debian-dylib/build/tools/clang/tools/extra/test/clang-doc/Output/basic-project.test.tmp/docs/index_json.js -check-prefix=JSON-INDEX
RUN: at line 5: FileCheck /b/1/llvm-x86_64-debian-dylib/llvm-project/clang-tools-extra/test/clang-doc/basic-project.test -input-file=/b/1/llvm-x86_64-debian-dylib/build/tools/clang/tools/extra/test/clang-doc/Output/basic-project.test.tmp/docs/GlobalNamespace/Shape.html -check-prefix=HTML-SHAPE
+ FileCheck /b/1/llvm-x86_64-debian-dylib/llvm-project/clang-tools-extra/test/clang-doc/basic-project.test -input-file=/b/1/llvm-x86_64-debian-dylib/build/tools/clang/tools/extra/test/clang-doc/Output/basic-project.test.tmp/docs/GlobalNamespace/Shape.html -check-prefix=HTML-SHAPE
/b/1/llvm-x86_64-debian-dylib/llvm-project/clang-tools-extra/test/clang-doc/basic-project.test:64:16: error: HTML-SHAPE: expected string not found in input
// HTML-SHAPE: <h3 id="{{([0-9A-F]{40})}}">area</h3>
               ^
/b/1/llvm-x86_64-debian-dylib/build/tools/clang/tools/extra/test/clang-doc/Output/basic-project.test.tmp/docs/GlobalNamespace/Shape.html:32:53: note: scanning from here
 <p>Defined at line 13 of file ./include/Shape.h</p>
                                                    ^
/b/1/llvm-x86_64-debian-dylib/build/tools/clang/tools/extra/test/clang-doc/Output/basic-project.test.tmp/docs/GlobalNamespace/Shape.html:47:41: note: possible intended match here
 <a href="#12896F9255F880ECD4A6482CCFA58B238FA2CC49">area</a>
                                        ^

Input file: /b/1/llvm-x86_64-debian-dylib/build/tools/clang/tools/extra/test/clang-doc/Output/basic-project.test.tmp/docs/GlobalNamespace/Shape.html
Check file: /b/1/llvm-x86_64-debian-dylib/llvm-project/clang-tools-extra/test/clang-doc/basic-project.test

-dump-input=help explains the following input dump.

Input was:
<<<<<<
            .
            .
            .
           27:  <div> 
...

ellishg added a commit that referenced this pull request Aug 1, 2024
Add the "Separate" option `--irpgo-profile-sort <profile` instead of
just the "Joined" option `--irpgo-profile-sort=<profile>`. This is
useful if the path has a `,` for some reason which would break when
trying to use `-Wl,--irpgo-profile-sort=<profile-with-comma>`.

While I'm here, use `static_cast<>` instead of the C style cast
introduced in #100627
banach-space pushed a commit to banach-space/llvm-project that referenced this pull request Aug 7, 2024
Add the "Separate" option `--irpgo-profile-sort <profile` instead of
just the "Joined" option `--irpgo-profile-sort=<profile>`. This is
useful if the path has a `,` for some reason which would break when
trying to use `-Wl,--irpgo-profile-sort=<profile-with-comma>`.

While I'm here, use `static_cast<>` instead of the C style cast
introduced in llvm#100627
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants