Page MenuHomePhabricator

joerg (Joerg Sonnenberger)
User

Projects

User does not belong to any projects.

User Details

User Since
Nov 23 2012, 10:16 AM (351 w, 4 d)

Recent Activity

Wed, Aug 7

joerg accepted D65762: lit: Use a License classifier that pypi will accept.

Thanks, this looks good enough within the limitations of the framework.

Wed, Aug 7, 1:20 PM · Restricted Project

Mon, Aug 5

joerg added a comment to D65280: Add a pass to lower is.constant and objectsize intrinsics.

Looking a bit more into the details. Chandler, you've originally suggested going with the LowerAtomic route and that actually does create code that fails the SDAG lowering if the pass is skipped, e.g. on ARM.
The second part is currently overlapping with the CodeGenPrepare pass. I can cleanup the implementation somewhat by reusing the same functionality as that pass is using OR I could factor out a minimal branch for doing the constant folding optimization from CodeGenPrepare as a general branch that is included in the pass chain for -O0, e.g. instead of the more general CodeGenPrepare pass. The main difference is that the non-optimized build would not get any recursive folding from PHI-simplification, but I think that's fine for the original use case. It would also not get the block merging, but again, that seems to be fine for the constraints.

Mon, Aug 5, 6:55 AM · Restricted Project

Sun, Aug 4

joerg added a comment to D65280: Add a pass to lower is.constant and objectsize intrinsics.

For the first part, I was actually asked to do that. I don't mind either way.

Sun, Aug 4, 7:31 AM · Restricted Project

Sat, Aug 3

joerg updated the diff for D65280: Add a pass to lower is.constant and objectsize intrinsics.

Generalize slightly to also cover llvm.objectsize which has very similar constraints.

Sat, Aug 3, 1:06 PM · Restricted Project

Fri, Aug 2

joerg added a comment to D60943: Delay diagnosing asm constraints that require immediates until after inlining.

The combination of D60942, D06943 and D65280 solves the problems for me on all targets I have.

Fri, Aug 2, 5:31 AM · Restricted Project, Restricted Project

Thu, Aug 1

joerg updated the diff for D65280: Add a pass to lower is.constant and objectsize intrinsics.

Update PHI nodes in disconnected block.

Thu, Aug 1, 11:34 AM · Restricted Project

Tue, Jul 30

joerg updated the diff for D65280: Add a pass to lower is.constant and objectsize intrinsics.

Replace boolean argument to replaceAndRecursivelySimplify with optional vector of un-modified instructions. This simplifies the API change significantly and allows other potential use cases. Redo the restart handling. After a successful simplification step, restart the current BB only, but always do another full pass of the function.

Tue, Jul 30, 7:46 AM · Restricted Project

Mon, Jul 29

joerg added inline comments to D64963: Add a pass to lower is.constant intrinsics.
Mon, Jul 29, 3:08 PM · Restricted Project
joerg added inline comments to D65280: Add a pass to lower is.constant and objectsize intrinsics.
Mon, Jul 29, 3:01 PM · Restricted Project
joerg added inline comments to D64963: Add a pass to lower is.constant intrinsics.
Mon, Jul 29, 2:45 PM · Restricted Project

Sat, Jul 27

joerg updated the diff for D65280: Add a pass to lower is.constant and objectsize intrinsics.

Avoid goto. Create new BranchInst instead of modifying in-place. Update tests to reflect changes. Move most of the x86 is-constant test to generic.

Sat, Jul 27, 1:22 PM · Restricted Project
joerg accepted D60942: Emit diagnostic if an inline asm constraint requires an immediate.

LGTM otherwise.

Sat, Jul 27, 1:13 PM · Restricted Project
joerg added a comment to D60942: Emit diagnostic if an inline asm constraint requires an immediate.

With the exception of the small improvements outlined, this looks good to me.

Sat, Jul 27, 12:01 PM · Restricted Project
joerg committed rG791951bd32ac: Stricter check for the memory access. (authored by joerg).
Stricter check for the memory access.
Sat, Jul 27, 11:59 AM
joerg committed rL367180: Stricter check for the memory access..
Stricter check for the memory access.
Sat, Jul 27, 11:58 AM
joerg added a comment to D60943: Delay diagnosing asm constraints that require immediates until after inlining.

I understand, but the current version just doesn't work anyway to delay it.

Sat, Jul 27, 11:55 AM · Restricted Project, Restricted Project

Fri, Jul 26

joerg added inline comments to D60942: Emit diagnostic if an inline asm constraint requires an immediate.
Fri, Jul 26, 7:44 AM · Restricted Project

Thu, Jul 25

joerg created D65280: Add a pass to lower is.constant and objectsize intrinsics.
Thu, Jul 25, 6:54 AM · Restricted Project
joerg abandoned D62025: Expand llvm.is.constant earlier.
Thu, Jul 25, 6:47 AM · Restricted Project

Wed, Jul 24

joerg added a comment to D60943: Delay diagnosing asm constraints that require immediates until after inlining.

You lost the changes to lib/Sema/SemaStmtAsm.cpp to actually do the delaying for immediate operands?

Wed, Jul 24, 3:33 PM · Restricted Project, Restricted Project

Jul 19 2019

joerg added inline comments to D64963: Add a pass to lower is.constant intrinsics.
Jul 19 2019, 5:38 AM · Restricted Project

Jun 7 2019

joerg committed rGb2e96169b0a4: [NFC] Don't export helpers of ConstantFoldCall (authored by joerg).
[NFC] Don't export helpers of ConstantFoldCall
Jun 7 2019, 6:27 AM
joerg committed rL362799: [NFC] Don't export helpers of ConstantFoldCall.
[NFC] Don't export helpers of ConstantFoldCall
Jun 7 2019, 6:27 AM

Jun 5 2019

joerg added a comment to D59744: Fix i386 ABI "__m64" type bug.

I think MMX code is obscure enough at this point that it doesn't matter much either way. Even less across DSO boundaries. That's why I don't really care either way.

Jun 5 2019, 1:22 PM · Restricted Project, Restricted Project
joerg added a comment to D60942: Emit diagnostic if an inline asm constraint requires an immediate.

The patch still needs to be generalized to handle all ICE constraints, but it is not blocked by the review for handling the is.constant intrinsic.

Jun 5 2019, 6:06 AM · Restricted Project

May 30 2019

joerg added inline comments to D60748: Fix i386 struct and union parameter alignment.
May 30 2019, 4:50 AM · Restricted Project

May 16 2019

joerg created D62025: Expand llvm.is.constant earlier.
May 16 2019, 12:25 PM · Restricted Project
joerg committed rGec6ee797ec18: Fix typos in comment. (authored by joerg).
Fix typos in comment.
May 16 2019, 11:01 AM
joerg committed rL360921: Fix typos in comment..
Fix typos in comment.
May 16 2019, 11:01 AM

May 13 2019

joerg added a comment to D60942: Emit diagnostic if an inline asm constraint requires an immediate.

On the frontend side:

  • __builtin_constant_p is not expanded correctly in EvalConstant.cpp when used as part of codegen for -O0. This means that trivially unreachable branches are emitted (e.g. with a BR with a literal false as condition),
  • The constant evaluator has a general problem with overly pessimistic analysis when it comes to possible side effects. Still have to discuss the correct handling with Richard for this.

GCC has this to say about __builtin_constant_p's behavior:

A return of 0 does not indicate that the value is not a constant, but merely that GCC cannot prove it is a constant with the specified value of the -O option.

When I was modifying our implementation recently, I made it so that if there are no optimizations then we evaluate to 0 in the front-end before code-gen. It might be that those blocks are being emitted, but they should have something like br i1 false, label %true_bb, label %false_bb or equivalent.

May 13 2019, 6:11 PM · Restricted Project
joerg added a comment to D60942: Emit diagnostic if an inline asm constraint requires an immediate.

On the frontend side:

  • __builtin_constant_p is not expanded correctly in EvalConstant.cpp when used as part of codegen for -O0. This means that trivially unreachable branches are emitted (e.g. with a BR with a literal false as condition),
  • The constant evaluator has a general problem with overly pessimistic analysis when it comes to possible side effects. Still have to discuss the correct handling with Richard for this.
May 13 2019, 4:54 PM · Restricted Project

Apr 24 2019

joerg committed rG8372b467f18d: [PowerPC] Allow using initial-exec TLS with PIC (authored by joerg).
[PowerPC] Allow using initial-exec TLS with PIC
Apr 24 2019, 3:11 PM
joerg committed rL359146: [PowerPC] Allow using initial-exec TLS with PIC.
[PowerPC] Allow using initial-exec TLS with PIC
Apr 24 2019, 3:11 PM
joerg closed D61026: Fix initial-exec in PIC mode for PPC32.
Apr 24 2019, 3:10 PM · Restricted Project
joerg added a comment to D60942: Emit diagnostic if an inline asm constraint requires an immediate.

It does not fix the issues on our side, but pushes them to a different place. It is still an improvement, but the problem is not solved yet.

Apr 24 2019, 1:43 PM · Restricted Project

Apr 23 2019

joerg created D61026: Fix initial-exec in PIC mode for PPC32.
Apr 23 2019, 9:26 AM · Restricted Project
joerg committed rG6e7cc49d5cb3: [SPARC] Use the correct register set for the "r" asm constraint. (authored by joerg).
[SPARC] Use the correct register set for the "r" asm constraint.
Apr 23 2019, 8:16 AM
joerg committed rL358998: [SPARC] Use the correct register set for the "r" asm constraint..
[SPARC] Use the correct register set for the "r" asm constraint.
Apr 23 2019, 8:13 AM

Apr 22 2019

joerg added a comment to D60943: Delay diagnosing asm constraints that require immediates until after inlining.

I'm in the process of testing this, but feedback will take a bit.

Apr 22 2019, 4:49 AM · Restricted Project, Restricted Project

Mar 27 2019

joerg added inline comments to D59816: [Support] Implement zlib independent crc32 computation.
Mar 27 2019, 6:42 AM · Restricted Project

Mar 26 2019

joerg added a comment to D59816: [Support] Implement zlib independent crc32 computation.

don't bother with the 1KB table in both binary and source, just recompute it on the first use

Is single threaded usage guaranteed?

Mar 26 2019, 8:55 AM · Restricted Project
joerg added a comment to D59816: [Support] Implement zlib independent crc32 computation.

I'd recomment copying the version from libarchive (https://github.com/libarchive/libarchive/blob/master/libarchive/archive_crc32.h):

  • don't bother with the 1KB table in both binary and source, just recompute it on the first use.
  • at least in the past unrolling the inner loop somewhat helped a lot
Mar 26 2019, 6:19 AM · Restricted Project

Mar 11 2019

joerg accepted D28791: [compiler-rt][crt] Simple crtbegin and crtend implementation.

LGTM from my perspective at least.

Mar 11 2019, 2:18 PM · Restricted Project, Restricted Project
joerg added a comment to D59118: creduce script for clang crashes.

For it to be really useful for the majority of bugs, it would be nice to figure out automatically how to get the preprocessing step done and filter out the # lines afterwards. That part alone significantly cuts down the creduce time.

Mar 11 2019, 2:10 PM · Restricted Project
joerg added a comment to D58250: [AIX][CMake] Changes for building on AIX with XL and GCC.

Why do you exclude clang? I would expect LLVM at this point to not support visibility attributes on XCOFF either?

Mar 11 2019, 1:30 PM · Restricted Project

Mar 5 2019

joerg added a comment to D58821: Inline asm constraints: allow ICE-like pointers for the "n" constraint (PR40890).

Well, that was a sample to illustrate the point. A full working (and now failing) example is:

Mar 5 2019, 5:50 AM · Restricted Project

Mar 4 2019

joerg added a comment to D58821: Inline asm constraints: allow ICE-like pointers for the "n" constraint (PR40890).

The other problem is that we don't use the CFG machinery to prune dead branches. Consider the x86 in/out instructions: one variant takes an immediate, the other a register. The classic way to deal with that is something like

Mar 4 2019, 3:39 PM · Restricted Project

Mar 2 2019

joerg added a comment to D58821: Inline asm constraints: allow ICE-like pointers for the "n" constraint (PR40890).

Can you include a patch for something like (int *)0xdeadbeeeeeef on amd64? That's a valid value for "n", but clearly too large for int. Thanks for looking at this, it is one of the two large remaining show stoppers for the asm constraint check.

Mar 2 2019, 12:44 PM · Restricted Project
joerg added a comment to D56828: [ELF] Simplify RelRo, TLS, NOBITS section ranks and make RW PT_LOAD start with RelRo.

Placing .bss.rel.ro before .data doesn't make sense. It forces the content of .bss.rel.ro to embedded into the binary. I also don't really understand the motivation here. Memory mappins on the kernel side are quite cheap.

Mar 2 2019, 10:19 AM · Restricted Project

Feb 28 2019

joerg committed rG01530291eafd: [PPC] Secure PLT only has meaning for PIC (authored by joerg).
[PPC] Secure PLT only has meaning for PIC
Feb 28 2019, 3:34 PM
joerg committed rL355154: [PPC] Secure PLT only has meaning for PIC.
[PPC] Secure PLT only has meaning for PIC
Feb 28 2019, 3:34 PM
joerg added inline comments to D58649: Fix inline assembler constraint validation.
Feb 28 2019, 5:22 AM · Restricted Project

Feb 27 2019

joerg committed rGa9488fbebf94: Ensure that set constrained asm operands are not affected by truncation. (authored by joerg).
Ensure that set constrained asm operands are not affected by truncation.
Feb 27 2019, 4:55 PM
joerg committed rL355058: Ensure that set constrained asm operands are not affected by truncation..
Ensure that set constrained asm operands are not affected by truncation.
Feb 27 2019, 4:54 PM
joerg committed rC355058: Ensure that set constrained asm operands are not affected by truncation..
Ensure that set constrained asm operands are not affected by truncation.
Feb 27 2019, 4:54 PM
joerg committed rG6a198366a0cc: Default to Secure PLT on PPC for NetBSD and OpenBSD. This matches the default… (authored by joerg).
Default to Secure PLT on PPC for NetBSD and OpenBSD. This matches the default…
Feb 27 2019, 1:56 PM
joerg committed rL355038: Default to Secure PLT on PPC for NetBSD and OpenBSD..
Default to Secure PLT on PPC for NetBSD and OpenBSD.
Feb 27 2019, 1:52 PM
joerg committed rGb4a9d3e83e58: Use Secure PLT as default on NetBSD/PowerPC. (authored by joerg).
Use Secure PLT as default on NetBSD/PowerPC.
Feb 27 2019, 1:48 PM
joerg committed rC355033: Use Secure PLT as default on NetBSD/PowerPC..
Use Secure PLT as default on NetBSD/PowerPC.
Feb 27 2019, 1:45 PM
joerg committed rL355033: Use Secure PLT as default on NetBSD/PowerPC..
Use Secure PLT as default on NetBSD/PowerPC.
Feb 27 2019, 1:45 PM

Feb 26 2019

joerg committed rG49ef2a4acdbb: Fix inline assembler constraint validation (authored by joerg).
Fix inline assembler constraint validation
Feb 26 2019, 4:41 PM
joerg committed rC354937: Fix inline assembler constraint validation.
Fix inline assembler constraint validation
Feb 26 2019, 4:41 PM
joerg committed rL354937: Fix inline assembler constraint validation.
Fix inline assembler constraint validation
Feb 26 2019, 4:41 PM
joerg closed D58649: Fix inline assembler constraint validation.
Feb 26 2019, 4:40 PM · Restricted Project

Feb 25 2019

joerg created D58649: Fix inline assembler constraint validation.
Feb 25 2019, 3:23 PM · Restricted Project

Feb 24 2019

joerg added a comment to D58592: [clang] [ToolChains/NetBSD] Support relative libc++ header path.

I'm not in favor of this. It adds overhead for the system compiler and generally makes the logic more complicated. This seems to be another hack around the fact that the driver has no clear notion of "use system runtime" vs "use custom runtime".

Feb 24 2019, 9:24 AM · Restricted Project, Restricted Project

Feb 19 2019

joerg added a comment to D58379: [compiler-rt] Intercept the bcmp() function..

Actually, since it used to be part of XSI, I would include it unconditionally and only conditionalize it if a system starts to actually drop it.

Feb 19 2019, 9:09 AM · Restricted Project, Restricted Project

Feb 5 2019

joerg added a comment to D57749: [LLD][ELF] - Set DF_STATIC_TLS flag for i386 target..

The description is certainly not right. IE can be used from dlopen'd objects without problems as long as there is still reserved space around and the data is not initialized non-trivially or the dynamic linker is willing to fix up all threads. The point of the flag is to allow the dynamic linker to make a quick decision without having to scan the relocation table first. A classic user of this was libGL. It also applies to many platforms, not just i386. As such, please update the description before any commit.

Feb 5 2019, 11:05 AM · Restricted Project

Jan 30 2019

joerg added a comment to D57385: [ELF] Support --{,no-}allow-shlib-undefined.

Neither GNU ld nor gold implement really useful behavior here. Ideally, every library would be linked with -z defs, but that normally doesn't work because on many systems certain symbols are imported from the main program by default. A good example are the entry points of the dynamic linker. A bad example historically speaking is libwrap. From a user perspective it would be desirable to have a way to say "This symbol will be supplied by someone else" when linking a shared library or potentially even the main binary. That would allow whitelisting intentionally undefined symbols and allow avoiding any symbol resolution dance for shared libraries. A single pass over the symbol tables is still necessary for the sake of knowing the defined symbols and for potential copy relocations, but no further work should be needed.

Jan 30 2019, 6:05 AM · Restricted Project

Jan 25 2019

joerg added a comment to D57143: [builtins] Rounding mode support for addxf3/subxf3.

This seems to imply hard-float support though? Accessing floating point registers when using soft-float is not going to get very far.

Jan 25 2019, 4:27 PM · Restricted Project, Restricted Project

Jan 21 2019

joerg added a comment to D56975: [Support] Reimplement getMainExecutable() using sysctl on NetBSD.

I'd really prefer to keep the argv[0] code as is. I'm not sure what that test case is supposed to do, but it seems quite questionable as "check" is not a valid language frontend nor a version suffix. It should not work.

Jan 21 2019, 1:26 AM · Restricted Project

Jan 15 2019

joerg added a comment to D28791: [compiler-rt][crt] Simple crtbegin and crtend implementation.

For the clang side, I don't understand why Driver::GetFilePath is not good enough. This shouldn't need all toolchain changes at all.

Jan 15 2019, 8:16 AM · Restricted Project, Restricted Project
joerg added a comment to D49466: Initial implementation of -fmacro-prefix-map and -ffile-prefix-map.

As discussed with dankm on IRC, I still would like to see the correct behavior going into 8.0, i.e. not change it later. Since this also matters for potential faster implementations later, it seems like a good idea to do it now. The changes are well-localized.

Jan 15 2019, 4:41 AM · Restricted Project, Restricted Project

Jan 14 2019

joerg added a comment to D56647: [WIP] [ELF] Implement --copy-dt-needed-entries.

As first step, it goes into the right direction. I would explicitly set --as-needed for all those indirectly loaded objects. If people want to retain the questionable behavior of newer GNU tools, it could be a separate flag so that a final round can warn if an indirectly pulled library is necessary, but that behavior doesn't IMO make much sense. Full version has to look at DT_RUNPATH/DT_RPATH and also --rpath-link.

Jan 14 2019, 1:10 PM · Restricted Project

Jan 11 2019

joerg added a comment to D56215: [lld] [ELF] Include default search paths for NetBSD driver.

Right, I'm not aware of anyone but FreeBSD really using the OSABI field. For FreeBSD it was a long standing hack around limitations in the ELF kernel loader. I'm not even sure if FreeBSD use(d) to set the OSABI field for the intermediate object files.

Jan 11 2019, 1:24 PM · lld
joerg added a comment to D49466: Initial implementation of -fmacro-prefix-map and -ffile-prefix-map.

That's the other reason why I find the GCC specification as string prefix confusing. I still say we should just go with mapping of path names and then the order question mostly goes away.

Jan 11 2019, 6:55 AM · Restricted Project, Restricted Project
joerg added inline comments to D56215: [lld] [ELF] Include default search paths for NetBSD driver.
Jan 11 2019, 5:57 AM · lld

Jan 8 2019

joerg added inline comments to D56215: [lld] [ELF] Include default search paths for NetBSD driver.
Jan 8 2019, 1:48 PM · lld
joerg added a comment to D56215: [lld] [ELF] Include default search paths for NetBSD driver.

@ruiu: No, it is exactly what you want, since it allows you to point lld into the normal sysroot. Cross-compiling is the default case for the NetBSD toolchain.

Jan 8 2019, 1:40 PM · lld
joerg added inline comments to D56215: [lld] [ELF] Include default search paths for NetBSD driver.
Jan 8 2019, 12:59 PM · lld
joerg added a comment to D56215: [lld] [ELF] Include default search paths for NetBSD driver.

Thanks, this looks like a good starting point.

Jan 8 2019, 12:59 PM · lld

Jan 4 2019

joerg added a comment to D56325: Sort symbols in .bss by size..

It can certainly make it worse, e.g. if you have one input section with half page size and page alignment followed by another input section with half page size. Depending on the precise size and order, it will fit into one page or two. It's a bit constructured, but gives the general idea.

Jan 4 2019, 2:28 PM
joerg added a comment to D56325: Sort symbols in .bss by size..

I think this should factor in any larger-than-normal alignment, otherwise it could easily result in a significant increase in size?

Jan 4 2019, 1:27 PM

Jan 3 2019

joerg added a comment to D56215: [lld] [ELF] Include default search paths for NetBSD driver.

Talking from the perspective of having had to deal with thousands of packages in pkgsrc over the years: it is naive to believe that there isn't a lot of software that calls the linker directly for various reasons. As such, it is very important to have a useful configuration in the linker by default. Such a configuration is by its very nature target specific. This doesn't mean it can't be done in a cross-compiler friendly way, on the contrary. Over the years NetBSD has been pushing its toolchain to be as similar for the native build and a cross-target as reasonable possible. Modulo the build time choices in the config.h sense, the only difference between the native and cross tools is the built-in default of the former.

Jan 3 2019, 11:16 AM · lld

Jan 2 2019

joerg added a comment to D56215: [lld] [ELF] Include default search paths for NetBSD driver.

This doesn't seem a reasonable approach at all:

(1) It breaks cross-linking.
(2) It is not correct for any target architecture, e.g. /usr/local/lib certainly doesn't belong on this list and /lib doesn't either.
(3) The correct list depends not only on the target architecture, but also the active emulation.

Is it acceptable to pass all the paths through configure/build phase of lld? It's done this way in GNU ld in the NetBSD distribution. If we need or want to hardcode all the specific paths it will be harder to maintain the proper list and behavior inside lld.

Jan 2 2019, 3:26 PM · lld
joerg added a comment to D56215: [lld] [ELF] Include default search paths for NetBSD driver.

This doesn't seem a reasonable approach at all:

Jan 2 2019, 1:34 PM · lld

Dec 27 2018

joerg added a comment to D55998: ELF: create "container" sections from PT_LOAD segments.

I foundamentally don't understand what you are trying to do. You can either look at the executable from a segment perspective or from a section perspective. But trying to mix the views is bound to give bogus results.

Dec 27 2018, 8:11 AM

Dec 24 2018

joerg added a comment to D55998: ELF: create "container" sections from PT_LOAD segments.

it is in theory possible to create an elf file where only a part of a section would belong to some segment (and another part to a different one).

ELF standard: "An object file segment contains one or more sections.", "An object file segment comprises one or more sections"

Dec 24 2018, 4:37 PM

Dec 11 2018

joerg added a comment to D43871: [modules] No longer include stdlib.h from mm_malloc.h..

This header is used on systems without glibc. So please don't argue about behavior based only on that. Granted, most other libc implementation are less annoying when it comes to free and malloc, but still.

Dec 11 2018, 1:16 PM
joerg added inline comments to D55542: [llvm-xray] Support for PIE.
Dec 11 2018, 12:55 PM

Dec 10 2018

joerg added a comment to D55167: Add a new interceptors for cdbr(3) and cdbw(3) API from NetBSD.

I don't see why this interceptor needs to know about the internals of struct cdbr or struct cdbw at all. They are fully opaque. All memory accessed by users is explicitly sized as argument to the functions or returned with the size.

Then we give up on TSAN check of fieds, or msan check if caller will try to read particular fields

Dec 10 2018, 2:49 PM · Restricted Project

Dec 9 2018

joerg added a comment to D55167: Add a new interceptors for cdbr(3) and cdbw(3) API from NetBSD.

I don't see why this interceptor needs to know about the internals of struct cdbr or struct cdbw at all. They are fully opaque. All memory accessed by users is explicitly sized as argument to the functions or returned with the size.

Dec 9 2018, 11:31 AM · Restricted Project

Dec 3 2018

joerg added a comment to D55213: Python2/3 compat - print.

I'd recomment another pass to reduce the now fully redundant () pairs around % string substitutions, but in principle LGTM.

Dec 3 2018, 5:07 AM
joerg accepted D55207: Python 2/3 compat - shebang.

LGTM

Dec 3 2018, 4:17 AM
joerg requested changes to D55199: Python2/3 compat - urllib.
Dec 3 2018, 4:16 AM
joerg accepted D55198: Python 2/3 compat - type destructuring.

LGTM

Dec 3 2018, 4:14 AM
joerg added inline comments to D55196: Python2/3 compatibility - StringIO.
Dec 3 2018, 4:10 AM
joerg accepted D55195: Python2/3 compat - exceptions.

LGTM

Dec 3 2018, 4:06 AM
joerg added a comment to D55194: Python2/3 compatiility - has_key.

LGTM

Dec 3 2018, 4:04 AM

Dec 2 2018

joerg added a comment to D55121: Make several Python scripts portable across Python2 and Python 3.

Can you split off the pure modernisation changes like new exception or print ? Those are completely non-contentious changes after all. I generally do not like the range and list related changes as many instances are clear regressions for the 2.x case. filter to list comprehension should IMO be a separate change as well, but those are much less problematic and often an improvement in terms of both performance and readability.

Dec 2 2018, 11:29 AM