Fixed "is not" with literal python warnings (LLNL#319)

- also more robust -Wall detection and -Wrange-loop-analysis fix in Spot.cpp
duzhuqi · Nov 17, 2020 · 5ed3da7 · 5ed3da7
1 parent 39b7b13
commit 5ed3da7
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 29 deletions.
diff --git a/src/services/CMakeLists.txt b/src/services/CMakeLists.txt
@@ -1,6 +1,13 @@
 # A macro to include service module source files in the caliper runtime lib
 include_directories(${CMAKE_CURRENT_BINARY_DIR})
 
+include(CheckCXXCompilerFlag)
+set(Wall_flag)
+check_cxx_compiler_flag("-Wall" Supports_Wall_Flag)
+if(Supports_Wall_Flag)
+    set(Wall_flag "-Wall")
+endif()
+
 set(CALIPER_SERVICES_SRC_ROOT_DIR ${CMAKE_CURRENT_SOURCE_DIR})
 
 macro(add_service_sources)
@@ -31,8 +38,7 @@ macro(add_service_objlib)
   if(${BUILD_SHARED_LIBS})
     set_property(TARGET "${ARGN}" PROPERTY POSITION_INDEPENDENT_CODE TRUE)
   endif()
-  target_compile_options("${ARGN}" PRIVATE
-    $<$<OR:$<CXX_COMPILER_ID:Clang>,$<CXX_COMPILER_ID:AppleClang>,$<CXX_COMPILER_ID:GNU>>:-Wall>)
+  target_compile_options("${ARGN}" PRIVATE ${Wall_flag})
   list(APPEND CALIPER_SERVICES_LIBS "$<TARGET_OBJECTS:${ARGN}>")
   set(CALIPER_SERVICES_LIBS ${CALIPER_SERVICES_LIBS} PARENT_SCOPE)
 endmacro()
@@ -133,8 +139,7 @@ add_custom_target(
 
 add_library(caliper-services OBJECT ${CALIPER_SERVICES_SOURCES})
 
-target_compile_options(caliper-services PRIVATE
-  $<$<OR:$<CXX_COMPILER_ID:Clang>,$<CXX_COMPILER_ID:AppleClang>,$<CXX_COMPILER_ID:GNU>>:-Wall>)
+target_compile_options(caliper-services PRIVATE ${Wall_flag})
 
 add_dependencies(caliper-services gen-services)
 

diff --git a/src/services/gen_services_inc.py.in b/src/services/gen_services_inc.py.in
@@ -39,7 +39,7 @@ copyrightNotice="""
 
 initial_service_string = "@CALIPER_SERVICE_NAMES@"
 def insert_magic_here(service_string):
-    service_list=[x for x in service_string.split(";") if x is not '']
+    service_list=[x for x in service_string.split(";") if x]
     # print service_list
     service_ret = []
     for service in service_list:
@@ -66,10 +66,10 @@ service_declaration_text=""
 for service in services_list:
     service_name = service[0]
     service_condition = service[1]
-    if service_condition is not "":
+    if service_condition:
         service_declaration_text += "#ifdef "+service_condition+"\n"
     service_declaration_text += "extern CaliperService "+service_name+";\n"
-    if service_condition is not "":
+    if service_condition:
         service_declaration_text += "#endif\n"
 
 service_list_definition_text = """
@@ -84,10 +84,10 @@ service_list_definition_text = """
 for service in services_list:
     service_name = service[0]
     service_condition = service[1]
-    if service_condition is not "":
+    if service_condition:
         service_list_definition_text += "#ifdef "+service_condition+"\n"
     service_list_definition_text += "        "+service_name+",\n"
-    if service_condition is not "":
+    if service_condition:
         service_list_definition_text += "#endif\n"
 
 service_list_definition_text += """

diff --git a/src/services/spot/Spot.cpp b/src/services/spot/Spot.cpp
@@ -60,7 +60,7 @@ namespace
 
         template<typename T>
         using List = std::vector<T>;
-        
+
         using AggregationHandler = cali::Aggregator;
         using SelectionHandler = cali::RecordSelector;
 
@@ -82,25 +82,25 @@ namespace
             AggregationHandler aggregator;
             SelectionHandler   selector;
         };
-        
+
         List<QueryProcessingPipeline> m_queries;
-        
+
         std::vector<std::string> y_axes;
 
         AggregationDescriptorList m_annotations_and_places;
         JsonListType m_jsons;
         std::string code_version;
         std::string recorded_time;
         std::vector<std::string> title;
-        
+
         void write_output_cb(Caliper* c, Channel* chn, const SnapshotRecord* flush_info) {
             int num_queries = m_queries.size();
             for (int i = 0; i < num_queries; ++i) {
                 Log(2).stream() << "spot: Flushing query " << i << std::endl;
 
                 auto &query = m_queries[i];
 
-                c->flush(chn, flush_info, 
+                c->flush(chn, flush_info,
                     [&query](CaliperMetadataAccessInterface& db, const std::vector<Entry>& rec){
                         if (query.selector.pass(db, rec)) {
                             query.aggregator.add(db, rec);
@@ -144,8 +144,8 @@ namespace
              std::ifstream ifs(place);
              std::string str(std::istreambuf_iterator<char>{ifs}, {});
              cali_rapidjson::Document doc;
-             cali_rapidjson::Value xtic_value; 
-             cali_rapidjson::Value commit_value; 
+             cali_rapidjson::Value xtic_value;
+             cali_rapidjson::Value commit_value;
              xtic_value.SetString(recorded_time.c_str(),doc.GetAllocator());
              commit_value.SetString(code_version.c_str(),doc.GetAllocator());
              if(str.size() > 0){
@@ -162,7 +162,7 @@ namespace
                        arrarr.PushBack(0,doc.GetAllocator());
                        arrarr.PushBack(((float)datum.second)/(1.0*divisor),doc.GetAllocator());
                        series_data.PushBack(arrarr,doc.GetAllocator());
-               } 
+               }
              }
              else {
                const char* json_string = "{\"show_exclusive\" : false}";
@@ -187,11 +187,11 @@ namespace
                auto& json_times = doc["XTics"];
                json_commits.GetArray().PushBack(commit_value,doc.GetAllocator());
                json_times.GetArray().PushBack(xtic_value,doc.GetAllocator());
-               cali_rapidjson::Value& series_array = doc["series"]; 
+               cali_rapidjson::Value& series_array = doc["series"];
                for(auto datum : json) {
                   std::string series_name = datum.first;
                   if(series_name.size()>1){
-                    cali_rapidjson::Value value_series_name; 
+                    cali_rapidjson::Value value_series_name;
                     value_series_name.SetString(series_name.c_str(),doc.GetAllocator());
                     series_array.GetArray().PushBack(value_series_name,doc.GetAllocator());
                     cali_rapidjson::Value outarr;
@@ -204,7 +204,7 @@ namespace
                     value_series_name.SetString(series_name.c_str(),doc.GetAllocator());
                     doc.AddMember(value_series_name,outarr,doc.GetAllocator());
                   }
-               } 
+               }
              }
 
              std::ofstream ofs(place.c_str());
@@ -213,7 +213,7 @@ namespace
              doc.Accept(writer);
           }
         }
-        
+
         Spot(Caliper* c, Channel* chn) {
             ConfigSet    config = chn->config().init("spot", s_configdata);
             std::string  config_string = config.get("config").to_string().c_str();
@@ -225,7 +225,7 @@ namespace
               struct tm * timeinfo;
               time (&rawtime);
               timeinfo = localtime (&rawtime);
-              recorded_time = std::string(asctime(timeinfo)); 
+              recorded_time = std::string(asctime(timeinfo));
             }
             std::string title_string = config.get("title").to_string();
             bool use_default_title = (title_string.size() == 0);
@@ -236,7 +236,7 @@ namespace
             util::split(y_axes_string,':',std::back_inserter(y_axes));
             std::vector<std::string> logging_configurations;
             util::split(config_string,',',std::back_inserter(logging_configurations));
-            for(const auto log_config : logging_configurations){
+            for(const auto& log_config : logging_configurations){
               m_jsons.emplace_back(std::vector<std::pair<std::string,TimeType>>());
               std::vector<std::string> annotation_and_place;
               util::split(log_config,':',std::back_inserter(annotation_and_place));
@@ -257,17 +257,17 @@ namespace
         }
 
         QueryProcessingPipeline create_query_processor(std::string query){
-           CalQLParser parser(query.c_str()); 
+           CalQLParser parser(query.c_str());
            if (parser.error()) {
-               CalQLParser default_parser("SELECT *"); 
+               CalQLParser default_parser("SELECT *");
                QuerySpec default_spec(default_parser.spec());
                Log(0).stream() << "spot: config parse error: " << parser.error_msg() << std::endl;
                return QueryProcessingPipeline(Aggregator(default_spec),RecordSelector(default_spec));
            }
            QuerySpec    spec(parser.spec());
            return { Aggregator(spec), RecordSelector(spec) };
         }
-               
+
         static std::string query_for_annotation(std::string grouping, std::string metric = "inclusive_sum(sum#time.duration)"){
           std::string end_grouping = "event.end#"+grouping;
           return "SELECT " + grouping+","+metric+",* WHERE " +grouping;
@@ -276,12 +276,12 @@ namespace
     public:
 
         ~Spot()
-            { 
+            {
             }
 
         static void create(Caliper* c, Channel* chn) {
             Spot* instance = new Spot(c, chn);
-            
+
             chn->events().write_output_evt.connect(
                 [instance](Caliper* c, Channel* chn, const SnapshotRecord* info){
                     instance->write_output_cb(c, chn, info);
@@ -324,7 +324,7 @@ namespace
         ConfigSet::Terminator
     };
 
-} // namespace 
+} // namespace
 
 namespace cali
 {