[PyTorch Mobile] Move static declaration of default_extra_files_mobile from header into .cpp file#50795
Closed
dhruvbird wants to merge 4 commits intogh/dhruvbird/26/basefrom
Closed
[PyTorch Mobile] Move static declaration of default_extra_files_mobile from header into .cpp file#50795dhruvbird wants to merge 4 commits intogh/dhruvbird/26/basefrom
dhruvbird wants to merge 4 commits intogh/dhruvbird/26/basefrom
Conversation
…e from header into .cpp file There's [a post](https://fb.workplace.com/groups/2148543255442743/permalink/2583012411995823/) about a customer having to pass in `-Wno-global-constructors` to disable warnings related to calling constructors for global objects. This is related to the initialization of `default_extra_files_mobile` in `import.h`. Instead, we can move this into `import.cpp` and assign it `extern` linkage for use from the `import.h` file. Differential Revision: [D25968216](https://our.internmc.facebook.com/intern/diff/D25968216/) [ghstack-poisoned]
dhruvbird
added a commit
that referenced
this pull request
Jan 20, 2021
…e from header into .cpp file There's [a post](https://fb.workplace.com/groups/2148543255442743/permalink/2583012411995823/) about a customer having to pass in `-Wno-global-constructors` to disable warnings related to calling constructors for global objects. This is related to the initialization of `default_extra_files_mobile` in `import.h`. Instead, we can move this into `import.cpp` and assign it `extern` linkage for use from the `import.h` file. Differential Revision: [D25968216](https://our.internmc.facebook.com/intern/diff/D25968216/) ghstack-source-id: 120019239 Pull Request resolved: #50795
…files_mobile from header into .cpp file" There's [a post](https://fb.workplace.com/groups/2148543255442743/permalink/2583012411995823/) about a customer having to pass in `-Wno-global-constructors` to disable warnings related to calling constructors for global objects. This is related to the initialization of `default_extra_files_mobile` in `import.h`. Instead, we can move this into `import.cpp` and assign it `extern` linkage for use from the `import.h` file. Differential Revision: [D25968216](https://our.internmc.facebook.com/intern/diff/D25968216/) [ghstack-poisoned]
dhruvbird
added a commit
that referenced
this pull request
Jan 20, 2021
…der import.h Pull Request resolved: #50795 There's [a post](https://fb.workplace.com/groups/2148543255442743/permalink/2583012411995823/) about a customer having to pass in `-Wno-global-constructors` to disable warnings related to calling constructors for global objects. This is related to the initialization of `default_extra_files_mobile` in `import.h`. It requires end users to pass in the compiler flag, since the definition is now in code (.cpp files) that they will be compiling. In addition, it makes the API for `_load_for_mobile` non-re-entrant (i.e. can not be safely used concurrently from multiple threads without the caller taking a mutex/lock) if the `extra_files_mobile` argument is not explicitly passed in. Instead, a better option would be to create different overloads; one which requires all 3 parameters, and one that can work with 1-2. This solves the problem without creating a static variable. ghstack-source-id: 120055794 Differential Revision: [D25968216](https://our.internmc.facebook.com/intern/diff/D25968216/)
…files_mobile from header into .cpp file" There's [a post](https://fb.workplace.com/groups/2148543255442743/permalink/2583012411995823/) about a customer having to pass in `-Wno-global-constructors` to disable warnings related to calling constructors for global objects. This is related to the initialization of `default_extra_files_mobile` in `import.h`. Instead, we can move this into `import.cpp` and assign it `extern` linkage for use from the `import.h` file. Differential Revision: [D25968216](https://our.internmc.facebook.com/intern/diff/D25968216/) [ghstack-poisoned]
…files_mobile from header into .cpp file" There's [a post](https://fb.workplace.com/groups/2148543255442743/permalink/2583012411995823/) about a customer having to pass in `-Wno-global-constructors` to disable warnings related to calling constructors for global objects. This is related to the initialization of `default_extra_files_mobile` in `import.h`. Instead, we can move this into `import.cpp` and assign it `extern` linkage for use from the `import.h` file. Differential Revision: [D25968216](https://our.internmc.facebook.com/intern/diff/D25968216/) [ghstack-poisoned]
Codecov Report
@@ Coverage Diff @@
## gh/dhruvbird/26/base #50795 +/- ##
========================================================
- Coverage 81.01% 81.01% -0.01%
========================================================
Files 1916 1916
Lines 209285 209291 +6
========================================================
+ Hits 169562 169564 +2
- Misses 39723 39727 +4 |
Contributor
|
This pull request has been merged in bb909d2. |
laurentdupin
pushed a commit
to laurentdupin/pytorch
that referenced
this pull request
Apr 24, 2026
…der import.h (pytorch#50795) Summary: Pull Request resolved: pytorch#50795 There's [a post](https://fb.workplace.com/groups/2148543255442743/permalink/2583012411995823/) about a customer having to pass in `-Wno-global-constructors` to disable warnings related to calling constructors for global objects. This is related to the initialization of `default_extra_files_mobile` in `import.h`. It requires end users to pass in the compiler flag, since the definition is now in code (.cpp files) that they will be compiling. In addition, it makes the API for `_load_for_mobile` non-re-entrant (i.e. can not be safely used concurrently from multiple threads without the caller taking a mutex/lock) if the `extra_files_mobile` argument is not explicitly passed in. Instead, a better option would be to create different overloads; one which requires all 3 parameters, and one that can work with 1-2. This solves the problem without creating a static variable. ghstack-source-id: 120127083 Test Plan: Build Lite Interpreter and sandcastle. Reviewed By: raziel Differential Revision: D25968216 fbshipit-source-id: fbd80dfcafb8ef7231aca301445c4a2ca9a08995
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stack from ghstack:
There's a post about a customer having to pass in
-Wno-global-constructorsto disable warnings related to calling constructors for global objects. This is related to the initialization ofdefault_extra_files_mobileinimport.h. Instead, we can move this intoimport.cppand assign itexternlinkage for use from theimport.hfile.Differential Revision: D25968216