Skip to content

fixed: In timer.cc:81, the destructor for Profiler fails as this does… #2888

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.

Sign up for GitHub

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

Merged
danpovey merged 1 commit into from
Nov 29, 2018
Merged

fixed: In timer.cc:81, the destructor for Profiler fails as this does… #2888

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/base/timer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ class Timer {
private:
LARGE_INTEGER time_start_;
};
}


#else
#include <sys/time.h>
Expand Down Expand Up @@ -88,6 +88,8 @@ class Timer {
struct timezone time_zone_;
};

#endif

class Profiler {
public:
Copy link
Copy Markdown
Contributor

@danpovey danpovey Nov 29, 2018

Choose a reason for hiding this comment

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

These changes seem unrelated to the original issue about class Profiler. I think the fix is to simply add {} after
~Profiler()

Copy link
Copy Markdown
Contributor Author

@huangruizhe huangruizhe Nov 29, 2018

Choose a reason for hiding this comment

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

Oh, it seems in the original file, the class Profiler will only be enabled under the Unix settings -- that is, when if defined(_MSC_VER) || defined(MINGW) (line 26) is false (starting from line 58).
Can we just add add {} after ~Profiler()?

Copy link
Copy Markdown
Contributor Author

@huangruizhe huangruizhe Nov 29, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

If just add {} after ~Profiler(), there will be no class definition for class Profiler in windows settings

Copy link
Copy Markdown
Contributor

@danpovey danpovey Nov 29, 2018

Choose a reason for hiding this comment

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

I'm not sure what you are saying, but I still think that is the correct fix. can you try that, and we can ask Brett to test it?

Copy link
Copy Markdown
Contributor

@danpovey danpovey Nov 29, 2018

Choose a reason for hiding this comment

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

Oh sorry, I see it now, your fix might be right.
@btiplitz can you please test?

Copy link
Copy Markdown
Contributor

@btiplitz btiplitz Nov 29, 2018

Choose a reason for hiding this comment

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

I’ll try it in the morning

Copy link
Copy Markdown
Contributor

@btiplitz btiplitz Nov 29, 2018

Choose a reason for hiding this comment

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

@danpovey It did build successfully

// Caution: the 'const char' should always be a string constant; for speed,
Expand All @@ -109,7 +111,5 @@ class Profiler {

} // namespace kaldi

#endif


#endif // KALDI_BASE_TIMER_H_