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

Make error codes into strings #67086

Conversation

GuillaumeGomez
Copy link
Member

@GuillaumeGomez GuillaumeGomez commented Dec 6, 2019

Part of #67061.

I used this script to convert all error codes into strings:

from os import listdir
from os.path import isdir, isfile, join


def remove_import_and_replace_err_codes(f_path):
    result = []
    with open(f_path) as f:
        lines = f.read().split("\n")
        i = 0
        found = False
        while i < len(lines):
            if lines[i] == 'use rustc_error_codes::*;':
                found = True
                pass
            elif ' E0' in lines[i] and not lines[i].strip().startswith('//') and not lines[i].endswith('\\') and not '"' in lines[i]:
                found = False
                parts = lines[i].split(' E0')
                p = parts[1].split(',', 1)
                if len(p) < 2:
                    result.append(lines[i])
                else:
                    result.append('{} "E0{}",{}'.format(parts[0], p[0], p[1]))
            elif '(E0' in lines[i] and not lines[i].strip().startswith('//') and not lines[i].endswith('
\\') and not '"' in lines[i]:
                found = False
                parts = lines[i].split('(E0')
                p = parts[1].split(')', 1)
                p2 = parts[1].split(',', 1)
                if len(p2[0]) < len(p[0]) and len(p2) > 1:
                    p = p2
                if len(p) < 2:
                    result.append(lines[i])
                else:
                    result.append('{}("E0{}"){}'.format(parts[0], p[0], p[1]))
            else:
                if len(lines[i]) != 0 or found is False:
                    result.append(lines[i])
                found = False
            i += 1
    with open(f_path, 'w') as f:
        f.write('\n'.join(result))


def remove_dependency(f_path):
    result = []
    with open(f_path) as f:
        lines = f.read().split("\n")
        i = 0
        while i < len(lines):
            if lines[i].startswith('rustc_error_codes = '):
      	        pass
            else:
                result.append(lines[i])
            i += 1
    with open(f_path, 'w') as f:
        f.write('\n'.join(result))


def loop_dirs(cur_dir):
    for entry in listdir(cur_dir):
        f = join(cur_dir, entry)
        if isfile(f):
            if entry.endswith(".rs"):
                remove_import_and_replace_err_codes(f)
            elif entry.endswith("Cargo.toml"):
                remove_dependency(f)
        elif isdir(f) and not entry.startswith("librustc_error_codes") and f.startswith('src/lib'):
            loop_dirs(f)


loop_dirs('src')

r? @Centril

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 6, 2019
@rust-highfive
Copy link
Collaborator

The job mingw-check of your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
2019-12-06T12:51:00.3621213Z ##[command]git remote add origin https://github.com/rust-lang/rust
2019-12-06T12:51:01.0994672Z ##[command]git config gc.auto 0
2019-12-06T12:51:01.1000074Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2019-12-06T12:51:01.1004616Z ##[command]git config --get-all http.proxy
2019-12-06T12:51:01.1009553Z ##[command]git -c http.extraheader="AUTHORIZATION: basic ***" fetch --force --tags --prune --progress --no-recurse-submodules --depth=2 origin +refs/heads/*:refs/remotes/origin/* +refs/pull/67086/merge:refs/remotes/pull/67086/merge
---
2019-12-06T12:58:28.8784743Z 
2019-12-06T12:58:28.8859613Z error[E0432]: unresolved import `errors`
2019-12-06T12:58:28.8860397Z   --> src/librustc_parse/parser/ty.rs:17:5
2019-12-06T12:58:28.8861038Z    |
2019-12-06T12:58:28.8861770Z 17 | use errors::{PResult, Applicability, pluralize};
2019-12-06T12:58:28.8862514Z    |     ^^^^^^ help: a similar path exists: `syntax::errors`
2019-12-06T12:58:30.1894849Z error: aborting due to 4 previous errors
2019-12-06T12:58:30.1895968Z 
2019-12-06T12:58:30.1903804Z For more information about this error, try `rustc --explain E0432`.
2019-12-06T12:58:30.2103051Z error: could not compile `rustc_parse`.
---
2019-12-06T12:59:12.4196356Z   local time: Fri Dec  6 12:59:12 UTC 2019
2019-12-06T12:59:12.5756832Z   network time: Fri, 06 Dec 2019 12:59:12 GMT
2019-12-06T12:59:12.5758443Z == end clock drift check ==
2019-12-06T12:59:13.6163453Z 
2019-12-06T12:59:13.6271267Z ##[error]Bash exited with code '1'.
2019-12-06T12:59:13.6301293Z ##[section]Starting: Checkout
2019-12-06T12:59:13.6302968Z ==============================================================================
2019-12-06T12:59:13.6303039Z Task         : Get sources
2019-12-06T12:59:13.6303086Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@Mark-Simulacrum
Copy link
Member

I am opposed to just using strings here -- that seems really error prone, and also harder to search for in tidy and the like. I would prefer that we add a ErrorCode(pub usize) struct to librustc_errors and use that everywhere (i.e., ErrorCode(303)). We should strive to avoid strings in any location where there is a more canonical type, or just not do this at all for now.

@GuillaumeGomez
Copy link
Member Author

@Mark-Simulacrum It comes after a discussion with @eddyb and @Centril. I guess I'll let them explain in details why. ;)

@eddyb
Copy link
Member

eddyb commented Dec 6, 2019

I am opposed to doing anything here unless explicitly sanctioned by @rust-lang/wg-diagnostics and I apologize if there was any confusion about this on Discord.

@estebank
Copy link
Contributor

estebank commented Dec 6, 2019

Even if we wanted to stringify these (and I don't think we do), the diagnostic macros could accept the code without having to surround it with quotes.

$crate::diagnostic_used!($code);
$session.span_fatal_with_code(
$span,
&format!($($message)*),
$crate::errors::DiagnosticId::Error(stringify!($code).to_owned()),
$crate::errors::DiagnosticId::Error($code.to_owned()),
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems like an unnecessary change; we can keep the identifiers and continue using stringify!($code). The only thing that's important from a recompilation POV is that diagnostic_used!(...) be removed or that if it exists for the purposes of tidy, that it shouldn't do anything (i.e. have an empty expansion).

@Centril
Copy link
Contributor

Centril commented Dec 6, 2019

The only thing that's important to me here is that we remove the dependency of librustc_error_codes from librustc_parser and so on. Otherwise you have to recompile everything every time you want to add a new error code. (I guess that doesn't matter if we have frozen new error code addition.)

@Centril
Copy link
Contributor

Centril commented Dec 6, 2019

I would prefer that we add a ErrorCode(pub usize) struct to librustc_errors and use that everywhere (i.e., ErrorCode(303)). We should strive to avoid strings in any location where there is a more canonical type, or just not do this at all for now.

That would also be fine by me since it means that we can still remove the librustc_error_codes dependency from librustc_parser et al.

@bors
Copy link
Contributor

bors commented Dec 7, 2019

☔ The latest upstream changes (presumably #67104) made this pull request unmergeable. Please resolve the merge conflicts.

@GuillaumeGomez
Copy link
Member Author

We're evolving towards strings identifiers like "[LifetimeError.Mismatch]". Therefore, strings make more sense to prepare for this switch.

@Centril
Copy link
Contributor

Centril commented Dec 8, 2019

@GuillaumeGomez Well there seems to be a lack of consensus in favor of strings, so could we limit this PR for now to just the tidy check and removing references to librustc_error_codes in the various Cargo.tomls in this PR? We can always move to strings later.

@GuillaumeGomez
Copy link
Member Author

Euh we can't really. Currently it's using consts so we have to pick something in order to remove the dependency.

@@ -1,112 +1,112 @@
#[macro_export]
macro_rules! diagnostic_used {
($code:ident) => (
($code:expr) => (
let _ = $code;
Copy link
Contributor

Choose a reason for hiding this comment

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

@GuillaumeGomez I don't think this macro has any function at all before or after the PR. These constants are all marked as pub inside librustc_error_codes so they will not be deemed unused if diagnostic_used!(...); is missing. Therefore the macro can be removed wholesale and any calls to it. Also, all you've done here is to replace stringify!($some_ident) with string literals instead. Keeping the identifiers and continuing to use stringify!(...) should work just as well.

@Dylan-DPC-zz Dylan-DPC-zz added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 13, 2020
@JohnTitor
Copy link
Member

Ping from triage: @GuillaumeGomez this PR is idle for a month, could you address Centril's review?

@GuillaumeGomez
Copy link
Member Author

I need to update it, indeed!

@GuillaumeGomez GuillaumeGomez force-pushed the make-err-codes-into-strings branch from c866e5f to 4a18062 Compare January 27, 2020 13:02
@rust-highfive
Copy link
Collaborator

The job mingw-check of your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
2020-01-27T13:02:40.2084404Z ========================== Starting Command Output ===========================
2020-01-27T13:02:40.2085929Z [command]/bin/bash --noprofile --norc /home/vsts/work/_temp/4927cc25-3f67-48d7-af51-3914b91fe529.sh
2020-01-27T13:02:40.2085964Z 
2020-01-27T13:02:40.2088578Z ##[section]Finishing: Disable git automatic line ending conversion
2020-01-27T13:02:40.2094253Z ##[section]Starting: Checkout rust-lang/rust@refs/pull/67086/merge to s
2020-01-27T13:02:40.2095966Z Task         : Get sources
2020-01-27T13:02:40.2096003Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.
2020-01-27T13:02:40.2096038Z Version      : 1.0.0
2020-01-27T13:02:40.2096073Z Author       : Microsoft
---
2020-01-27T13:02:41.0852707Z ##[command]git remote add origin https://github.com/rust-lang/rust
2020-01-27T13:02:41.0958218Z ##[command]git config gc.auto 0
2020-01-27T13:02:41.1006735Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2020-01-27T13:02:41.1060179Z ##[command]git config --get-all http.proxy
2020-01-27T13:02:41.1193799Z ##[command]git -c http.extraheader="AUTHORIZATION: basic ***" fetch --force --tags --prune --progress --no-recurse-submodules --depth=2 origin +refs/heads/*:refs/remotes/origin/* +refs/pull/67086/merge:refs/remotes/pull/67086/merge
---
2020-01-27T13:09:23.9763850Z     Checking rustc_builtin_macros v0.0.0 (/checkout/src/librustc_builtin_macros)
2020-01-27T13:09:43.3711091Z error: unused variable: `error_code`
2020-01-27T13:09:43.3712445Z    --> src/librustc/infer/error_reporting/need_type_info.rs:218:9
2020-01-27T13:09:43.3712954Z     |
2020-01-27T13:09:43.3713458Z 218 |         error_code: TypeAnnotationNeeded,
2020-01-27T13:09:43.3714015Z     |         ^^^^^^^^^^ help: consider prefixing with an underscore: `_error_code`
2020-01-27T13:09:43.3714984Z     = note: `-D unused-variables` implied by `-D warnings`
2020-01-27T13:09:43.3715210Z 
2020-01-27T13:10:01.0627419Z error: aborting due to previous error
2020-01-27T13:10:01.0628235Z 
2020-01-27T13:10:01.0628235Z 
2020-01-27T13:10:01.0983289Z error: could not compile `rustc`.
2020-01-27T13:10:01.0984101Z 
2020-01-27T13:10:01.0984654Z To learn more, run the command again with --verbose.
2020-01-27T13:10:01.1020212Z command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "check" "-Zconfig-profile" "--target" "x86_64-unknown-linux-gnu" "-Zbinary-dep-depinfo" "-j" "2" "--release" "--color" "always" "--features" " llvm" "--manifest-path" "/checkout/src/rustc/Cargo.toml" "--message-format" "json-render-diagnostics"
2020-01-27T13:10:01.1030768Z failed to run: /checkout/obj/build/bootstrap/debug/bootstrap check
2020-01-27T13:10:01.1031558Z Build completed unsuccessfully in 0:04:17
2020-01-27T13:10:01.1084210Z == clock drift check ==
2020-01-27T13:10:01.1101802Z   local time: Mon Jan 27 13:10:01 UTC 2020
2020-01-27T13:10:01.1101802Z   local time: Mon Jan 27 13:10:01 UTC 2020
2020-01-27T13:10:01.6788722Z   network time: Mon, 27 Jan 2020 13:10:01 GMT
2020-01-27T13:10:01.6790603Z == end clock drift check ==
2020-01-27T13:10:02.0812113Z 
2020-01-27T13:10:02.0902540Z ##[error]Bash exited with code '1'.
2020-01-27T13:10:02.0936347Z ##[section]Finishing: Run build
2020-01-27T13:10:02.0952687Z ##[section]Starting: Checkout rust-lang/rust@refs/pull/67086/merge to s
2020-01-27T13:10:02.0954553Z Task         : Get sources
2020-01-27T13:10:02.0954604Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.
2020-01-27T13:10:02.0954778Z Version      : 1.0.0
2020-01-27T13:10:02.0954822Z Author       : Microsoft
2020-01-27T13:10:02.0954822Z Author       : Microsoft
2020-01-27T13:10:02.0954890Z Help         : [More Information](https://go.microsoft.com/fwlink/?LinkId=798199)
2020-01-27T13:10:02.0954944Z ==============================================================================
2020-01-27T13:10:02.4885320Z Cleaning any cached credential from repository: rust-lang/rust (GitHub)
2020-01-27T13:10:02.4924964Z ##[section]Finishing: Checkout rust-lang/rust@refs/pull/67086/merge to s
2020-01-27T13:10:02.5039699Z Cleaning up task key
2020-01-27T13:10:02.5040800Z Start cleaning up orphan processes.
2020-01-27T13:10:02.5342040Z Terminate orphan process: pid (3918) (python)
2020-01-27T13:10:02.5364386Z ##[section]Finishing: Finalize Job

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@GuillaumeGomez GuillaumeGomez force-pushed the make-err-codes-into-strings branch from 4a18062 to 0c5462b Compare January 27, 2020 16:12
@GuillaumeGomez
Copy link
Member Author

Updated: however one issue remains: since error codes are now strings, the JSON rendering changes. How can I implement RustcEncodable on librustc_errors::DiagnosticCode by hand so the code field doesn't get handled like a string? I didn't find any "manual" implementation of it...

@Centril
Copy link
Contributor

Centril commented Jan 27, 2020

I fixed the dependency issue in #68353. Beyond that, it's not really clear to me what benefits the stringification brings, and there seems to be a lack of consensus in favor of that change for now. So I'll r? @Mark-Simulacrum @oli-obk @estebank.

@eddyb
Copy link
Member

eddyb commented Jan 27, 2020

@GuillaumeGomez That's because the trait is named Encodable.

@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-llvm-7 of your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
2020-01-27T16:13:38.6731696Z ========================== Starting Command Output ===========================
2020-01-27T16:13:38.6733136Z [command]/bin/bash --noprofile --norc /home/vsts/work/_temp/07f670d6-5a33-4e83-b35d-cef237c46c06.sh
2020-01-27T16:13:38.6733167Z 
2020-01-27T16:13:38.6735482Z ##[section]Finishing: Disable git automatic line ending conversion
2020-01-27T16:13:38.6741072Z ##[section]Starting: Checkout rust-lang/rust@refs/pull/67086/merge to s
2020-01-27T16:13:38.6742626Z Task         : Get sources
2020-01-27T16:13:38.6742661Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.
2020-01-27T16:13:38.6742695Z Version      : 1.0.0
2020-01-27T16:13:38.6742778Z Author       : Microsoft
---
2020-01-27T16:13:39.6930443Z ##[command]git remote add origin https://github.com/rust-lang/rust
2020-01-27T16:13:39.6941759Z ##[command]git config gc.auto 0
2020-01-27T16:13:39.6944128Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2020-01-27T16:13:39.6945832Z ##[command]git config --get-all http.proxy
2020-01-27T16:13:39.6951950Z ##[command]git -c http.extraheader="AUTHORIZATION: basic ***" fetch --force --tags --prune --progress --no-recurse-submodules --depth=2 origin +refs/heads/*:refs/remotes/origin/* +refs/pull/67086/merge:refs/remotes/pull/67086/merge

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@GuillaumeGomez
Copy link
Member Author

@Centril It appeared that error codes are used by people. So a solution to improve their usability was to allow to have "error codes" looking like "lifetime-mismatch" instead of "E0XXX". If we handle error codes with strings instead of enums, we can then use them however we want.

@Centril
Copy link
Contributor

Centril commented Jan 27, 2020

@GuillaumeGomez I mean, it could just as well look like lifetime_mismatch (still :ident), or lifetime::mismatch (:path).

@GuillaumeGomez
Copy link
Member Author

Rereading from the conversation, I'm not even sure that @rust-lang/wg-diagnostics came to a conclusion on this. So let's wait to hear from them?

Or maybe you had some news on your side @eddyb ?

@eddyb
Copy link
Member

eddyb commented Jan 27, 2020

Or maybe you had some news on your side @eddyb ?

I am not involved in anything where this came up since our Discord discussion, so no.

@Centril
Copy link
Contributor

Centril commented Jan 27, 2020

Note that @rust-lang/wg-diagnostics is not really a group that holds regular meetings, has a well-defined membership, and whatnot. For now it's just @oli-obk, @estebank, @davidtwco, and myself.

@oli-obk
Copy link
Contributor

oli-obk commented Jan 27, 2020

I think moving away from numeric error codes is a good thing, I do worry about the checks for duplicate use and undocumented errors though. It seems the current impl is very hardcoded to things starting with "E0 without the error emitters checking that you actually name your error codes this way

@Mark-Simulacrum
Copy link
Member

@Centril noted elsewhere (not sure where) that the dependency breaking part of this has already been done. It is not clear to me that this PR makes things better wrt to stringification, as a result of that. I think my earlier comment (#67086 (comment)) still stands.

Splitting out the unused/definitely used checking code into a separate PR (or making that the only change in this PR) seems good, in theory, though I have not reviewed whether:

  • we need that code
  • it is (in fact) correct

@GuillaumeGomez
Copy link
Member Author

Ok, let's wait and see then!

@GuillaumeGomez
Copy link
Member Author

Got the comments after commenting...

@Mark-Simulacrum I can send the check for the double error codes usage in another PR if you want?

@Mark-Simulacrum
Copy link
Member

Sure, that'd be great.

@GuillaumeGomez
Copy link
Member Author

@oli-obk We can store the error codes inside an hashmap and make this check when throwing one, and then we could use this hashmap as reference for the possible duplicates? (even though I'm not completely sure how to correctly handle it, i.e. differentiating "normal" strings and error codes)

@GuillaumeGomez
Copy link
Member Author

Sure, that'd be great.

@Mark-Simulacrum Ok, sending the PR this evening or tomorrow.

@bors
Copy link
Contributor

bors commented Jan 29, 2020

☔ The latest upstream changes (presumably #68635) made this pull request unmergeable. Please resolve the merge conflicts.

@GuillaumeGomez GuillaumeGomez force-pushed the make-err-codes-into-strings branch from 0c5462b to 397e941 Compare February 21, 2020 13:45
@GuillaumeGomez
Copy link
Member Author

ping @oli-obk

@rust-highfive
Copy link
Collaborator

The job mingw-check of your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
2020-02-21T13:46:01.1096555Z ========================== Starting Command Output ===========================
2020-02-21T13:46:01.1101771Z [command]/bin/bash --noprofile --norc /home/vsts/work/_temp/8a258190-a660-438b-8245-92a5f549e2ab.sh
2020-02-21T13:46:01.1103018Z 
2020-02-21T13:46:01.1108237Z ##[section]Finishing: Disable git automatic line ending conversion
2020-02-21T13:46:01.1123221Z ##[section]Starting: Checkout rust-lang/rust@refs/pull/67086/merge to s
2020-02-21T13:46:01.1125928Z Task         : Get sources
2020-02-21T13:46:01.1126145Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.
2020-02-21T13:46:01.1126355Z Version      : 1.0.0
2020-02-21T13:46:01.1126495Z Author       : Microsoft
---
2020-02-21T13:46:02.3730369Z ##[command]git remote add origin https://github.com/rust-lang/rust
2020-02-21T13:46:02.3738499Z ##[command]git config gc.auto 0
2020-02-21T13:46:02.3744845Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2020-02-21T13:46:02.3751289Z ##[command]git config --get-all http.proxy
2020-02-21T13:46:02.3761168Z ##[command]git -c http.extraheader="AUTHORIZATION: basic ***" fetch --force --tags --prune --progress --no-recurse-submodules --depth=2 origin +refs/heads/*:refs/remotes/origin/* +refs/pull/67086/merge:refs/remotes/pull/67086/merge
---
2020-02-21T13:48:46.6798874Z Caused by:
2020-02-21T13:48:46.6799192Z   could not parse input as TOML
2020-02-21T13:48:46.6799497Z 
2020-02-21T13:48:46.6799768Z Caused by:
2020-02-21T13:48:46.6800124Z   unexpected character found: `<` at line 3858 column 1
2020-02-21T13:48:46.6806546Z Build completed unsuccessfully in 0:00:10
2020-02-21T13:48:46.6855634Z == clock drift check ==
2020-02-21T13:48:46.6864780Z   local time: Fri Feb 21 13:48:46 UTC 2020
2020-02-21T13:48:46.9754761Z   network time: Fri, 21 Feb 2020 13:48:46 GMT
2020-02-21T13:48:46.9754761Z   network time: Fri, 21 Feb 2020 13:48:46 GMT
2020-02-21T13:48:46.9759424Z == end clock drift check ==
2020-02-21T13:48:54.5065372Z 
2020-02-21T13:48:54.5137519Z ##[error]Bash exited with code '1'.
2020-02-21T13:48:54.5152423Z ##[section]Finishing: Run build
2020-02-21T13:48:54.5193812Z ##[section]Starting: Checkout rust-lang/rust@refs/pull/67086/merge to s
2020-02-21T13:48:54.5197821Z Task         : Get sources
2020-02-21T13:48:54.5198100Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.
2020-02-21T13:48:54.5198548Z Version      : 1.0.0
2020-02-21T13:48:54.5198896Z Author       : Microsoft
2020-02-21T13:48:54.5198896Z Author       : Microsoft
2020-02-21T13:48:54.5199180Z Help         : [More Information](https://go.microsoft.com/fwlink/?LinkId=798199)
2020-02-21T13:48:54.5199525Z ==============================================================================
2020-02-21T13:48:54.7985655Z Cleaning any cached credential from repository: rust-lang/rust (GitHub)
2020-02-21T13:48:54.8028734Z ##[section]Finishing: Checkout rust-lang/rust@refs/pull/67086/merge to s
2020-02-21T13:48:54.8097353Z Cleaning up task key
2020-02-21T13:48:54.8098504Z Start cleaning up orphan processes.
2020-02-21T13:48:54.8251929Z Terminate orphan process: pid (4442) (python)
2020-02-21T13:48:54.8365651Z ##[section]Finishing: Finalize Job

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@bors
Copy link
Contributor

bors commented Feb 26, 2020

☔ The latest upstream changes (presumably #61812) made this pull request unmergeable. Please resolve the merge conflicts.

@oli-obk
Copy link
Contributor

oli-obk commented Feb 26, 2020

Reading the discussion on zulip (https://rust-lang.zulipchat.com/#narrow/stream/147480-t-compiler.2Fwg-diagnostics/topic/get.20rid.20of.20error.20codes/near/187901869) I don't think we're ready to do this right now. What we definitely don't want is to have quoted strings in the macro, since any non-numeric names will just be snake case names and thus valid idents that can be stringified internally.

So basically

  • this PR should not be touching any call-sites of the error creation macros
  • in the future we may want to accept error codes that are not in the E0XXX format
  • we may want to have a translation step in the macros allowing us to use the numeric and named error codes interchangeably

@@ -3855,7 +3855,10 @@ dependencies = [
"rustc_ast_pretty",
"rustc_attr",
"rustc_data_structures",
<<<<<<< HEAD
Copy link
Contributor

Choose a reason for hiding this comment

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

Bad merge

@joelpalmer
Copy link

Triaged

@joelpalmer joelpalmer added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 30, 2020
@GuillaumeGomez GuillaumeGomez deleted the make-err-codes-into-strings branch August 19, 2024 12:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.