Skip to content

Fix safe-heap regression with handling of existing imports#1237

Merged
kripken merged 1 commit intomasterfrom
fix-existing-safe-heap
Oct 24, 2017
Merged

Fix safe-heap regression with handling of existing imports#1237
kripken merged 1 commit intomasterfrom
fix-existing-safe-heap

Conversation

@kripken
Copy link
Copy Markdown
Member

@kripken kripken commented Oct 23, 2017

That case wasn't tested. Added a test with the fix.

@kripken
Copy link
Copy Markdown
Member Author

kripken commented Oct 23, 2017

For some reason flake8 is now showing errors it didn't before (on stuff not changed by this PR). Very strange. I pushed what I think is a fix, but does anyone know why it flake8 would suddenly check more things?

@binji
Copy link
Copy Markdown
Member

binji commented Oct 23, 2017

Looks like it upgraded from flake8-3.4.1 to flake8-3.5.0. Probably would be good to pin it if possible.

Comment thread scripts/fuzz_passes.py Outdated
apply_passes(smaller)
assert run() == normal
except:
except Exception, e:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Modern style is except Exception: or except Exception as e:

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks, updated.

@kripken
Copy link
Copy Markdown
Member Author

kripken commented Oct 24, 2017

I don't know what's going on with this PR... now the windows machines are showing an error on unchanged code, as if they also just changed to new settings...

Worse, this doesn't makes sense. It says first may be used initialized, but it's a parameter :-o

  template<typename T, typename... Args>
  T pickGivenNum(size_t num, T first, Args... args) {
    if (num == 0) return first;
    return pickGivenNum<T>(num - 1, args...);
  }
C:/projects/binaryen/src/tools/translate-to-fuzz.h: In member function 'wasm::Expression* wasm::TranslateToFuzzReader::makeConst(wasm::WasmType)':
C:/projects/binaryen/src/tools/translate-to-fuzz.h:1267:5: error: 'first' may be used uninitialized in this function [-Werror=maybe-uninitialized]
   T pickGivenNum(size_t num, T first, Args... args) {
     ^~~~~~~~~~~~
[ 79%] Linking CXX executable bin/wasm-shell.exe
cc1plus.exe: all warnings being treated as errors

@kripken
Copy link
Copy Markdown
Member Author

kripken commented Oct 24, 2017

Yeah, gcc on appveyor bot has upgraded from 6.3 to 7.2.

Could be a gcc bug. I don't see anything in the tracker that looks similar, though :(

@kripken kripken force-pushed the fix-existing-safe-heap branch from 67eea87 to 712e0ac Compare October 24, 2017 17:32
@kripken
Copy link
Copy Markdown
Member Author

kripken commented Oct 24, 2017

Anyhow, there's a better PR for the python stuff, #1239, and I'll look into the new gcc failure separately as well. Merging this PR which is unrelated to those two new failures, and which fixes existing breakage.

@kripken kripken merged commit a26b8ce into master Oct 24, 2017
@kripken kripken deleted the fix-existing-safe-heap branch October 24, 2017 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants